Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import React from 'react';
import { render } from '@testing-library/react';

import {
NotificationsSkeleton,
ToolbarSkeleton,
} from '../../../../../lib/components/app-layout/visual-refresh-toolbar/skeleton/skeleton-parts';

describe('ToolbarSkeleton', () => {
test('renders with discoveredBreadcrumbs', () => {
const { container } = render(
<ToolbarSkeleton
appLayoutInternals={
{
breadcrumbs: null,
discoveredBreadcrumbs: { items: [] },
} as any
}
toolbarProps={{} as any}
/>
);
expect(container).toBeTruthy();
});
});

describe('NotificationsSkeleton', () => {
test('renders', () => {
const { container } = render(<NotificationsSkeleton appLayoutInternals={{} as any}>{null}</NotificationsSkeleton>);
expect(container).toBeTruthy();
});
});
4 changes: 3 additions & 1 deletion src/app-layout/visual-refresh-toolbar/skeleton/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,14 @@ export const SkeletonLayout = ({
contentElAttributes,
} = skeletonSlotsAttributes;

const isWidgetLoaded = !!appLayoutState.widgetizedState;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here we could reuse this isWidgetReady function:

export function isWidgetReady(state: AppLayoutPendingState): state is AppLayoutState {
return !!state.widgetizedState;
}


return (
<VisualContext contextName="app-layout-toolbar">
<div
{...getAnalyticsMetadataAttribute({ component: componentAnalyticsMetadata })}
ref={appLayoutState.rootRef as React.Ref<HTMLDivElement>}
data-awsui-app-layout-widget-loaded={false}
data-awsui-app-layout-widget-loaded={isWidgetLoaded}
{...wrapperElAttributes}
className={wrapperElAttributes?.className ?? clsx(styles.root, testutilStyles.root)}
style={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import React from 'react';
import { AppLayoutNotificationsImplementationProps } from '../notifications';
import { AppLayoutToolbarImplementationProps } from '../toolbar';
import { SkeletonPartProps } from './interfaces';
import { BreadcrumbsSlot, NotificationsSlot, ToolbarSlot } from './slots';
import { NotificationsSlot } from './slots';
import { ToolbarSkeletonStructure } from './toolbar-container';

import styles from './styles.css.js';

Expand All @@ -17,11 +18,7 @@ export const BeforeMainSlotSkeleton = React.forwardRef<HTMLElement, SkeletonPart
({ toolbarProps, appLayoutProps }, ref) => {
return (
<>
{!!toolbarProps && (
<ToolbarSlot ref={ref}>
<BreadcrumbsSlot ownBreadcrumbs={appLayoutProps.breadcrumbs} />
</ToolbarSlot>
)}
{!!toolbarProps && <ToolbarSkeletonStructure ref={ref} ownBreadcrumbs={appLayoutProps.breadcrumbs} />}
{toolbarProps?.navigationOpen && <div className={styles.navigation} />}
</>
);
Expand All @@ -34,15 +31,14 @@ export const BeforeMainSlotSkeleton = React.forwardRef<HTMLElement, SkeletonPart

export const ToolbarSkeleton = React.forwardRef<HTMLElement, AppLayoutToolbarImplementationProps>(
({ appLayoutInternals }: AppLayoutToolbarImplementationProps, ref) => (
<ToolbarSlot ref={ref}>
<BreadcrumbsSlot
ownBreadcrumbs={appLayoutInternals.breadcrumbs}
discoveredBreadcrumbs={appLayoutInternals.discoveredBreadcrumbs}
/>
</ToolbarSlot>
<ToolbarSkeletonStructure
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codecov points at this line (34) and line 43 below as needing coverage. To be fair, it looks like they were already missing it. Is the change in line 43 necessary though?

ref={ref}
ownBreadcrumbs={appLayoutInternals.breadcrumbs}
discoveredBreadcrumbs={appLayoutInternals.discoveredBreadcrumbs}
/>
)
);

export const NotificationsSkeleton = React.forwardRef<HTMLElement, AppLayoutNotificationsImplementationProps>(
(props: AppLayoutNotificationsImplementationProps, ref) => <NotificationsSlot ref={ref} />
(_props: AppLayoutNotificationsImplementationProps, ref) => <NotificationsSlot ref={ref} />
);
16 changes: 13 additions & 3 deletions src/app-layout/visual-refresh-toolbar/skeleton/slots.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,14 @@ interface ToolbarSlotProps {
}

export const ToolbarSlot = React.forwardRef<HTMLElement, ToolbarSlotProps>(({ className, style, children }, ref) => (
<section ref={ref as React.Ref<any>} className={clsx(styles['toolbar-container'], className)} style={style}>
<section
ref={ref as React.Ref<any>}
className={clsx(styles['toolbar-container'], className)}
style={{
insetBlockStart: style?.insetBlockStart ?? 0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this line the actual fix? Undefined was not behaving the same way as 0?

...style,
}}
>
{children}
</section>
));
Expand All @@ -41,10 +48,13 @@ interface BreadcrumbsSlotProps {
}

export function BreadcrumbsSlot({ ownBreadcrumbs, discoveredBreadcrumbs }: BreadcrumbsSlotProps) {
const isSSR = typeof window === 'undefined';
const contextValue = React.useMemo(() => ({ isInToolbar: true }), []);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to use useMemo here? Unless we really know that the benefit outweighs the overhead, I would be inclined to simplify.


return (
<BreadcrumbsSlotContext.Provider value={{ isInToolbar: true }}>
<BreadcrumbsSlotContext.Provider value={contextValue}>
<div className={styles['breadcrumbs-own']}>{ownBreadcrumbs}</div>
{discoveredBreadcrumbs && (
{discoveredBreadcrumbs && !isSSR && (
<div className={styles['breadcrumbs-discovered']}>
<BreadcrumbGroupImplementation
{...discoveredBreadcrumbs}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import React from 'react';
import clsx from 'clsx';

import { BreadcrumbGroupProps } from '../../../breadcrumb-group/interfaces';
import { BreadcrumbsSlot, ToolbarSlot } from './slots';

import testutilStyles from '../../test-classes/styles.css.js';
import toolbarStyles from '../toolbar/styles.css.js';

interface ToolbarContainerProps {
children: React.ReactNode;
hasAiDrawer?: boolean;
}

export function ToolbarContainer({ children, hasAiDrawer }: ToolbarContainerProps) {
return (
<div className={clsx(toolbarStyles['toolbar-container'], hasAiDrawer && toolbarStyles['with-ai-drawer'])}>
{children}
</div>
);
}

interface ToolbarBreadcrumbsWrapperProps {
children: React.ReactNode;
includeTestUtils?: boolean;
}

export function ToolbarBreadcrumbsWrapper({ children, includeTestUtils = false }: ToolbarBreadcrumbsWrapperProps) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This component is only used once further down, so it does not need to be exported, or it could even be removed?

return (
<div
className={clsx(toolbarStyles['universal-toolbar-breadcrumbs'], includeTestUtils && testutilStyles.breadcrumbs)}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we only add the testutilStyles.breadcrumbs class conditionally?

>
{children}
</div>
);
}

export function ToolbarDrawersWrapper() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this is used only once below in the same file

return <div className={toolbarStyles['universal-toolbar-drawers']} />;
}

interface ToolbarBreadcrumbsSectionProps {
ownBreadcrumbs: React.ReactNode;
discoveredBreadcrumbs?: BreadcrumbGroupProps | null;
includeTestUtils?: boolean;
}

export function ToolbarBreadcrumbsSection({
ownBreadcrumbs,
discoveredBreadcrumbs,
includeTestUtils = false,
}: ToolbarBreadcrumbsSectionProps) {
return (
<ToolbarBreadcrumbsWrapper includeTestUtils={includeTestUtils}>
<BreadcrumbsSlot ownBreadcrumbs={ownBreadcrumbs} discoveredBreadcrumbs={discoveredBreadcrumbs} />
</ToolbarBreadcrumbsWrapper>
);
}

interface ToolbarSkeletonStructureProps {
ownBreadcrumbs: React.ReactNode;
discoveredBreadcrumbs?: BreadcrumbGroupProps | null;
}

export const ToolbarSkeletonStructure = React.forwardRef<HTMLElement, ToolbarSkeletonStructureProps>(
({ ownBreadcrumbs, discoveredBreadcrumbs }, ref) => (
<ToolbarSlot ref={ref}>
<ToolbarContainer>
<ToolbarBreadcrumbsSection ownBreadcrumbs={ownBreadcrumbs} discoveredBreadcrumbs={discoveredBreadcrumbs} />
<ToolbarDrawersWrapper />
</ToolbarContainer>
</ToolbarSlot>
)
);
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export const useSkeletonSlotsAttributes = (
[customCssProps.navigationWidth]: `${navigationWidth}px`,
[customCssProps.toolsWidth]: `${activeDrawerSize}px`,
},
'data-awsui-app-layout-widget-loaded': true,
};

const mainElAttributes = {
Expand Down
22 changes: 13 additions & 9 deletions src/app-layout/visual-refresh-toolbar/toolbar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@ import clsx from 'clsx';

import { useResizeObserver } from '@cloudscape-design/component-toolkit/internal';

import { createWidgetizedComponent } from '../../../internal/widgets';
import { AppLayoutProps } from '../../interfaces';
import { OnChangeParams } from '../../utils/use-drawers';
import { Focusable, FocusControlMultipleStates } from '../../utils/use-focus-control';
import { AppLayoutInternals } from '../interfaces';
import { BreadcrumbsSlot, ToolbarSlot } from '../skeleton/slots';
import { ToolbarSkeleton } from '../skeleton/skeleton-parts';
import { ToolbarSlot } from '../skeleton/slots';
import { ToolbarBreadcrumbsSection, ToolbarContainer } from '../skeleton/toolbar-container';
import { DrawerTriggers, SplitPanelToggleProps } from './drawer-triggers';
import TriggerButton from './trigger-button';

Expand Down Expand Up @@ -188,7 +191,7 @@ export function AppLayoutToolbarImplementation({
</div>
)}
</Transition>
<div className={clsx(styles['toolbar-container'], !!aiDrawer?.trigger && styles['with-ai-drawer'])}>
<ToolbarContainer hasAiDrawer={!!aiDrawer?.trigger}>
{hasNavigation && (
<nav {...navLandmarkAttributes} className={clsx(styles['universal-toolbar-nav'])}>
<TriggerButton
Expand All @@ -212,12 +215,11 @@ export function AppLayoutToolbarImplementation({
</nav>
)}
{(breadcrumbs || discoveredBreadcrumbs) && (
<div className={clsx(styles['universal-toolbar-breadcrumbs'], testutilStyles.breadcrumbs)}>
<BreadcrumbsSlot
ownBreadcrumbs={appLayoutInternals.breadcrumbs}
discoveredBreadcrumbs={appLayoutInternals.discoveredBreadcrumbs}
/>
</div>
<ToolbarBreadcrumbsSection
ownBreadcrumbs={appLayoutInternals.breadcrumbs}
discoveredBreadcrumbs={appLayoutInternals.discoveredBreadcrumbs}
includeTestUtils={true}
/>
)}
{(drawers?.length || globalDrawers?.length || (hasSplitPanel && splitPanelToggleProps?.displayed)) && (
<div className={clsx(styles['universal-toolbar-drawers'])}>
Expand All @@ -240,7 +242,9 @@ export function AppLayoutToolbarImplementation({
/>
</div>
)}
</div>
</ToolbarContainer>
</ToolbarSlot>
);
}

export const AppLayoutToolbar = createWidgetizedComponent(AppLayoutToolbarImplementation, ToolbarSkeleton);
Loading