Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Sticky offset in nested AppLayout #3013

Merged
merged 10 commits into from
Dec 4, 2024
58 changes: 58 additions & 0 deletions pages/app-layout/multi-layout-with-table-sticky-header.page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import React from 'react';

import AppLayout from '~components/app-layout';
import Button from '~components/button';
import Header from '~components/header';
import Table from '~components/table';

import { generateItems, Instance } from '../table/generate-data';
import { columnsConfig } from '../table/shared-configs';
import ScreenshotArea from '../utils/screenshot-area';
import { Breadcrumbs, Navigation } from './utils/content-blocks';
import { Tools } from './utils/content-blocks';
import labels from './utils/labels';
import * as toolsContent from './utils/tools-content';

const items = generateItems(100);

export default function () {
return (
<ScreenshotArea gutters={false}>
Copy link
Member

Choose a reason for hiding this comment

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

Need to ensure this dev page passes new integration tests added there: #3063

Copy link
Member Author

Choose a reason for hiding this comment

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

It does

<AppLayout
data-testid="main-layout"
ariaLabels={labels}
navigation={<Navigation />}
toolsHide={true}
disableContentPaddings={true}
content={
<AppLayout
data-testid="secondary-layout"
ariaLabels={labels}
breadcrumbs={<Breadcrumbs />}
navigationHide={true}
content={
<Table<Instance>
header={
<Header
variant="awsui-h1-sticky"
description="Demo page with footer"
actions={<Button variant="primary">Create</Button>}
>
Sticky Scrollbar Example
</Header>
}
stickyHeader={true}
variant="full-page"
columnDefinitions={columnsConfig}
items={items}
/>
}
tools={<Tools>{toolsContent.long}</Tools>}
/>
}
/>
</ScreenshotArea>
);
}
13 changes: 12 additions & 1 deletion src/app-layout/__integ__/awsui-applayout.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import useBrowser from '@cloudscape-design/browser-test-tools/use-browser';

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

import testutilStyles from '../../../lib/components/app-layout/test-classes/styles.selectors.js';

Expand Down Expand Up @@ -197,4 +197,15 @@ describe.each(['classic', 'refresh', 'refresh-toolbar'] as Theme[])('%s', theme
await expect(page.getNavPosition()).resolves.toEqual(navBefore);
})
);

testIf(theme === 'refresh-toolbar')(
'should keep header visible and in position while scrolling',
setupTest({ pageName: 'multi-layout-with-table-sticky-header' }, async page => {
const tableWrapper = createWrapper().findTable();
const { top: headerOldOffset } = await page.getBoundingBox(tableWrapper.findHeaderSlot().toSelector());
await page.windowScrollTo({ top: 200 });
const { top: headerNewOffset } = await page.getBoundingBox(tableWrapper.findHeaderSlot().toSelector());
expect(headerOldOffset).toEqual(headerNewOffset);
})
);
});
40 changes: 36 additions & 4 deletions src/app-layout/visual-refresh-toolbar/index.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import React, { useEffect, useImperativeHandle, useState } from 'react';
import React, { useEffect, useImperativeHandle, useLayoutEffect, useRef, useState } from 'react';

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

Expand All @@ -9,6 +9,7 @@ import { SplitPanelSideToggleProps } from '../../internal/context/split-panel-co
import { fireNonCancelableEvent } from '../../internal/events';
import { useControllable } from '../../internal/hooks/use-controllable';
import { useIntersectionObserver } from '../../internal/hooks/use-intersection-observer';
import { useMergeRefs } from '../../internal/hooks/use-merge-refs';
import { useMobile } from '../../internal/hooks/use-mobile';
import { useUniqueId } from '../../internal/hooks/use-unique-id';
import { useGetGlobalBreadcrumbs } from '../../internal/plugins/helpers/use-global-breadcrumbs';
Expand Down Expand Up @@ -78,6 +79,8 @@ const AppLayoutVisualRefreshToolbar = React.forwardRef<AppLayoutProps.Ref, AppLa
const [notificationsHeight, setNotificationsHeight] = useState(0);
const [navigationAnimationDisabled, setNavigationAnimationDisabled] = useState(true);
const [splitPanelAnimationDisabled, setSplitPanelAnimationDisabled] = useState(true);
const [isNested, setIsNested] = useState(false);
const rootRef = useRef<HTMLDivElement>(null);

const [toolsOpen = false, setToolsOpen] = useControllable(controlledToolsOpen, onToolsChange, false, {
componentName: 'AppLayout',
Expand Down Expand Up @@ -432,16 +435,45 @@ const AppLayoutVisualRefreshToolbar = React.forwardRef<AppLayoutProps.Ref, AppLa
placement.inlineSize,
]);

/**
* Returns true if the AppLayout is nested
* Does not apply to iframe
*/
const getIsNestedInAppLayout = (element: HTMLElement | null): boolean => {
let currentElement: Element | null = element?.parentElement ?? null;

// this traverse is needed only for JSDOM
// in real browsers the globalVar will be propagated to all descendants and this loops exits after initial iteration
while (currentElement) {
if (getComputedStyle(currentElement).getPropertyValue(globalVars.stickyVerticalTopOffset)) {
return true;
}
currentElement = currentElement.parentElement;
Copy link
Member

Choose a reason for hiding this comment

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

CSS var values propagate. You do not need to manually walk up

Copy link
Member Author

Choose a reason for hiding this comment

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

Technically, the decision suggested by you is better, but it does not work in u tests env so I turned it back

Copy link
Member

Choose a reason for hiding this comment

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

Okay, so getComputedStyle implementation is JSDOM is more naive.

Let's leave a comment for the future readers this loop is needed for JSDOM.

Fine to keep it because in the real browsers it will exit after the first iteration anyway

}

return false;
};

useLayoutEffect(() => {
if (!hasToolbar) {
setIsNested(getIsNestedInAppLayout(rootRef.current));
}
}, [hasToolbar]);

return (
<>
{/* Rendering a hidden copy of breadcrumbs to trigger their deduplication */}
{!hasToolbar && breadcrumbs ? <ScreenreaderOnly>{breadcrumbs}</ScreenreaderOnly> : null}
<SkeletonLayout
ref={intersectionObserverRef}
ref={useMergeRefs(intersectionObserverRef, rootRef)}
style={{
[globalVars.stickyVerticalTopOffset]: `${verticalOffsets.header}px`,
[globalVars.stickyVerticalBottomOffset]: `${placement.insetBlockEnd}px`,
paddingBlockEnd: splitPanelOpen && splitPanelPosition === 'bottom' ? splitPanelReportedSize : '',
...(hasToolbar || !isNested
? {
[globalVars.stickyVerticalTopOffset]: `${verticalOffsets.header}px`,
[globalVars.stickyVerticalBottomOffset]: `${placement.insetBlockEnd}px`,
}
: {}),
...(!isMobile ? { minWidth: `${minContentWidth}px` } : {}),
}}
toolbar={
Expand Down
Loading