From f6fe2956dfa0d6dd2629dcc9e03dc311439f103f Mon Sep 17 00:00:00 2001 From: Joan Perals Date: Thu, 3 Aug 2023 15:19:55 +0200 Subject: [PATCH 1/6] 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; From 91342657888b66a663b0de85c1fa2a0a2e9f08e6 Mon Sep 17 00:00:00 2001 From: Andrei Zhaleznichenka Date: Thu, 3 Aug 2023 16:32:13 +0200 Subject: [PATCH 2/6] refactor: Sticky columns utils (#1400) --- src/table/__tests__/empty-state.test.tsx | 3 +- src/table/internal.tsx | 1 + .../__tests__/use-sticky-columns.test.tsx | 6 +- src/table/sticky-columns/index.ts | 8 +- src/table/sticky-columns/interfaces.ts | 33 ++++++ .../sticky-columns/use-sticky-columns.ts | 108 ++++-------------- src/table/sticky-columns/utils.ts | 52 +++++++++ 7 files changed, 116 insertions(+), 95 deletions(-) create mode 100644 src/table/sticky-columns/interfaces.ts create mode 100644 src/table/sticky-columns/utils.ts diff --git a/src/table/__tests__/empty-state.test.tsx b/src/table/__tests__/empty-state.test.tsx index 98348e8846..09e481e36a 100644 --- a/src/table/__tests__/empty-state.test.tsx +++ b/src/table/__tests__/empty-state.test.tsx @@ -27,10 +27,9 @@ jest.mock('../../../lib/components/table/sticky-columns', () => ({ })); const mockStickyStateModel = { - isEnabled: false, store: jest.fn(), style: { - wrapper: '', + wrapper: undefined, }, refs: { table: jest.fn(), diff --git a/src/table/internal.tsx b/src/table/internal.tsx index 7503707d1c..1a214c9c22 100644 --- a/src/table/internal.tsx +++ b/src/table/internal.tsx @@ -292,6 +292,7 @@ const InternalTable = React.forwardRef( [styles['has-footer']]: hasFooter, [styles['has-header']]: hasHeader, })} + style={stickyState.style.wrapper} onScroll={handleScroll} {...wrapperProps} > diff --git a/src/table/sticky-columns/__tests__/use-sticky-columns.test.tsx b/src/table/sticky-columns/__tests__/use-sticky-columns.test.tsx index 4ac8178dfc..43d751e4ab 100644 --- a/src/table/sticky-columns/__tests__/use-sticky-columns.test.tsx +++ b/src/table/sticky-columns/__tests__/use-sticky-columns.test.tsx @@ -32,7 +32,7 @@ function createMockTable( return { wrapper, table, cells }; } -test('isEnabled is false, wrapper styles is empty and wrapper listener is not attached when feature is off', () => { +test('wrapper styles is empty and wrapper listener is not attached when feature is off', () => { const tableWrapper = document.createElement('div'); const addTableWrapperOnScrollSpy = jest.spyOn(tableWrapper, 'addEventListener'); const { result } = renderHook(() => @@ -40,12 +40,11 @@ test('isEnabled is false, wrapper styles is empty and wrapper listener is not at ); result.current.refs.wrapper(tableWrapper); - expect(result.current.isEnabled).toBe(false); expect(result.current.style.wrapper).not.toBeDefined(); expect(addTableWrapperOnScrollSpy).not.toHaveBeenCalled(); }); -test('isEnabled is true, wrapper styles is not empty and wrapper listener is attached when feature is on', () => { +test('wrapper styles is not empty and wrapper listener is attached when feature is on', () => { const tableWrapper = document.createElement('div'); const addTableWrapperOnScrollSpy = jest.spyOn(tableWrapper, 'addEventListener'); const { result } = renderHook(() => @@ -53,7 +52,6 @@ test('isEnabled is true, wrapper styles is not empty and wrapper listener is att ); result.current.refs.wrapper(tableWrapper); - expect(result.current.isEnabled).toBe(true); expect(result.current.style.wrapper).toEqual({ scrollPaddingLeft: 0, scrollPaddingRight: 0 }); expect(addTableWrapperOnScrollSpy).toHaveBeenCalledWith('scroll', expect.any(Function)); }); diff --git a/src/table/sticky-columns/index.ts b/src/table/sticky-columns/index.ts index 5452a44a30..ab09629b39 100644 --- a/src/table/sticky-columns/index.ts +++ b/src/table/sticky-columns/index.ts @@ -1,9 +1,5 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -export { - useStickyColumns, - useStickyCellStyles, - StickyColumnsModel, - StickyColumnsCellState, -} from './use-sticky-columns'; +export { StickyColumnsCellState } from './interfaces'; +export { useStickyColumns, useStickyCellStyles, StickyColumnsModel } from './use-sticky-columns'; diff --git a/src/table/sticky-columns/interfaces.ts b/src/table/sticky-columns/interfaces.ts new file mode 100644 index 0000000000..140e65e819 --- /dev/null +++ b/src/table/sticky-columns/interfaces.ts @@ -0,0 +1,33 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +export interface StickyColumnsProps { + visibleColumns: readonly PropertyKey[]; + stickyColumnsFirst: number; + stickyColumnsLast: number; +} + +export interface StickyColumnsState { + cellState: Record; + wrapperState: StickyColumnsWrapperState; +} + +// Cell state is used to apply respective styles and offsets to sticky cells. +export interface StickyColumnsCellState { + padLeft: boolean; + lastLeft: boolean; + lastRight: boolean; + offset: { left?: number; right?: number }; +} + +// Scroll padding is applied to table's wrapper so that the table scrolls when focus goes behind sticky column. +export interface StickyColumnsWrapperState { + scrollPaddingLeft: number; + scrollPaddingRight: number; +} + +export interface CellOffsets { + offsets: Map; + stickyWidthLeft: number; + stickyWidthRight: number; +} diff --git a/src/table/sticky-columns/use-sticky-columns.ts b/src/table/sticky-columns/use-sticky-columns.ts index fdc540c080..4955613f43 100644 --- a/src/table/sticky-columns/use-sticky-columns.ts +++ b/src/table/sticky-columns/use-sticky-columns.ts @@ -6,19 +6,20 @@ import AsyncStore from '../../area-chart/async-store'; import { useStableEventHandler } from '../../internal/hooks/use-stable-event-handler'; import { useResizeObserver } from '../../internal/hooks/container-queries'; import clsx from 'clsx'; +import { + CellOffsets, + StickyColumnsCellState, + StickyColumnsProps, + StickyColumnsState, + StickyColumnsWrapperState, +} from './interfaces'; +import { isCellStatesEqual, isWrapperStatesEqual, updateCellOffsets } from './utils'; // We allow the table to have a minimum of 148px of available space besides the sum of the widths of the sticky columns // This value is an UX recommendation and is approximately 1/3 of our smallest breakpoint (465px) const MINIMUM_SCROLLABLE_SPACE = 148; -interface StickyColumnsProps { - visibleColumns: readonly PropertyKey[]; - stickyColumnsFirst: number; - stickyColumnsLast: number; -} - export interface StickyColumnsModel { - isEnabled: boolean; store: StickyColumnsStore; style: { wrapper?: React.CSSProperties; @@ -30,25 +31,6 @@ export interface StickyColumnsModel { }; } -export interface StickyColumnsState { - cellState: Record; - wrapperState: StickyColumnsWrapperState; -} - -// Cell state is used to apply respective styles and offsets to sticky cells. -export interface StickyColumnsCellState { - padLeft: boolean; - lastLeft: boolean; - lastRight: boolean; - offset: { left?: number; right?: number }; -} - -// Scroll padding is applied to table's wrapper so that the table scrolls when focus goes behind sticky column. -export interface StickyColumnsWrapperState { - scrollPaddingLeft: number; - scrollPaddingRight: number; -} - export function useStickyColumns({ visibleColumns, stickyColumnsFirst, @@ -142,7 +124,6 @@ export function useStickyColumns({ }, []); return { - isEnabled: hasStickyColumns, store, style: { // Provide wrapper styles as props so that a re-render won't cause invalidation. @@ -169,7 +150,6 @@ export function useStickyCellStyles({ columnId, getClassName, }: UseStickyCellStylesProps): StickyCellStyles { - const cellRef = useRef(null) as React.MutableRefObject; const setCell = stickyColumns.refs.cell; // unsubscribeRef to hold the function to unsubscribe from the store's updates @@ -177,15 +157,14 @@ export function useStickyCellStyles({ // refCallback updates the cell ref and sets up the store subscription const refCallback = useCallback( - node => { + cellElement => { if (unsubscribeRef.current) { // Unsubscribe before we do any updates to avoid leaving any subscriptions hanging unsubscribeRef.current(); } // Update cellRef and the store's state to point to the new DOM node - cellRef.current = node; - setCell(columnId, node); + setCell(columnId, cellElement); // Update cell styles imperatively to avoid unnecessary re-renders. const selector = (state: StickyColumnsState) => state.cellState[columnId]; @@ -196,7 +175,6 @@ export function useStickyCellStyles({ } const className = getClassName(state); - const cellElement = cellRef.current; if (cellElement) { Object.keys(className).forEach(key => { if (className[key]) { @@ -212,7 +190,7 @@ export function useStickyCellStyles({ // If the node is not null (i.e., the table cell is being mounted or updated, not unmounted), // set up a new subscription to the store's updates - if (node) { + if (cellElement) { unsubscribeRef.current = stickyColumns.store.subscribe(selector, (newState, prevState) => { updateCellStyles(selector(newState), selector(prevState)); }); @@ -233,23 +211,6 @@ export function useStickyCellStyles({ }; } -function isCellStatesEqual(s1: null | StickyColumnsCellState, s2: null | StickyColumnsCellState): boolean { - if (s1 && s2) { - return ( - s1.padLeft === s2.padLeft && - s1.lastLeft === s2.lastLeft && - s1.lastRight === s2.lastRight && - s1.offset.left === s2.offset.left && - s1.offset.right === s2.offset.right - ); - } - return s1 === s2; -} - -function isWrapperStatesEqual(s1: StickyColumnsWrapperState, s2: StickyColumnsWrapperState): boolean { - return s1.scrollPaddingLeft === s2.scrollPaddingLeft && s1.scrollPaddingRight === s2.scrollPaddingRight; -} - interface UpdateCellStylesProps { wrapper: HTMLElement; table: HTMLElement; @@ -260,9 +221,11 @@ interface UpdateCellStylesProps { } export default class StickyColumnsStore extends AsyncStore { - private cellOffsets = new Map(); - private stickyWidthLeft = 0; - private stickyWidthRight = 0; + private cellOffsets: CellOffsets = { + offsets: new Map(), + stickyWidthLeft: 0, + stickyWidthRight: 0, + }; private isStuckToTheLeft = false; private isStuckToTheRight = false; private padLeft = false; @@ -273,14 +236,17 @@ export default class StickyColumnsStore extends AsyncStore { public updateCellStyles(props: UpdateCellStylesProps) { const hasStickyColumns = props.stickyColumnsFirst + props.stickyColumnsLast > 0; - const hadStickyColumns = this.cellOffsets.size > 0; + const hadStickyColumns = this.cellOffsets.offsets.size > 0; if (hasStickyColumns || hadStickyColumns) { this.updateScroll(props); this.updateCellOffsets(props); this.set(() => ({ cellState: this.generateCellStyles(props), - wrapperState: { scrollPaddingLeft: this.stickyWidthLeft, scrollPaddingRight: this.stickyWidthRight }, + wrapperState: { + scrollPaddingLeft: this.cellOffsets.stickyWidthLeft, + scrollPaddingRight: this.cellOffsets.stickyWidthRight, + }, })); } } @@ -321,8 +287,8 @@ export default class StickyColumnsStore extends AsyncStore { // Determine the offset of the sticky column using the `cellOffsets` state object const isFirstColumn = index === 0; - const stickyColumnOffsetLeft = this.cellOffsets.get(columnId)?.first ?? 0; - const stickyColumnOffsetRight = this.cellOffsets.get(columnId)?.last ?? 0; + const stickyColumnOffsetLeft = this.cellOffsets.offsets.get(columnId)?.first ?? 0; + const stickyColumnOffsetRight = this.cellOffsets.offsets.get(columnId)?.last ?? 0; acc[columnId] = { padLeft: isFirstColumn && this.padLeft, @@ -338,31 +304,7 @@ export default class StickyColumnsStore extends AsyncStore { }; private updateCellOffsets = (props: UpdateCellStylesProps): void => { - const firstColumnsWidths: number[] = []; - for (let i = 0; i < props.visibleColumns.length; i++) { - const element = props.cells[props.visibleColumns[i]]; - const cellWidth = element.getBoundingClientRect().width ?? 0; - firstColumnsWidths[i] = (firstColumnsWidths[i - 1] ?? 0) + cellWidth; - } - - const lastColumnsWidths: number[] = []; - for (let i = props.visibleColumns.length - 1; i >= 0; i--) { - const element = props.cells[props.visibleColumns[i]]; - const cellWidth = element.getBoundingClientRect().width ?? 0; - lastColumnsWidths[i] = (lastColumnsWidths[i + 1] ?? 0) + cellWidth; - } - lastColumnsWidths.reverse(); - - this.stickyWidthLeft = firstColumnsWidths[props.stickyColumnsFirst - 1] ?? 0; - this.stickyWidthRight = lastColumnsWidths[props.stickyColumnsLast - 1] ?? 0; - this.cellOffsets = props.visibleColumns.reduce( - (map, columnId, columnIndex) => - map.set(columnId, { - first: firstColumnsWidths[columnIndex - 1] ?? 0, - last: lastColumnsWidths[props.visibleColumns.length - 1 - columnIndex - 1] ?? 0, - }), - new Map() - ); + this.cellOffsets = updateCellOffsets(props.cells, props); }; private isEnabled = (props: UpdateCellStylesProps): boolean => { @@ -378,7 +320,7 @@ export default class StickyColumnsStore extends AsyncStore { return false; } - const totalStickySpace = this.stickyWidthLeft + this.stickyWidthRight; + const totalStickySpace = this.cellOffsets.stickyWidthLeft + this.cellOffsets.stickyWidthRight; const tablePaddingLeft = parseFloat(getComputedStyle(props.table).paddingLeft) || 0; const tablePaddingRight = parseFloat(getComputedStyle(props.table).paddingRight) || 0; const hasEnoughScrollableSpace = diff --git a/src/table/sticky-columns/utils.ts b/src/table/sticky-columns/utils.ts new file mode 100644 index 0000000000..c734eae9fc --- /dev/null +++ b/src/table/sticky-columns/utils.ts @@ -0,0 +1,52 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { CellOffsets, StickyColumnsCellState, StickyColumnsProps, StickyColumnsWrapperState } from './interfaces'; + +export function isCellStatesEqual(s1: null | StickyColumnsCellState, s2: null | StickyColumnsCellState): boolean { + if (s1 && s2) { + return ( + s1.padLeft === s2.padLeft && + s1.lastLeft === s2.lastLeft && + s1.lastRight === s2.lastRight && + s1.offset.left === s2.offset.left && + s1.offset.right === s2.offset.right + ); + } + return s1 === s2; +} + +export function isWrapperStatesEqual(s1: StickyColumnsWrapperState, s2: StickyColumnsWrapperState): boolean { + return s1.scrollPaddingLeft === s2.scrollPaddingLeft && s1.scrollPaddingRight === s2.scrollPaddingRight; +} + +export function updateCellOffsets(cells: Record, props: StickyColumnsProps): CellOffsets { + const totalColumns = props.visibleColumns.length; + + const firstColumnsWidths: number[] = []; + for (let i = 0; i < Math.min(totalColumns, props.stickyColumnsFirst); i++) { + const element = cells[props.visibleColumns[i]]; + const cellWidth = element.getBoundingClientRect().width ?? 0; + firstColumnsWidths[i] = (firstColumnsWidths[i - 1] ?? 0) + cellWidth; + } + + const lastColumnsWidths: number[] = []; + for (let i = 0; i < Math.min(totalColumns, props.stickyColumnsLast); i++) { + const element = cells[props.visibleColumns[totalColumns - 1 - i]]; + const cellWidth = element.getBoundingClientRect().width ?? 0; + lastColumnsWidths[i] = (lastColumnsWidths[i - 1] ?? 0) + cellWidth; + } + + const stickyWidthLeft = firstColumnsWidths[props.stickyColumnsFirst - 1] ?? 0; + const stickyWidthRight = lastColumnsWidths[props.stickyColumnsLast - 1] ?? 0; + const offsets = props.visibleColumns.reduce( + (map, columnId, columnIndex) => + map.set(columnId, { + first: firstColumnsWidths[columnIndex - 1] ?? 0, + last: lastColumnsWidths[totalColumns - 1 - columnIndex - 1] ?? 0, + }), + new Map() + ); + + return { offsets, stickyWidthLeft, stickyWidthRight }; +} From dd7f779825ddc0a624804f63189848ac0c679193 Mon Sep 17 00:00:00 2001 From: Timo <2446349+timogasda@users.noreply.github.com> Date: Fri, 4 Aug 2023 10:55:34 +0200 Subject: [PATCH 3/6] feat: Add `focus` method to alert (#1388) --- pages/alert/simple.page.tsx | 15 +- .../__snapshots__/documenter.test.ts.snap | 11 +- src/alert/__tests__/alert.test.tsx | 6 + src/alert/index.tsx | 81 ++++----- src/alert/interfaces.ts | 7 + src/alert/internal.tsx | 165 ++++++++++-------- src/alert/styles.scss | 46 ++++- 7 files changed, 207 insertions(+), 124 deletions(-) diff --git a/pages/alert/simple.page.tsx b/pages/alert/simple.page.tsx index dce91d30b0..def7d24120 100644 --- a/pages/alert/simple.page.tsx +++ b/pages/alert/simple.page.tsx @@ -1,7 +1,8 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import React, { useState } from 'react'; -import Alert from '~components/alert'; +import React, { useEffect, useRef, useState } from 'react'; +import Alert, { AlertProps } from '~components/alert'; +import Button from '~components/button'; import Link from '~components/link'; import ScreenshotArea from '../utils/screenshot-area'; import SpaceBetween from '~components/space-between'; @@ -12,10 +13,19 @@ import messages from '~components/i18n/messages/all.en'; export default function AlertScenario() { const [visible, setVisible] = useState(true); + const alertRef = useRef(null); + + useEffect(() => { + if (visible) { + alertRef.current?.focus(); + } + }, [visible]); + return (

Simple alert

+
@@ -27,6 +37,7 @@ export default function AlertScenario() { buttonText="Button text" type="warning" onDismiss={() => setVisible(false)} + ref={alertRef} > Content
diff --git a/src/__tests__/__snapshots__/documenter.test.ts.snap b/src/__tests__/__snapshots__/documenter.test.ts.snap index 58fda8aeb8..746e594c76 100644 --- a/src/__tests__/__snapshots__/documenter.test.ts.snap +++ b/src/__tests__/__snapshots__/documenter.test.ts.snap @@ -16,7 +16,14 @@ when the \`dismissible\` property is set to \`true\`.", "name": "onDismiss", }, ], - "functions": Array [], + "functions": Array [ + Object { + "description": "Sets focus on the alert content.", + "name": "focus", + "parameters": Array [], + "returnType": "void", + }, + ], "name": "Alert", "properties": Array [ Object { @@ -54,7 +61,7 @@ An \`onDismiss\` event is fired when a user clicks the button.", "type": "string", }, Object { - "defaultValue": "\\"info\\"", + "defaultValue": "'info'", "description": "Specifies the type of message you want to display.", "inlineType": Object { "name": "AlertProps.Type", diff --git a/src/alert/__tests__/alert.test.tsx b/src/alert/__tests__/alert.test.tsx index cc38c8fb87..0c681505f1 100644 --- a/src/alert/__tests__/alert.test.tsx +++ b/src/alert/__tests__/alert.test.tsx @@ -105,6 +105,12 @@ describe('Alert Component', () => { wrapper.findDismissButton()!.click(); expect(onDismissSpy).toHaveBeenCalled(); }); + it('can be focused through the API', () => { + let ref: AlertProps.Ref | null = null; + render( (ref = element)} />); + ref!.focus(); + expect(document.activeElement).toHaveClass(styles['alert-focus-wrapper']); + }); }); it('renders `action` content', () => { diff --git a/src/alert/index.tsx b/src/alert/index.tsx index 877c0d4c17..3c310b804e 100644 --- a/src/alert/index.tsx +++ b/src/alert/index.tsx @@ -11,47 +11,50 @@ import { getNameFromSelector, getSubStepAllSelector } from '../internal/analytic export { AlertProps }; -export default function Alert({ type = 'info', visible = true, ...props }: AlertProps) { - const baseComponentProps = useBaseComponent('Alert'); - - const { funnelInteractionId, submissionAttempt, funnelState, errorCount } = useFunnel(); - const { stepNumber, stepNameSelector } = useFunnelStep(); - const { subStepSelector, subStepNameSelector } = useFunnelSubStep(); - - useEffect(() => { - if (funnelInteractionId && visible && type === 'error' && funnelState.current !== 'complete') { - const stepName = getNameFromSelector(stepNameSelector); - const subStepName = getNameFromSelector(subStepNameSelector); - - errorCount.current++; - - if (subStepSelector) { - FunnelMetrics.funnelSubStepError({ - funnelInteractionId, - subStepSelector, - subStepName, - subStepNameSelector, - stepNumber, - stepName, - stepNameSelector, - subStepAllSelector: getSubStepAllSelector(), - }); - } else { - FunnelMetrics.funnelError({ - funnelInteractionId, - }); +const Alert = React.forwardRef( + ({ type = 'info', visible = true, ...props }: AlertProps, ref: React.Ref) => { + const baseComponentProps = useBaseComponent('Alert'); + + const { funnelInteractionId, submissionAttempt, funnelState, errorCount } = useFunnel(); + const { stepNumber, stepNameSelector } = useFunnelStep(); + const { subStepSelector, subStepNameSelector } = useFunnelSubStep(); + + useEffect(() => { + if (funnelInteractionId && visible && type === 'error' && funnelState.current !== 'complete') { + const stepName = getNameFromSelector(stepNameSelector); + const subStepName = getNameFromSelector(subStepNameSelector); + + errorCount.current++; + + if (subStepSelector) { + FunnelMetrics.funnelSubStepError({ + funnelInteractionId, + subStepSelector, + subStepName, + subStepNameSelector, + stepNumber, + stepName, + stepNameSelector, + subStepAllSelector: getSubStepAllSelector(), + }); + } else { + FunnelMetrics.funnelError({ + funnelInteractionId, + }); + } + + return () => { + // eslint-disable-next-line react-hooks/exhaustive-deps + errorCount.current--; + }; } - return () => { - // eslint-disable-next-line react-hooks/exhaustive-deps - errorCount.current--; - }; - } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [funnelInteractionId, visible, submissionAttempt, errorCount]); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [funnelInteractionId, visible, submissionAttempt, errorCount]); - - return ; -} + return ; + } +); applyDisplayName(Alert, 'Alert'); +export default Alert; diff --git a/src/alert/interfaces.ts b/src/alert/interfaces.ts index f3f77b1e53..c70217d2a5 100644 --- a/src/alert/interfaces.ts +++ b/src/alert/interfaces.ts @@ -6,6 +6,13 @@ import { NonCancelableEventHandler } from '../internal/events'; export namespace AlertProps { export type Type = 'success' | 'error' | 'warning' | 'info'; + + export interface Ref { + /** + * Sets focus on the alert content. + */ + focus(): void; + } } export interface AlertProps extends BaseComponentProps { diff --git a/src/alert/internal.tsx b/src/alert/internal.tsx index 7084da95ce..4f4c183e20 100644 --- a/src/alert/internal.tsx +++ b/src/alert/internal.tsx @@ -1,6 +1,6 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import React from 'react'; +import React, { useRef } from 'react'; import clsx from 'clsx'; import { InternalButton } from '../button/internal'; import { IconProps } from '../icon/interfaces'; @@ -11,6 +11,7 @@ import styles from './styles.css.js'; import { fireNonCancelableEvent } from '../internal/events'; import { useContainerBreakpoints } from '../internal/hooks/container-queries'; import { useVisualRefresh } from '../internal/hooks/use-visual-mode'; +import useForwardFocus from '../internal/hooks/forward-focus'; import { AlertProps } from './interfaces'; import { InternalBaseComponentProps } from '../internal/hooks/use-base-component'; import { useMergeRefs } from '../internal/hooks/use-merge-refs'; @@ -27,84 +28,98 @@ const typeToIcon: Record = { type InternalAlertProps = SomeRequired & InternalBaseComponentProps; -export default function InternalAlert({ - type, - statusIconAriaLabel, - visible = true, - dismissible, - dismissAriaLabel, - children, - header, - buttonText, - action, - onDismiss, - onButtonClick, - __internalRootRef = null, - ...rest -}: InternalAlertProps) { - const baseProps = getBaseProps(rest); - const i18n = useInternalI18n('alert'); +const InternalAlert = React.forwardRef( + ( + { + type, + statusIconAriaLabel, + visible = true, + dismissible, + dismissAriaLabel, + children, + header, + buttonText, + action, + onDismiss, + onButtonClick, + __internalRootRef = null, + ...rest + }: InternalAlertProps, + ref: React.Ref + ) => { + const baseProps = getBaseProps(rest); + const i18n = useInternalI18n('alert'); - const [breakpoint, breakpointRef] = useContainerBreakpoints(['xs']); - const mergedRef = useMergeRefs(breakpointRef, __internalRootRef); + const focusRef = useRef(null); + useForwardFocus(ref, focusRef); - const isRefresh = useVisualRefresh(); - const size = isRefresh ? 'normal' : header && children ? 'big' : 'normal'; + const [breakpoint, breakpointRef] = useContainerBreakpoints(['xs']); + const mergedRef = useMergeRefs(breakpointRef, __internalRootRef); - const actionButton = action || ( - fireNonCancelableEvent(onButtonClick)} - formAction="none" - > - {buttonText} - - ); + const isRefresh = useVisualRefresh(); + const size = isRefresh ? 'normal' : header && children ? 'big' : 'normal'; - const hasAction = Boolean(action || buttonText); - const analyticsAttributes = { - [DATA_ATTR_ANALYTICS_ALERT]: type, - }; + const actionButton = action || ( + fireNonCancelableEvent(onButtonClick)} + formAction="none" + > + {buttonText} + + ); - return ( -
- -
-
- -
-
-
- {header &&
{header}
} -
{children}
+ const hasAction = Boolean(action || buttonText); + const analyticsAttributes = { + [DATA_ATTR_ANALYTICS_ALERT]: type, + }; + + return ( +
+ +
+
+
+
+ +
+
+
+ {header &&
{header}
} +
{children}
+
+
+
+ {hasAction &&
{actionButton}
}
- {hasAction &&
{actionButton}
} + {dismissible && ( +
+ fireNonCancelableEvent(onDismiss)} + /> +
+ )}
- {dismissible && ( -
- fireNonCancelableEvent(onDismiss)} - /> -
- )} -
- -
- ); -} + +
+ ); + } +); + +export default InternalAlert; diff --git a/src/alert/styles.scss b/src/alert/styles.scss index 9e4809757a..4f540df4ea 100644 --- a/src/alert/styles.scss +++ b/src/alert/styles.scss @@ -6,6 +6,7 @@ @use 'sass:map'; @use '../internal/styles/tokens' as awsui; @use '../internal/styles' as styles; +@use '../internal/hooks/focus-visible' as focus-visible; @use './motion'; @@ -25,6 +26,9 @@ display: flex; justify-content: flex-start; align-items: flex-start; + // display: grid; + grid-template-columns: auto auto 1fr; + border-radius: awsui.$border-radius-alert; border: awsui.$border-field-width solid; padding: awsui.$space-alert-vertical awsui.$space-alert-horizontal; @@ -50,6 +54,23 @@ /* used in test-utils */ } +.alert-focus-wrapper { + display: flex; + flex: 1 1 0%; + + &:focus { + outline: none; + } + @include focus-visible.when-visible { + @include styles.focus-highlight(awsui.$space-button-focus-outline-gutter); + } +} + +.alert-mobile-block { + display: flex; + flex: 1 1 0%; +} + .text { padding: awsui.$border-field-width 0; // To account for vertical misalignment due to button borders margin: awsui.$space-scaled-xxs awsui.$space-xxs; @@ -65,11 +86,24 @@ } /* stylelint-disable selector-max-type */ -.root.breakpoint-default > div > .alert > .body { - display: block; - & > .action { - margin-left: awsui.$space-xxs; - margin-bottom: awsui.$space-xxs; +.root.breakpoint-default > div > .alert { + // Below the breakpoint, arrange actions below the content + & > .alert-mobile-block { + display: block; + & > .action { + margin-bottom: awsui.$space-xxs; + } + } + + // Indent actions according to the size of the alert icon + &.icon-size-medium > .alert-mobile-block > .action { + margin-left: calc(#{awsui.$size-icon-medium} + #{awsui.$space-xs}); + } + &.icon-size-big > .alert-mobile-block > .action { + margin-left: calc(#{awsui.$size-icon-big} + #{awsui.$space-xs}); + } + &.icon-size-normal > .alert-mobile-block > .action { + margin-left: calc(#{awsui.$size-icon-normal} + #{awsui.$space-xs}); } } /* stylelint-enable selector-max-type */ @@ -122,7 +156,7 @@ $_text-colors: ( .type-#{$type} { border-color: #{map.get($_border-colors, $type)}; background-color: #{map.get($_background-colors, $type)}; - > .icon { + > .alert-mobile-block > .alert-focus-wrapper > .icon { color: #{map.get($_text-colors, $type)}; } } From 0a4390223ab7ee2308c5bf8d461ce4453c72f85a Mon Sep 17 00:00:00 2001 From: Timo <2446349+timogasda@users.noreply.github.com> Date: Fri, 4 Aug 2023 17:11:09 +0200 Subject: [PATCH 4/6] chore: Fix visual regression in alert (#1413) --- src/alert/styles.scss | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/alert/styles.scss b/src/alert/styles.scss index 4f540df4ea..fd74a600f8 100644 --- a/src/alert/styles.scss +++ b/src/alert/styles.scss @@ -47,7 +47,7 @@ .action { white-space: nowrap; - margin-left: awsui.$space-s; + margin-left: awsui.$space-alert-action-left; } .action-button { @@ -79,9 +79,6 @@ } &.message { margin-right: awsui.$space-alert-message-right; - & + .action { - margin-left: awsui.$space-alert-action-left; - } } } From 715264b967c0811cde77f29ec7e98312d780cd45 Mon Sep 17 00:00:00 2001 From: Gethin Webster Date: Mon, 7 Aug 2023 09:14:30 +0200 Subject: [PATCH 5/6] feat: Update primary link styling and usage (#1405) --- pages/cards/hooks.page.tsx | 6 +- pages/cards/permutations.page.tsx | 3 +- pages/help-panel/permutations.page.tsx | 4 +- .../__snapshots__/themes.test.ts.snap | 40 +- .../__snapshots__/design-tokens.test.ts.snap | 20 +- .../__snapshots__/documenter.test.ts.snap | 10 +- src/alert/internal.tsx | 55 +-- src/breadcrumb-group/item/styles.scss | 2 +- src/cards/index.tsx | 111 ++--- src/help-panel/index.tsx | 5 +- .../context/link-default-variant-context.ts | 8 + src/internal/styles/links.scss | 2 + src/link/constants.scss | 13 +- src/link/index.tsx | 6 +- src/link/interfaces.ts | 7 + src/link/internal.tsx | 5 +- src/popover/internal.tsx | 21 +- src/table/internal.tsx | 421 +++++++++--------- .../congratulation-screen.tsx | 2 +- .../components/tutorial-list/index.tsx | 1 + src/wizard/wizard-navigation.tsx | 1 + style-dictionary/classic/colors.ts | 3 +- style-dictionary/classic/typography.ts | 3 - style-dictionary/utils/token-names.ts | 4 - style-dictionary/visual-refresh/colors.ts | 1 - style-dictionary/visual-refresh/typography.ts | 3 - 26 files changed, 382 insertions(+), 375 deletions(-) create mode 100644 src/internal/context/link-default-variant-context.ts diff --git a/pages/cards/hooks.page.tsx b/pages/cards/hooks.page.tsx index f1bbe30a87..fcde6ac35b 100644 --- a/pages/cards/hooks.page.tsx +++ b/pages/cards/hooks.page.tsx @@ -14,7 +14,11 @@ import { EmptyState, getMatchesCountText, paginationLabels, pageSizeOptions } fr import ScreenshotArea from '../utils/screenshot-area'; export const cardDefinition: CardsProps.CardDefinition = { - header: item => {item.id}, + header: item => ( + + {item.id} + + ), sections: [ { id: 'type', diff --git a/pages/cards/permutations.page.tsx b/pages/cards/permutations.page.tsx index 0548481561..417f1e64bb 100644 --- a/pages/cards/permutations.page.tsx +++ b/pages/cards/permutations.page.tsx @@ -9,6 +9,7 @@ import Cards, { CardsProps } from '~components/cards/index'; import createPermutations from '../utils/permutations'; import PermutationsView from '../utils/permutations-view'; import ScreenshotArea from '../utils/screenshot-area'; +import { Link } from '~components'; interface Item { number: number; @@ -27,7 +28,7 @@ function createSimpleItems(count: number) { } const cardDefinition: CardsProps.CardDefinition = { - header: item => item.text, + header: item => (item.number === 2 ? {item.text} : item.text), sections: [ { id: 'description', diff --git a/pages/help-panel/permutations.page.tsx b/pages/help-panel/permutations.page.tsx index da065a84e7..cbe03dd361 100644 --- a/pages/help-panel/permutations.page.tsx +++ b/pages/help-panel/permutations.page.tsx @@ -6,6 +6,7 @@ import Box from '~components/box'; import Icon from '~components/icon'; import ScreenshotArea from '../utils/screenshot-area'; import styles from './styles.scss'; +import { Link } from '~components'; const mainContent = (
@@ -97,7 +98,8 @@ const mainContent = (