From f6fe2956dfa0d6dd2629dcc9e03dc311439f103f Mon Sep 17 00:00:00 2001 From: Joan Perals Date: Thu, 3 Aug 2023 15:19:55 +0200 Subject: [PATCH] fix: Render header background height in sync with content (#1387) --- ...table-with-variable-header-height.page.tsx | 38 ++++++ .../__tests__/background-overlap.test.tsx | 113 ++++++++++++++++++ src/app-layout/visual-refresh/background.tsx | 5 +- src/app-layout/visual-refresh/context.tsx | 24 ++-- src/app-layout/visual-refresh/layout.tsx | 19 +-- .../visual-refresh/use-background-overlap.tsx | 55 +++++++++ .../container-queries/use-resize-observer.ts | 11 +- src/internal/hooks/container-queries/utils.ts | 14 +++ .../__tests__/use-dynamic-overlap.test.tsx | 6 +- .../hooks/use-dynamic-overlap/index.ts | 23 +++- 10 files changed, 261 insertions(+), 47 deletions(-) create mode 100644 pages/app-layout/full-page-table-with-variable-header-height.page.tsx create mode 100644 src/app-layout/__tests__/background-overlap.test.tsx create mode 100644 src/app-layout/visual-refresh/use-background-overlap.tsx create mode 100644 src/internal/hooks/container-queries/utils.ts diff --git a/pages/app-layout/full-page-table-with-variable-header-height.page.tsx b/pages/app-layout/full-page-table-with-variable-header-height.page.tsx new file mode 100644 index 0000000000..850da6009a --- /dev/null +++ b/pages/app-layout/full-page-table-with-variable-header-height.page.tsx @@ -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 ( + + header={ + <> +
Header that changes size when scrolling
+ +
Content
+ + + } + stickyHeader={true} + variant="full-page" + columnDefinitions={columnsConfig} + items={items} + /> + } + /> + ); +} diff --git a/src/app-layout/__tests__/background-overlap.test.tsx b/src/app-layout/__tests__/background-overlap.test.tsx new file mode 100644 index 0000000000..339c3fcd27 --- /dev/null +++ b/src/app-layout/__tests__/background-overlap.test.tsx @@ -0,0 +1,113 @@ +// 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('Background overlap', () => { + function ComponentWithDynamicOverlap() { + const ref = useDynamicOverlap(); + const { hasBackgroundOverlap, isBackgroundOverlapDisabled } = useAppLayoutInternals(); + return ( + <> +
+
{hasBackgroundOverlap.toString()}
+
{isBackgroundOverlapDisabled.toString()}
+ + ); + } + + function ComponentWithoutDynamicOverlap() { + const { hasBackgroundOverlap, isBackgroundOverlapDisabled } = useAppLayoutInternals(); + return ( + <> +
{hasBackgroundOverlap.toString()}
+
{isBackgroundOverlapDisabled.toString()}
+ + ); + } + + function renderApp(appLayoutProps?: AppLayoutProps) { + const { rerender } = render(); + return { + hasBackgroundOverlap: () => screen.getByTestId('has-background-overlap').textContent, + isOverlapDisabled: () => screen.getByTestId('is-background-overlap-disabled').textContent, + rerender: (appLayoutProps?: AppLayoutProps) => rerender(), + }; + } + + beforeEach(() => { + positiveHeight = true; + }); + + describe('is applied', () => { + test('when a child component sets the height dynamically with a height higher than 0', () => { + const { hasBackgroundOverlap, isOverlapDisabled } = renderApp({ + content: , + }); + expect(hasBackgroundOverlap()).toBe('true'); + expect(isOverlapDisabled()).toBe('false'); + }); + + test('when content header is present', () => { + const { hasBackgroundOverlap, isOverlapDisabled } = renderApp({ + content: , + contentHeader: 'Content header', + }); + expect(hasBackgroundOverlap()).toBe('true'); + expect(isOverlapDisabled()).toBe('false'); + }); + }); + + describe('is not applied', () => { + test('when no content header is present and height is 0', () => { + positiveHeight = false; + const { hasBackgroundOverlap, isOverlapDisabled } = renderApp({ + content: , + }); + expect(hasBackgroundOverlap()).toBe('false'); + expect(isOverlapDisabled()).toBe('true'); + }); + + test('when no content header is present and no child component sets the height dynamically', () => { + const { hasBackgroundOverlap, isOverlapDisabled } = renderApp({ + content: , + }); + expect(hasBackgroundOverlap()).toBe('false'); + expect(isOverlapDisabled()).toBe('true'); + }); + }); + + test('is disabled when explicitly specified in the app layout props', () => { + const { isOverlapDisabled } = renderApp({ + content: , + disableContentHeaderOverlap: true, + }); + expect(isOverlapDisabled()).toBe('true'); + }); + + test('is updated accordingly when re-rendering', () => { + const { hasBackgroundOverlap, isOverlapDisabled, rerender } = renderApp({ + content: , + }); + expect(hasBackgroundOverlap()).toBe('true'); + expect(isOverlapDisabled()).toBe('false'); + rerender({ content: }); + expect(hasBackgroundOverlap()).toBe('false'); + expect(isOverlapDisabled()).toBe('true'); + }); +}); diff --git a/src/app-layout/visual-refresh/background.tsx b/src/app-layout/visual-refresh/background.tsx index bb0c00509e..256d9f203f 100644 --- a/src/app-layout/visual-refresh/background.tsx +++ b/src/app-layout/visual-refresh/background.tsx @@ -8,15 +8,14 @@ import styles from './styles.css.js'; export default function Background() { const { breadcrumbs, - contentHeader, - dynamicOverlapHeight, + hasBackgroundOverlap, hasNotificationsContent, hasStickyBackground, isMobile, stickyNotifications, } = useAppLayoutInternals(); - if (!hasNotificationsContent && (!breadcrumbs || isMobile) && !contentHeader && dynamicOverlapHeight <= 0) { + if (!hasNotificationsContent && (!breadcrumbs || isMobile) && !hasBackgroundOverlap) { return null; } diff --git a/src/app-layout/visual-refresh/context.tsx b/src/app-layout/visual-refresh/context.tsx index 95a0736890..27a9c97fdb 100644 --- a/src/app-layout/visual-refresh/context.tsx +++ b/src/app-layout/visual-refresh/context.tsx @@ -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 useBackgroundOverlap from './use-background-overlap'; interface AppLayoutInternals extends AppLayoutProps { activeDrawerId?: string | null; @@ -40,18 +41,19 @@ interface AppLayoutInternals extends AppLayoutProps { drawerRef: React.Ref; resizeHandle: React.ReactElement; drawersTriggerCount: number; - dynamicOverlapHeight: number; handleDrawersClick: (activeDrawerId: string | null, skipFocusControl?: boolean) => void; handleSplitPanelClick: () => void; handleNavigationClick: (isOpen: boolean) => void; handleSplitPanelPreferencesChange: (detail: AppLayoutProps.SplitPanelPreferences) => void; handleSplitPanelResize: (detail: { size: number }) => void; handleToolsClick: (value: boolean, skipFocusControl?: boolean) => void; + hasBackgroundOverlap: boolean; hasDefaultToolsWidth: boolean; hasDrawerViewportOverlay: boolean; hasNotificationsContent: boolean; hasOpenDrawer?: boolean; hasStickyBackground: boolean; + isBackgroundOverlapDisabled: boolean; isMobile: boolean; isNavigationOpen: boolean; isSplitPanelForcedPosition: boolean; @@ -130,13 +132,6 @@ export const AppLayoutInternalsProvider = React.forwardRef( } } - /** - * The overlap height has a default set in CSS but can also be dynamically overridden - * for content types (such as Table and Wizard) that have variable size content in the overlap. - * 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 [hasStickyBackground, setHasStickyBackground] = useState(false); /** @@ -487,6 +482,12 @@ export const AppLayoutInternalsProvider = React.forwardRef( const mainElement = useRef(null); const [mainOffsetLeft, setMainOffsetLeft] = useState(0); + const { hasBackgroundOverlap, updateBackgroundOverlapHeight } = useBackgroundOverlap({ + contentHeader: props.contentHeader, + disableContentHeaderOverlap: props.disableContentHeaderOverlap, + layoutElement, + }); + useLayoutEffect( function handleMainOffsetLeft() { setMainOffsetLeft(mainElement?.current?.offsetLeft ?? 0); @@ -600,7 +601,6 @@ export const AppLayoutInternalsProvider = React.forwardRef( drawerRef, resizeHandle, drawersTriggerCount, - dynamicOverlapHeight, headerHeight, footerHeight, hasDefaultToolsWidth, @@ -611,9 +611,11 @@ export const AppLayoutInternalsProvider = React.forwardRef( handleSplitPanelPreferencesChange, handleSplitPanelResize, handleToolsClick, + hasBackgroundOverlap, hasNotificationsContent, hasOpenDrawer, hasStickyBackground, + isBackgroundOverlapDisabled: props.disableContentHeaderOverlap || !hasBackgroundOverlap, isMobile, isNavigationOpen: isNavigationOpen ?? false, isSplitPanelForcedPosition, @@ -660,7 +662,9 @@ export const AppLayoutInternalsProvider = React.forwardRef( setHasStickyBackground, }} > - {children} + + {children} + ); diff --git a/src/app-layout/visual-refresh/layout.tsx b/src/app-layout/visual-refresh/layout.tsx index 8821a5ed88..45ec49e356 100644 --- a/src/app-layout/visual-refresh/layout.tsx +++ b/src/app-layout/visual-refresh/layout.tsx @@ -23,15 +23,14 @@ export default function Layout({ children }: LayoutProps) { contentHeader, contentType, disableBodyScroll, - disableContentHeaderOverlap, disableContentPaddings, drawersTriggerCount, - dynamicOverlapHeight, footerHeight, hasNotificationsContent, hasStickyBackground, hasOpenDrawer, headerHeight, + isBackgroundOverlapDisabled, isMobile, isNavigationOpen, layoutElement, @@ -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 (
0 && { [customCssProps.overlapHeight]: `${dynamicOverlapHeight}px` }), }} > {children} @@ -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( diff --git a/src/app-layout/visual-refresh/use-background-overlap.tsx b/src/app-layout/visual-refresh/use-background-overlap.tsx new file mode 100644 index 0000000000..d9b04680aa --- /dev/null +++ b/src/app-layout/visual-refresh/use-background-overlap.tsx @@ -0,0 +1,55 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +import React, { useCallback, useState } from 'react'; +import customCssProps from '../../internal/generated/custom-css-properties'; + +/** + * The overlap height has a default set in CSS but can also be dynamically overridden + * for content types (such as Table and Wizard) that have variable size content in the overlap. + * If a child component utilizes a sticky header the hasStickyBackground property will determine + * if the background remains in the same vertical position. + */ +export default function useBackgroundOverlap({ + contentHeader, + disableContentHeaderOverlap, + layoutElement, +}: { + contentHeader: React.ReactNode; + disableContentHeaderOverlap?: boolean; + layoutElement: React.Ref; +}) { + const hasContentHeader = !!contentHeader; + + const [hasBackgroundOverlap, setHasBackgroundOverlap] = useState(hasContentHeader); + + const updateBackgroundOverlapHeight = useCallback( + (height: number) => { + const hasOverlap = hasContentHeader || height > 0; + setHasBackgroundOverlap(hasOverlap); + + /** + * 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. + */ + // Layout component uses RefObject, we don't expect a RefCallback + const element = typeof layoutElement !== 'function' && layoutElement?.current; + if (!element) { + return; + } + if (disableContentHeaderOverlap || !hasOverlap || height <= 0) { + element.style.removeProperty(customCssProps.overlapHeight); + } else { + element.style.setProperty(customCssProps.overlapHeight, `${height}px`); + } + }, + [hasContentHeader, layoutElement, disableContentHeaderOverlap] + ); + + return { + hasBackgroundOverlap, + updateBackgroundOverlapHeight, + }; +} diff --git a/src/internal/hooks/container-queries/use-resize-observer.ts b/src/internal/hooks/container-queries/use-resize-observer.ts index a57836441c..f82911f2a5 100644 --- a/src/internal/hooks/container-queries/use-resize-observer.ts +++ b/src/internal/hooks/container-queries/use-resize-observer.ts @@ -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; @@ -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, - }; -} diff --git a/src/internal/hooks/container-queries/utils.ts b/src/internal/hooks/container-queries/utils.ts new file mode 100644 index 0000000000..68c1668fb5 --- /dev/null +++ b/src/internal/hooks/container-queries/utils.ts @@ -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, + }; +} diff --git a/src/internal/hooks/use-dynamic-overlap/__tests__/use-dynamic-overlap.test.tsx b/src/internal/hooks/use-dynamic-overlap/__tests__/use-dynamic-overlap.test.tsx index 73f6a35a07..0efdb3a0c2 100644 --- a/src/internal/hooks/use-dynamic-overlap/__tests__/use-dynamic-overlap.test.tsx +++ b/src/internal/hooks/use-dynamic-overlap/__tests__/use-dynamic-overlap.test.tsx @@ -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) { diff --git a/src/internal/hooks/use-dynamic-overlap/index.ts b/src/internal/hooks/use-dynamic-overlap/index.ts index f586f88988..ce847c16e9 100644 --- a/src/internal/hooks/use-dynamic-overlap/index.ts +++ b/src/internal/hooks/use-dynamic-overlap/index.ts @@ -3,7 +3,9 @@ import { useContext, useLayoutEffect } from 'react'; import { DynamicOverlapContext } from '../../context/dynamic-overlap-context'; -import { useContainerQuery } from '@cloudscape-design/component-toolkit'; +import { useRef, useCallback } from 'react'; +import { useResizeObserver } from '../container-queries'; +import { ContainerQueryEntry } from '@cloudscape-design/component-toolkit'; export interface UseDynamicOverlapProps { /** @@ -21,21 +23,30 @@ export interface UseDynamicOverlapProps { export function useDynamicOverlap(props?: UseDynamicOverlapProps) { const disabled = props?.disabled ?? false; const setDynamicOverlapHeight = useContext(DynamicOverlapContext); - const [overlapHeight, overlapElementRef] = useContainerQuery(rect => rect.contentBoxHeight); + const overlapElementRef = useRef(null); - useLayoutEffect( - function handleDynamicOverlapHeight() { + const getElement = useCallback(() => overlapElementRef.current, [overlapElementRef]); + const updateState = useCallback( + (entry: ContainerQueryEntry) => { if (!disabled) { - setDynamicOverlapHeight(overlapHeight ?? 0); + setDynamicOverlapHeight(entry.contentBoxHeight); } + }, + [disabled, setDynamicOverlapHeight] + ); + useResizeObserver(getElement, updateState); + + useLayoutEffect( + function handleDynamicOverlapHeight() { + // Set overlap height back to 0 when unmounting return () => { if (!disabled) { setDynamicOverlapHeight(0); } }; }, - [disabled, overlapHeight, setDynamicOverlapHeight] + [disabled, setDynamicOverlapHeight] ); return overlapElementRef;