Skip to content

Commit

Permalink
fix: Render Visual Refresh background height in sync with content (#1331
Browse files Browse the repository at this point in the history
)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Jessica Kuelz <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Connor Lanigan <[email protected]>
Co-authored-by: Rúben Carvalho <[email protected]>
Co-authored-by: Abdallah AlHalees <[email protected]>
Co-authored-by: Timo <[email protected]>
Co-authored-by: Gethin Webster <[email protected]>
Co-authored-by: Avinash Dwarapu <[email protected]>
Co-authored-by: Andrei Zhaleznichenka <[email protected]>
Co-authored-by: Boris Serdiuk <[email protected]>
Co-authored-by: Johannes Weber <[email protected]>
  • Loading branch information
12 people authored Jul 28, 2023
1 parent c203e09 commit 254fbb1
Show file tree
Hide file tree
Showing 9 changed files with 227 additions and 41 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// 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 labels from './utils/labels';
import Table from '~components/table';
import { generateItems, Instance } from '../table/generate-data';
import { columnsConfig } from '../table/shared-configs';
import ExpandableSection from '~components/expandable-section';
import Header from '~components/header';

const items = generateItems(20);

export default function () {
return (
<AppLayout
ariaLabels={labels}
contentType="table"
navigationHide={true}
content={
<Table<Instance>
header={
<>
<Header variant="awsui-h1-sticky">Header that changes size when scrolling</Header>
<ExpandableSection headerText="Click to expand header area">
<div style={{ height: '300px' }}>Content</div>
</ExpandableSection>
</>
}
stickyHeader={true}
variant="full-page"
columnDefinitions={columnsConfig}
items={items}
/>
}
/>
);
}
106 changes: 106 additions & 0 deletions src/app-layout/__tests__/dynamic-overlap.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import React from 'react';
import { render, screen } from '@testing-library/react';
import AppLayout, { AppLayoutProps } from '../../../lib/components/app-layout';
import { useDynamicOverlap } from '../../../lib/components/internal/hooks/use-dynamic-overlap';
import { useAppLayoutInternals } from '../../../lib/components/app-layout/visual-refresh/context';

jest.mock('../../../lib/components/internal/hooks/use-visual-mode', () => ({
...jest.requireActual('../../../lib/components/internal/hooks/use-visual-mode'),
useVisualRefresh: jest.fn().mockReturnValue(true),
}));

let positiveHeight = true;

jest.mock('../../../lib/components/internal/hooks/container-queries/utils', () => ({
...jest.requireActual('../../../lib/components/internal/hooks/container-queries/utils'),
convertResizeObserverEntry: () => ({ contentBoxHeight: positiveHeight ? 800 : 0 }),
}));

