Skip to content

Commit

Permalink
fix: Hidden breadcrumbs issue (#3147)
Browse files Browse the repository at this point in the history
Co-authored-by: Boris Serdiuk <[email protected]>
  • Loading branch information
georgylobko and just-boris authored Dec 17, 2024
1 parent 049115b commit 3bf2183
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import React, { useRef, useState } from 'react';

import AppLayout from '~components/app-layout';
import BreadcrumbGroup from '~components/breadcrumb-group';
import Header from '~components/header';
import ScreenreaderOnly from '~components/internal/components/screenreader-only';
import Link from '~components/link';
Expand All @@ -12,7 +13,7 @@ import SpaceBetween from '~components/space-between';
import './utils/external-widget';
import { IframeWrapper } from '../utils/iframe-wrapper';
import ScreenshotArea from '../utils/screenshot-area';
import { Breadcrumbs, Tools } from './utils/content-blocks';
import { Tools } from './utils/content-blocks';
import labels from './utils/labels';

function createView(name: string) {
Expand All @@ -21,7 +22,17 @@ function createView(name: string) {
<AppLayout
data-testid="secondary-layout"
ariaLabels={labels}
breadcrumbs={<Breadcrumbs />}
breadcrumbs={
name !== 'page2' && (
<BreadcrumbGroup
onFollow={event => event.preventDefault()}
items={[
{ text: 'Home', href: '#' },
{ text: name, href: `#${name}` },
]}
/>
)
}
navigationHide={true}
content={
<SpaceBetween size="s">
Expand Down
19 changes: 18 additions & 1 deletion src/app-layout/__integ__/multi-app-layout-navigation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { BasePageObject } from '@cloudscape-design/browser-test-tools/page-objec
import useBrowser from '@cloudscape-design/browser-test-tools/use-browser';

import createWrapper from '../../../lib/components/test-utils/selectors';
import { getUrlParams } from './utils';
import { getUrlParams, testIf } from './utils';

const theme = 'refresh-toolbar';

Expand Down Expand Up @@ -53,5 +53,22 @@ describe('Multi app layout navigation', () => {
});
})
);

testIf(iframe)(
'should clean up and restore previous breadcrumb state, specific for a page',
setupTest(async page => {
await expect(page.getText(mainLayout.findBreadcrumbs().toSelector())).resolves.toContain('page1');

await page.clickHref('page2');
expect(await page.isExisting(mainLayout.findBreadcrumbs().toSelector())).toBeFalsy();

await page.clickHref('page3');
await page.waitForVisible(mainLayout.findBreadcrumbs().toSelector());
await expect(page.getText(mainLayout.findBreadcrumbs().toSelector())).resolves.toContain('page3');

await page.clickHref('page1');
await expect(page.getText(mainLayout.findBreadcrumbs().toSelector())).resolves.toContain('page1');
})
);
});
});
5 changes: 5 additions & 0 deletions src/app-layout/visual-refresh-toolbar/contexts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,8 @@ interface BreadcrumbsSlotContextType {

export const BreadcrumbsSlotContext =
awsuiPluginsInternal.sharedReactContexts.createContext<BreadcrumbsSlotContextType>(React, 'BreadcrumbsSlotContext');

export const AppLayoutVisibilityContext = awsuiPluginsInternal.sharedReactContexts.createContext<boolean>(
React,
'AppLayoutVisibilityContext'
);
5 changes: 3 additions & 2 deletions src/app-layout/visual-refresh-toolbar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { useFocusControl, useMultipleFocusControl } from '../utils/use-focus-con
import { useSplitPanelFocusControl } from '../utils/use-split-panel-focus-control';
import { ActiveDrawersContext } from '../utils/visibility-context';
import { computeHorizontalLayout, computeVerticalLayout, CONTENT_PADDING } from './compute-layout';
import { AppLayoutVisibilityContext } from './contexts';
import { AppLayoutInternals } from './interfaces';
import {
AppLayoutDrawer,
Expand Down Expand Up @@ -461,7 +462,7 @@ const AppLayoutVisualRefreshToolbar = React.forwardRef<AppLayoutProps.Ref, AppLa
}, [hasToolbar]);

return (
<>
<AppLayoutVisibilityContext.Provider value={isIntersecting}>
{/* Rendering a hidden copy of breadcrumbs to trigger their deduplication */}
{!hasToolbar && breadcrumbs ? <ScreenreaderOnly>{breadcrumbs}</ScreenreaderOnly> : null}
<SkeletonLayout
Expand Down Expand Up @@ -532,7 +533,7 @@ const AppLayoutVisualRefreshToolbar = React.forwardRef<AppLayoutProps.Ref, AppLa
maxContentWidth={maxContentWidth}
disableContentPaddings={disableContentPaddings}
/>
</>
</AppLayoutVisibilityContext.Provider>
);
}
);
Expand Down
10 changes: 7 additions & 3 deletions src/internal/plugins/helpers/use-global-breadcrumbs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
import { useContext, useEffect, useLayoutEffect, useRef, useState } from 'react';

import { useAppLayoutToolbarEnabled } from '../../../app-layout/utils/feature-flags';
import { BreadcrumbsSlotContext } from '../../../app-layout/visual-refresh-toolbar/contexts';
import {
AppLayoutVisibilityContext,
BreadcrumbsSlotContext,
} from '../../../app-layout/visual-refresh-toolbar/contexts';
import { BreadcrumbGroupProps } from '../../../breadcrumb-group/interfaces';
import { awsuiPluginsInternal } from '../api';
import { BreadcrumbsGlobalRegistration } from '../controllers/breadcrumbs';
Expand All @@ -13,11 +16,12 @@ function useSetGlobalBreadcrumbsImplementation({
...props
}: BreadcrumbGroupProps<any> & { __disableGlobalization?: boolean }) {
const { isInToolbar } = useContext(BreadcrumbsSlotContext) ?? {};
const isLayoutVisible = useContext(AppLayoutVisibilityContext) ?? true;
const registrationRef = useRef<BreadcrumbsGlobalRegistration<BreadcrumbGroupProps> | null>();
const [registered, setRegistered] = useState(false);

useEffect(() => {
if (isInToolbar || __disableGlobalization) {
if (isInToolbar || __disableGlobalization || !isLayoutVisible) {
return;
}
const registration = awsuiPluginsInternal.breadcrumbs.registerBreadcrumbs(props, isRegistered =>
Expand All @@ -30,7 +34,7 @@ function useSetGlobalBreadcrumbsImplementation({
};
// subsequent prop changes are handled by another effect
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isInToolbar, __disableGlobalization]);
}, [isInToolbar, __disableGlobalization, isLayoutVisible]);

useLayoutEffect(() => {
registrationRef.current?.update(props);
Expand Down

0 comments on commit 3bf2183

Please sign in to comment.