describe('Dynamic overlap', () => {
function ComponentWithDynamicOverlap() {
const ref = useDynamicOverlap();
const { isDynamicOverlapSet, isDynamicOverlapDisabled } = useAppLayoutInternals();
return (
<>
<div ref={ref} />
<div data-testid="is-dynamic-overlap-set">{isDynamicOverlapSet.toString()}</div>
<div data-testid="is-dynamic-overlap-disabled">{isDynamicOverlapDisabled.toString()}</div>
</>
);
}

function ComponentWithoutDynamicOverlap() {
const { isDynamicOverlapSet, isDynamicOverlapDisabled } = useAppLayoutInternals();
return (
<>
<div data-testid="is-dynamic-overlap-set">{isDynamicOverlapSet.toString()}</div>
<div data-testid="is-dynamic-overlap-disabled">{isDynamicOverlapDisabled.toString()}</div>
</>
);
}

function renderApp(appLayoutProps?: AppLayoutProps) {
const { rerender } = render(<AppLayout {...appLayoutProps} />);
return {
isDynamicOverlapSet: () => screen.getByTestId('is-dynamic-overlap-set').textContent,
isDynamicOverlapDisabled: () => screen.getByTestId('is-dynamic-overlap-disabled').textContent,
rerender: (appLayoutProps?: AppLayoutProps) => rerender(<AppLayout {...appLayoutProps} />),
};
}

beforeEach(() => {
positiveHeight = true;
});

test('sets dynamic overlap when content header is present', () => {
const { isDynamicOverlapSet, isDynamicOverlapDisabled } = renderApp({
content: <ComponentWithDynamicOverlap />,
contentHeader: 'Content header',
});
expect(isDynamicOverlapSet()).toBe('true');
expect(isDynamicOverlapDisabled()).toBe('false');
});

test('sets dynamic overlap when height is higher than 0', () => {
const { isDynamicOverlapSet, isDynamicOverlapDisabled } = renderApp({ content: <ComponentWithDynamicOverlap /> });
expect(isDynamicOverlapSet()).toBe('true');
expect(isDynamicOverlapDisabled()).toBe('false');
});

test('does not set dynamic overlap when no content header is present and height is 0', () => {
positiveHeight = false;
const { isDynamicOverlapSet, isDynamicOverlapDisabled } = renderApp({ content: <ComponentWithDynamicOverlap /> });
expect(isDynamicOverlapSet()).toBe('false');
expect(isDynamicOverlapDisabled()).toBe('true');
});

test('does not set dynamic overlap when the useDynamicOverlap hook is not used', () => {
const { isDynamicOverlapSet, isDynamicOverlapDisabled } = renderApp({
content: <ComponentWithoutDynamicOverlap />,
disableContentHeaderOverlap: true,
});
expect(isDynamicOverlapSet()).toBe('false');
expect(isDynamicOverlapDisabled()).toBe('true');
});

test('disables dynamic overlap when explicitly specified in the app layout props', () => {
const { isDynamicOverlapDisabled } = renderApp({
content: <ComponentWithDynamicOverlap />,
disableContentHeaderOverlap: true,
});
expect(isDynamicOverlapDisabled()).toBe('true');
});

test('updates state accordingly when re-rendering', () => {
const { isDynamicOverlapSet, isDynamicOverlapDisabled, rerender } = renderApp({
content: <ComponentWithDynamicOverlap />,
});
expect(isDynamicOverlapSet()).toBe('true');
expect(isDynamicOverlapDisabled()).toBe('false');
rerender({ content: <ComponentWithoutDynamicOverlap /> });
expect(isDynamicOverlapSet()).toBe('false');
expect(isDynamicOverlapDisabled()).toBe('true');
});
});
5 changes: 2 additions & 3 deletions src/app-layout/visual-refresh/background.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,14 @@ import styles from './styles.css.js';
export default function Background() {
const {
breadcrumbs,
contentHeader,
dynamicOverlapHeight,
isDynamicOverlapSet,
hasNotificationsContent,
hasStickyBackground,
isMobile,
stickyNotifications,
} = useAppLayoutInternals();

if (!hasNotificationsContent && (!breadcrumbs || isMobile) && !contentHeader && dynamicOverlapHeight <= 0) {
if (!hasNotificationsContent && (!breadcrumbs || isMobile) && !isDynamicOverlapSet) {
return null;
}

Expand Down
46 changes: 42 additions & 4 deletions src/app-layout/visual-refresh/context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { warnOnce } from '@cloudscape-design/component-toolkit/internal';
import useResize from '../utils/use-resize';
import styles from './styles.css.js';
import { useContainerQuery } from '@cloudscape-design/component-toolkit';
import customCssProps from '../../internal/generated/custom-css-properties';

interface AppLayoutInternals extends AppLayoutProps {
activeDrawerId?: string | null;
Expand All @@ -40,7 +41,6 @@ interface AppLayoutInternals extends AppLayoutProps {
drawerRef: React.Ref<HTMLElement>;
resizeHandle: React.ReactElement;
drawersTriggerCount: number;
dynamicOverlapHeight: number;
handleDrawersClick: (activeDrawerId: string | null, skipFocusControl?: boolean) => void;
handleSplitPanelClick: () => void;
handleNavigationClick: (isOpen: boolean) => void;
Expand All @@ -52,6 +52,8 @@ interface AppLayoutInternals extends AppLayoutProps {
hasNotificationsContent: boolean;
hasOpenDrawer?: boolean;
hasStickyBackground: boolean;
isDynamicOverlapDisabled: boolean;
isDynamicOverlapSet: boolean;
isMobile: boolean;
isNavigationOpen: boolean;
isSplitPanelForcedPosition: boolean;
Expand Down Expand Up @@ -136,7 +138,8 @@ export const AppLayoutInternalsProvider = React.forwardRef(
* If a child component utilizes a sticky header the hasStickyBackground property will determine
* if the background remains in the same vertical position.
*/
const [dynamicOverlapHeight, setDynamicOverlapHeight] = useState(0);
const [isDynamicOverlapSet, setIsDynamicOverlapSet] = useState(false);
const [isDynamicOverlapDisabled, setIsDynamicOverlapDisabled] = useState(true);
const [hasStickyBackground, setHasStickyBackground] = useState(false);

/**
Expand Down Expand Up @@ -487,6 +490,40 @@ export const AppLayoutInternalsProvider = React.forwardRef(
const mainElement = useRef<HTMLDivElement>(null);
const [mainOffsetLeft, setMainOffsetLeft] = useState(0);

const hasContentHeader = !!props.contentHeader;

const updateDynamicOverlapHeight = useCallback(
(height: number) => {
/**
* The disableContentHeaderOverlap property is absolute and will always disable the overlap
* if it is set to true. If there is no contentHeader then the overlap should be disabled
* unless there is a dynamicOverlapHeight. The dynamicOverlapHeight property is set by a
* component in the content slot that needs to manually control the overlap height.
*/
const isOverlapSet = hasContentHeader || height > 0;
const isOverlapDisabled = props.disableContentHeaderOverlap || !isOverlapSet;
setIsDynamicOverlapSet(isOverlapSet);
setIsDynamicOverlapDisabled(isOverlapDisabled);

/**
* React 18 will trigger a paint before the state is correctly updated
* (see https://github.com/facebook/react/issues/24331).
* To work around this, we bypass React state updates and imperatively update the custom property on the DOM.
* An alternative would be to use `queueMicrotask` and `flushSync` in the ResizeObserver callback,
* but that would have some performance impact as it would delay the render.
* Using React state for `isDynamicOverlapSet` and `isDynamicOverlapDisabled` is less problematic
* because they will rarely change within the lifecycle of an application.
*/
const element = typeof layoutElement !== 'function' && layoutElement?.current; // Layout component uses RefObject
if (isOverlapDisabled || height <= 0) {
element?.style.removeProperty(customCssProps.overlapHeight);
} else {
element?.style.setProperty(customCssProps.overlapHeight, `${height}px`);
}
},
[layoutElement, hasContentHeader, props.disableContentHeaderOverlap]
);

useLayoutEffect(
function handleMainOffsetLeft() {
setMainOffsetLeft(mainElement?.current?.offsetLeft ?? 0);
Expand Down Expand Up @@ -600,7 +637,6 @@ export const AppLayoutInternalsProvider = React.forwardRef(
drawerRef,
resizeHandle,
drawersTriggerCount,
dynamicOverlapHeight,
headerHeight,
footerHeight,
hasDefaultToolsWidth,
Expand All @@ -616,6 +652,8 @@ export const AppLayoutInternalsProvider = React.forwardRef(
hasStickyBackground,
isMobile,
isNavigationOpen: isNavigationOpen ?? false,
isDynamicOverlapDisabled,
isDynamicOverlapSet,
isSplitPanelForcedPosition,
isSplitPanelOpen,
isToolsOpen,
Expand Down Expand Up @@ -660,7 +698,7 @@ export const AppLayoutInternalsProvider = React.forwardRef(
setHasStickyBackground,
}}
>
<DynamicOverlapContext.Provider value={setDynamicOverlapHeight}>{children}</DynamicOverlapContext.Provider>
<DynamicOverlapContext.Provider value={updateDynamicOverlapHeight}>{children}</DynamicOverlapContext.Provider>
</AppLayoutContext.Provider>
</AppLayoutInternalsContext.Provider>
);
Expand Down
19 changes: 4 additions & 15 deletions src/app-layout/visual-refresh/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,14 @@ export default function Layout({ children }: LayoutProps) {
contentHeader,
contentType,
disableBodyScroll,
disableContentHeaderOverlap,
disableContentPaddings,
drawersTriggerCount,
dynamicOverlapHeight,
footerHeight,
hasNotificationsContent,
hasStickyBackground,
hasOpenDrawer,
headerHeight,
isDynamicOverlapDisabled,
isMobile,
isNavigationOpen,
layoutElement,
Expand All @@ -53,14 +52,6 @@ export default function Layout({ children }: LayoutProps) {
const hasContentGapLeft = isNavigationOpen || navigationHide;
const hasContentGapRight = drawersTriggerCount <= 0 || hasOpenDrawer;

/**
* The disableContentHeaderOverlap property is absolute and will always disable the overlap
* if it is set to true. If there is no contentHeader then the overlap should be disabled
* unless there is a dynamicOverlapHeight. The dynamicOverlapHeight property is set by a
* component in the content slot that needs to manually control the overlap height.
*/
const isOverlapDisabled = disableContentHeaderOverlap || (!contentHeader && dynamicOverlapHeight <= 0);

return (
<main
className={clsx(
Expand All @@ -80,7 +71,7 @@ export default function Layout({ children }: LayoutProps) {
[styles['has-split-panel']]: splitPanelDisplayed,
[styles['has-sticky-background']]: hasStickyBackground,
[styles['has-sticky-notifications']]: stickyNotifications && hasNotificationsContent,
[styles['is-overlap-disabled']]: isOverlapDisabled,
[styles['is-overlap-disabled']]: isDynamicOverlapDisabled,
},
testutilStyles.root
)}
Expand All @@ -93,8 +84,6 @@ export default function Layout({ children }: LayoutProps) {
...(maxContentWidth && { [customCssProps.maxContentWidth]: `${maxContentWidth}px` }),
...(minContentWidth && { [customCssProps.minContentWidth]: `${minContentWidth}px` }),
[customCssProps.notificationsHeight]: `${notificationsHeight}px`,
...(!isOverlapDisabled &&
dynamicOverlapHeight > 0 && { [customCssProps.overlapHeight]: `${dynamicOverlapHeight}px` }),
}}
>
{children}
Expand All @@ -104,8 +93,8 @@ export default function Layout({ children }: LayoutProps) {

/*
The Notifications, Breadcrumbs, Header, and Main are all rendered in the center
column of the grid layout. Any of these could be the first child to render in the
content area if the previous siblings do not exist. The grid gap before the first
column of the grid layout. Any of these could be the first child to render in the
content area if the previous siblings do not exist. The grid gap before the first
child will be different to ensure vertical alignment with the trigger buttons.
*/
function getContentFirstChild(
Expand Down
11 changes: 1 addition & 10 deletions src/internal/hooks/container-queries/use-resize-observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ResizeObserver, ResizeObserverEntry } from '@juggle/resize-observer';
import React, { useEffect, useLayoutEffect } from 'react';
import { useStableEventHandler } from '../use-stable-event-handler';
import { ContainerQueryEntry } from '@cloudscape-design/component-toolkit';
import { convertResizeObserverEntry } from './utils';

type ElementReference = (() => Element | null) | React.RefObject<Element>;

Expand Down Expand Up @@ -58,13 +59,3 @@ export function useResizeObserver(elementRef: ElementReference, onObserve: (entr
}
}, [elementRef, stableOnObserve]);
}

function convertResizeObserverEntry(entry: ResizeObserverEntry): ContainerQueryEntry {
return {
target: entry.target,
contentBoxWidth: entry.contentBoxSize[0].inlineSize,
contentBoxHeight: entry.contentBoxSize[0].blockSize,
borderBoxWidth: entry.borderBoxSize[0].inlineSize,
borderBoxHeight: entry.borderBoxSize[0].blockSize,
};
}
14 changes: 14 additions & 0 deletions src/internal/hooks/container-queries/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import { ResizeObserverEntry } from '@juggle/resize-observer';
import { ContainerQueryEntry } from '@cloudscape-design/component-toolkit';

export function convertResizeObserverEntry(entry: ResizeObserverEntry): ContainerQueryEntry {
return {
target: entry.target,
contentBoxWidth: entry.contentBoxSize[0].inlineSize,
contentBoxHeight: entry.contentBoxSize[0].blockSize,
borderBoxWidth: entry.borderBoxSize[0].inlineSize,
borderBoxHeight: entry.borderBoxSize[0].blockSize,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import { render, screen } from '@testing-library/react';
import { useDynamicOverlap } from '../../../../../lib/components/internal/hooks/use-dynamic-overlap';
import { DynamicOverlapContext } from '../../../../../lib/components/internal/context/dynamic-overlap-context';

jest.mock('@cloudscape-design/component-toolkit', () => ({
...jest.requireActual('@cloudscape-design/component-toolkit'),
useContainerQuery: () => [800, () => {}],
jest.mock('../../../../../lib/components/internal/hooks/container-queries/utils', () => ({
...jest.requireActual('../../../../../lib/components/internal/hooks/container-queries/utils'),
convertResizeObserverEntry: () => ({ contentBoxHeight: 800 }),
}));

function renderApp(children: React.ReactNode) {
Expand Down
Loading

0 comments on commit 254fbb1

Please sign in to comment.