From 7245654ae256dd66a88b52b5dc449ebc41dd177a Mon Sep 17 00:00:00 2001 From: Joan Perals Tresserra Date: Tue, 6 Oct 2026 18:54:43 +0200 Subject: [PATCH 01/29] feat: Add deadlock-safe auto-wrap to internal ControlGroup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add built-in responsive wrap/stack to the internal ControlGroup, reconciled onto main's consumer-controlled `direction` prop. Prop/design: `direction?: 'horizontal' | 'vertical' | 'auto'`, default `'auto'`. - `'auto'` measures the available width (hidden inert ghost row measured with useContainerQuery + bounded ancestor-walk + ResizeObserver) and resolves to `'horizontal'` when the single row fits, `'vertical'` when it does not. - `'horizontal'`/`'vertical'` force that axis and skip measurement (no ghost rendered). The component-level `'auto'` is resolved to a two-value `GroupedControlDirection` (`resolvedDirection`) before it feeds the `root-${dir}` class, each `control-${position}-${dir}` class, and the `GroupedControlContext` value `{ position, direction: resolvedDirection }` — byte-for-byte consistent. The old boolean `.stacked` class is removed in favor of main's `-vertical` scheme. Per-file resolution: - index.tsx: keep OURS measurement machinery (ghost ref, inert effect, measureAvailableWidth ancestor-walk, ResizeObserver, useMergeRefs) + THEIRS direction prop/class/context scheme; widen prop to add `'auto'`; add `resolvedDirection`; gate ghost render on `direction === 'auto'`. - styles.scss: keep THEIRS `root-horizontal`/`root-vertical` and `control-*-horizontal`/`control-*-vertical` seam rules; add `flex-shrink: 0` and `position: relative` on `.root` and the `.ghost` row rule; drop all `.stacked` selectors. - control-group.test.tsx: keep THEIRS position/direction suites (switched to getAllByTestId(...)[0] for the ghost duplicate) + OURS responsiveness suite rewritten from `.stacked` to `root-horizontal`/`root-vertical`; add a forced-direction-bypass test (no ghost rendered). Verification (run from the worktree root): - Build: `npm run quick-build` — OK (finished, lib/components current). - Unit tests: `TZ=UTC node_modules/.bin/jest -c jest.unit.config.js` on src/internal/components/control-group/__tests__/control-group.test.tsx (14), src/input/__tests__/control-group.test.tsx (2), src/select/__tests__/control-group.test.tsx (1), src/autosuggest/__tests__/control-group.test.tsx (1), src/multiselect/__tests__/control-group.test.tsx (2) — 5 suites, 19 passed, 0 failed. - Typecheck: `npx tsc --noEmit` — clean (exit 0). - Lint: `npx eslint` on index.tsx + control-group.test.tsx — 0 errors; `npx stylelint` on styles.scss — 0 errors. --- pages/control-group/responsiveness.page.tsx | 85 ++++++++ src/input/__tests__/control-group.test.tsx | 11 +- .../__tests__/control-group.test.tsx | 203 ++++++++++++++++-- .../components/control-group/index.tsx | 176 +++++++++++++-- .../components/control-group/styles.scss | 28 +++ .../__tests__/control-group.test.tsx | 7 +- 6 files changed, 464 insertions(+), 46 deletions(-) create mode 100644 pages/control-group/responsiveness.page.tsx diff --git a/pages/control-group/responsiveness.page.tsx b/pages/control-group/responsiveness.page.tsx new file mode 100644 index 0000000000..d25313bf06 --- /dev/null +++ b/pages/control-group/responsiveness.page.tsx @@ -0,0 +1,85 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +import React, { useState } from 'react'; + +import Box from '~components/box'; +import Input from '~components/input'; +import ControlGroup from '~components/internal/components/control-group'; +import Select, { SelectProps } from '~components/select'; +import SpaceBetween from '~components/space-between'; + +import { SimplePage } from '../app/templates'; + +const operators: SelectProps.Option[] = [ + { value: '=', label: '=' }, + { value: '!=', label: '!=' }, +]; + +// A resizable container so the group can be narrowed below its required row width (which +// stacks all controls at once) and widened again (which re-expands them). This exercises +// the ancestor-walk + ResizeObserver. +const resizableContainerStyle: React.CSSProperties = { + resize: 'horizontal', + overflow: 'auto', + inlineSize: 480, + minInlineSize: 160, + maxInlineSize: '100%', + padding: 16, + border: '1px dashed var(--awsui-color-border-divider-default, #b6bec9)', + borderRadius: 8, +}; + +// A control group of real controls, matching the permutations-page fixtures. +function Group() { + const [name, setName] = useState('service'); + const [operator, setOperator] = useState(operators[0]); + const [value, setValue] = useState('production'); + return ( + + setName(e.detail.value)} /> + setValue(e.detail.value)} /> + + ); +} + +export default function ControlGroupResponsiveness() { + return ( + + + + Narrow container +
+ +
+ + + + Inside SpaceBetween (flexbox deadlock) + {/* + The group sits inside a horizontal SpaceBetween (a flex row) alongside another + element, itself inside the resizable container. A naive "measure my parent" + group would deadlock here: once stacked, the shrink-wrapping flex item reports + the collapsed width, so the group would never see the room to re-expand. The + ancestor-walk skips those shrink-wrapping ancestors, so widening the container + re-expands the group. + */} +
+ + + Sibling content + +
+ + + + ); +} diff --git a/src/input/__tests__/control-group.test.tsx b/src/input/__tests__/control-group.test.tsx index c4c3f019a0..a847ee0629 100644 --- a/src/input/__tests__/control-group.test.tsx +++ b/src/input/__tests__/control-group.test.tsx @@ -10,23 +10,26 @@ import { PositionProbe } from '../../internal/components/control-group/__tests__ const noop = () => {}; describe('Input in control group', () => { + // The control group renders a hidden measurement ghost that duplicates its children, + // so prefix/suffix content (which renders immediately) appears twice. The first match + // is the real control's; the second is in the inert ghost. test('resets the context for prefix content', () => { - const { getByTestId } = render( + const { getAllByTestId } = render( } /> ); - expect(getByTestId('probe')).toHaveTextContent('none'); + expect(getAllByTestId('probe')[0]).toHaveTextContent('none'); }); test('resets the context for suffix content', () => { - const { getByTestId } = render( + const { getAllByTestId } = render( } /> ); - expect(getByTestId('probe')).toHaveTextContent('none'); + expect(getAllByTestId('probe')[0]).toHaveTextContent('none'); }); }); diff --git a/src/internal/components/control-group/__tests__/control-group.test.tsx b/src/internal/components/control-group/__tests__/control-group.test.tsx index cc36ac14a4..290825efe3 100644 --- a/src/internal/components/control-group/__tests__/control-group.test.tsx +++ b/src/internal/components/control-group/__tests__/control-group.test.tsx @@ -1,20 +1,86 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 import React from 'react'; -import { render } from '@testing-library/react'; +import { act, render } from '@testing-library/react'; + +import { useContainerQuery } from '@cloudscape-design/component-toolkit'; import ControlGroup from '../../../../../lib/components/internal/components/control-group'; import { ResetGroupedControlContext } from '../../../../../lib/components/internal/context/control-group-context'; import { DirectionProbe, PositionProbe } from './common'; +import styles from '../../../../../lib/components/internal/components/control-group/styles.css.js'; + +// `requiredRowWidth` comes from `useContainerQuery` (the hidden ghost's content width). +// Mock it so each test controls that width directly; `availableWidth` is driven through +// the stubbed ResizeObserver + ancestor measurement below. +let requiredRowWidth: number | null = null; +jest.mock('@cloudscape-design/component-toolkit', () => ({ + ...jest.requireActual('@cloudscape-design/component-toolkit'), + useContainerQuery: jest.fn(), +})); + +// jsdom does not lay out, so the ancestor-walk sees zero widths and no ResizeObserver. +// Stub a ResizeObserver that fires once on observe, and give the group a parent whose +// content width is `availableWidth` and whose clientWidth is reported wider than the root +// (so the walk stops at it). The root's own width stays 0 in jsdom, so any positive +// `availableWidth` is "wider than the group" and becomes the available width. +let availableWidth = 1000; +// The most recent observer's callback, so a test can fire it to simulate a resize after +// changing `availableWidth`. +let lastObserverCallback: ResizeObserverCallback | null = null; + +class StubResizeObserver { + constructor(callback: ResizeObserverCallback) { + lastObserverCallback = callback; + } + observe() { + lastObserverCallback?.([], this as unknown as ResizeObserver); + } + unobserve() {} + disconnect() {} +} + +function fireResize() { + lastObserverCallback?.([], undefined as unknown as ResizeObserver); +} + +beforeEach(() => { + requiredRowWidth = null; + availableWidth = 1000; + lastObserverCallback = null; + (useContainerQuery as jest.Mock).mockImplementation(() => [requiredRowWidth, () => {}]); + (global as any).ResizeObserver = StubResizeObserver; + // The ancestor-walk reads the parent's computed padding and clientWidth. Report no + // padding and a clientWidth equal to `availableWidth`, wider than the root's 0-width box + // in jsdom, so the walk stops at the immediate parent and uses that width. + jest.spyOn(window, 'getComputedStyle').mockImplementation( + () => + ({ + paddingLeft: '0px', + paddingRight: '0px', + overflowX: 'visible', + overflow: 'visible', + }) as unknown as CSSStyleDeclaration + ); + jest.spyOn(HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => availableWidth); +}); + +afterEach(() => { + jest.restoreAllMocks(); + delete (global as any).ResizeObserver; +}); + describe('Control group', () => { test('keeps focus on a control when the children are reordered', () => { const alpha = ; const beta = ; - const { getByTestId, rerender } = render({[alpha, beta]}); + const { getAllByTestId, rerender } = render({[alpha, beta]}); - const alphaInput = getByTestId('alpha'); + // The ghost duplicates the children, so there are two "alpha" nodes; the first is the + // real (focusable) one, the second is in the inert ghost. + const alphaInput = getAllByTestId('alpha')[0]; alphaInput.focus(); expect(document.activeElement).toBe(alphaInput); @@ -22,23 +88,25 @@ describe('Control group', () => { rerender({[beta, alpha]}); // The same DOM node is still focused; it was moved, not remounted. - expect(getByTestId('alpha')).toBe(alphaInput); + expect(getAllByTestId('alpha')[0]).toBe(alphaInput); expect(document.activeElement).toBe(alphaInput); }); describe('position', () => { + // The ghost duplicates each child, so every probe matches twice; the first match is + // the real control, the second is the inert ghost copy. test('exposes the "only" position to a single child control', () => { - const { getByTestId } = render( + const { getAllByTestId } = render( ); - expect(getByTestId('probe')).toHaveTextContent('only'); + expect(getAllByTestId('probe')[0]).toHaveTextContent('only'); }); test('exposes first / middle / last positions to each child in order', () => { - const { getByTestId } = render( + const { getAllByTestId } = render( @@ -46,15 +114,15 @@ describe('Control group', () => { ); - expect(getByTestId('a')).toHaveTextContent('first'); - expect(getByTestId('b')).toHaveTextContent('middle'); - expect(getByTestId('c')).toHaveTextContent('last'); + expect(getAllByTestId('a')[0]).toHaveTextContent('first'); + expect(getAllByTestId('b')[0]).toHaveTextContent('middle'); + expect(getAllByTestId('c')[0]).toHaveTextContent('last'); }); test('resets the grouped position for content wrapped in ResetGroupedControlContext', () => { // Mirrors a nested control rendered inside a control's custom slot (e.g. // Autosuggest `empty`): it must not inherit the surrounding group position. - const { getByTestId } = render( + const { getAllByTestId } = render( @@ -62,25 +130,28 @@ describe('Control group', () => { ); - expect(getByTestId('probe')).toHaveTextContent('none'); + expect(getAllByTestId('probe')[0]).toHaveTextContent('none'); }); }); describe('direction', () => { + // With the default `direction="auto"` and the jsdom harness (`availableWidth = 1000`, + // `requiredRowWidth = null`), the measurement falls back and `resolvedDirection` is + // `'horizontal'`, so the "defaults to horizontal" expectation holds unchanged. test('defaults the direction to "horizontal" and exposes it to each child', () => { - const { getByTestId } = render( + const { getAllByTestId } = render( ); - expect(getByTestId('a')).toHaveTextContent('horizontal'); - expect(getByTestId('b')).toHaveTextContent('horizontal'); + expect(getAllByTestId('a')[0]).toHaveTextContent('horizontal'); + expect(getAllByTestId('b')[0]).toHaveTextContent('horizontal'); }); test('exposes direction="vertical" to each child when the group is vertical', () => { - const { getByTestId } = render( + const { getAllByTestId } = render( @@ -88,13 +159,13 @@ describe('Control group', () => { ); - expect(getByTestId('a')).toHaveTextContent('vertical'); - expect(getByTestId('b')).toHaveTextContent('vertical'); - expect(getByTestId('c')).toHaveTextContent('vertical'); + expect(getAllByTestId('a')[0]).toHaveTextContent('vertical'); + expect(getAllByTestId('b')[0]).toHaveTextContent('vertical'); + expect(getAllByTestId('c')[0]).toHaveTextContent('vertical'); }); test('ResetGroupedControlContext preserves the group direction', () => { - const { getByTestId } = render( + const { getAllByTestId } = render( @@ -102,7 +173,97 @@ describe('Control group', () => { ); - expect(getByTestId('direction')).toHaveTextContent('vertical'); + expect(getAllByTestId('direction')[0]).toHaveTextContent('vertical'); }); }); }); + +describe('Control group responsiveness', () => { + function renderGroup() { + return render( + + + + + ); + } + + function getRoot(container: HTMLElement) { + return container.querySelector(`.${styles.root}`)!; + } + + test('renders the hidden measurement ghost: aria-hidden and inert, horizontal by default', () => { + requiredRowWidth = 300; + availableWidth = 1000; + const { container } = renderGroup(); + + const ghost = container.querySelector(`.${styles.ghost}`) as HTMLElement; + expect(ghost).not.toBeNull(); + expect(ghost).toHaveAttribute('aria-hidden', 'true'); + // `inert` is set imperatively after mount so the duplicated controls are not focusable + // and not announced (no phantom tab stops). + expect(ghost.inert).toBe(true); + + // The group fits (available 1000 > required 300), so it resolves to horizontal. + expect(getRoot(container)).toHaveClass(styles['root-horizontal']); + expect(getRoot(container)).not.toHaveClass(styles['root-vertical']); + }); + + test('falls back to horizontal while measurements are null', () => { + // No required width measured yet. + requiredRowWidth = null; + availableWidth = 1000; + const { container } = renderGroup(); + expect(getRoot(container)).toHaveClass(styles['root-horizontal']); + expect(getRoot(container)).not.toHaveClass(styles['root-vertical']); + }); + + test('stacks (vertical) when the available width is below the required row width', () => { + requiredRowWidth = 500; + availableWidth = 200; + const { container } = renderGroup(); + expect(getRoot(container)).toHaveClass(styles['root-vertical']); + expect(getRoot(container)).not.toHaveClass(styles['root-horizontal']); + }); + + test('re-expands (horizontal) when the available width grows back above the required width', () => { + requiredRowWidth = 500; + availableWidth = 200; + const { container } = renderGroup(); + expect(getRoot(container)).toHaveClass(styles['root-vertical']); + + // Widen the available space and fire a resize so the ancestor-walk re-runs. This is + // the deadlock-safe re-expand: the walk re-reads the (now wider) ancestor width. + availableWidth = 1000; + act(() => { + fireResize(); + }); + expect(getRoot(container)).toHaveClass(styles['root-horizontal']); + expect(getRoot(container)).not.toHaveClass(styles['root-vertical']); + }); + + test('focus reaches only the real controls, not the inert ghost duplicates', () => { + requiredRowWidth = 300; + const { container } = renderGroup(); + const ghost = container.querySelector(`.${styles.ghost}`) as HTMLElement; + // The ghost subtree is inert, so its duplicated inputs are not in the tab order. + expect(ghost.inert).toBe(true); + expect(ghost.querySelectorAll('input')).toHaveLength(2); + }); + + test('a forced direction wins and bypasses measurement (no ghost rendered)', () => { + // Even with a tiny available width and a large required width, a forced horizontal + // direction stays horizontal and renders no measurement ghost. + requiredRowWidth = 500; + availableWidth = 50; + const { container } = render( + + + + + ); + expect(getRoot(container)).toHaveClass(styles['root-horizontal']); + expect(getRoot(container)).not.toHaveClass(styles['root-vertical']); + expect(container.querySelector(`.${styles.ghost}`)).toBeNull(); + }); +}); diff --git a/src/internal/components/control-group/index.tsx b/src/internal/components/control-group/index.tsx index 1ee3ed8122..a3fa991716 100644 --- a/src/internal/components/control-group/index.tsx +++ b/src/internal/components/control-group/index.tsx @@ -1,8 +1,11 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import React from 'react'; +import React, { forwardRef, useCallback, useLayoutEffect, useRef, useState } from 'react'; import clsx from 'clsx'; +import { useContainerQuery } from '@cloudscape-design/component-toolkit'; +import { useMergeRefs } from '@cloudscape-design/component-toolkit/internal'; + import { BaseComponentProps } from '../../../types/base-component'; import { getBaseProps } from '../../base-component'; import { @@ -14,36 +17,171 @@ import { flattenChildren } from '../../utils/flatten-children'; import styles from './styles.css.js'; +// The prop widens the shared two-value `GroupedControlDirection` with `'auto'`, which only +// exists at the component boundary: `'auto'` measures and resolves to a concrete axis before +// anything reaches context, classes, or SCSS. +type ControlGroupDirection = GroupedControlDirection | 'auto'; + export interface InternalControlGroupProps extends BaseComponentProps { children?: React.ReactNode; - direction?: GroupedControlDirection; + /** + * The axis along which the controls are laid out. + * - `'auto'` (default): measures the available width and lays the controls out as a single + * row when they fit, stacking them vertically when they do not. + * - `'horizontal'` / `'vertical'`: forces that axis and skips measurement. + */ + direction?: ControlGroupDirection; } -export default function InternalControlGroup({ - children, - direction = 'horizontal', - ...props -}: InternalControlGroupProps) { - const baseProps = getBaseProps(props); +const InternalControlGroup = forwardRef( + ({ children, direction = 'auto', ...props }, ref) => { + const baseProps = getBaseProps(props); + + const flattenedChildren = flattenChildren(children, 'ControlGroup'); + const controlCount = flattenedChildren.length; + + // Stack the controls only when they don't fit the available width. The decision + // compares two independent widths so the group's own collapse can't feed back into it + // (which would leave it stuck stacked): + // - `availableWidth`: the width of the line the group sits on (see below). + // - `requiredRowWidth`: the width the controls need as a single row, measured from a + // hidden ghost row that is always a row and out of flow, so it never shrinks. + const [availableWidth, setAvailableWidth] = useState(null); + const [requiredRowWidth, ghostWidthRef] = useContainerQuery(entry => entry.contentBoxWidth); + + // The root is `flex-shrink: 0` (see styles.scss), so it and any shrink-wrapping + // ancestor take the group's width — including the narrow width after it stacks. So to + // measure the real available width we walk up and skip those ancestors, stopping at the + // first one that actually constrains the group. Its width does not follow the collapse, + // so the group re-expands when the space returns. + const rootElRef = useRef(null); + const observerRef = useRef(null); + + const measureAvailableWidth = useCallback(() => { + const root = rootElRef.current; + if (!root) { + return; + } + const rootWidth = root.getBoundingClientRect().width; + let container: HTMLElement | null = root.parentElement; + let available: number | null = null; + // Walk up (bounded) to the first ancestor that constrains the group, skipping ones + // that just shrink-wrap to it. + for (let i = 0; container && i < 20; i++) { + const style = getComputedStyle(container); + // clientWidth excludes borders/scrollbar; subtract padding for the content box. + const paddingInline = parseFloat(style.paddingLeft) + parseFloat(style.paddingRight); + const contentWidth = container.clientWidth - paddingInline; + available = contentWidth; + // Wider than the group: this ancestor defines the available width. + if (contentWidth > rootWidth + 1) { + break; + } + // Not wider, but it clips/scrolls or the group overflows it: it constrains the + // group, so its (narrower) width is the available width. + const clipsOrScrolls = + style.overflowX !== 'visible' || + style.overflow !== 'visible' || + container.scrollWidth > container.clientWidth; + if (clipsOrScrolls) { + break; + } + // Otherwise it just shrink-wraps to the group: skip it and keep looking. + container = container.parentElement; + } + setAvailableWidth(available); + }, []); + + const measureRootRef = useCallback( + (node: HTMLDivElement | null) => { + observerRef.current?.disconnect(); + observerRef.current = null; + rootElRef.current = node; + if (!node || typeof ResizeObserver === 'undefined') { + return; + } + // Re-run the walk whenever the root or any ancestor resizes. + const observer = new ResizeObserver(() => measureAvailableWidth()); + observer.observe(node); + for (let el: HTMLElement | null = node.parentElement, i = 0; el && i < 20; i++, el = el.parentElement) { + observer.observe(el); + } + observerRef.current = observer; + measureAvailableWidth(); + }, + [measureAvailableWidth] + ); + const rootRef = useMergeRefs(ref, measureRootRef); + + useLayoutEffect(() => () => observerRef.current?.disconnect(), []); - const flattenedChildren = flattenChildren(children, 'ControlGroup'); - const controlCount = flattenedChildren.length; + // The ghost row renders a full duplicate of the controls for width measurement. It is + // visually hidden and out of flow, but its inputs/selects/buttons would still be in the + // tab order (and announced), adding phantom tab stops between control groups. Mark the + // whole subtree `inert` so it is non-focusable and hidden from assistive tech. Set + // imperatively via a ref because `inert` isn't rendered by React < 19. + const ghostElRef = useRef(null); + useLayoutEffect(() => { + if (ghostElRef.current) { + ghostElRef.current.inert = true; + } + }); + const ghostRef = useMergeRefs(ghostWidthRef, ghostElRef); - return ( -
- {flattenedChildren.map((child, index) => { + // Decide the layout. In `auto` mode: measure and stack the controls as one unit when the + // row doesn't fit. The `-1` tolerance avoids flipping on sub-pixel rounding; the group + // stays a row until both widths are measured (the documented fallback). + const stacked = + availableWidth !== null && requiredRowWidth !== null ? availableWidth < requiredRowWidth - 1 : false; + + // Resolve the component-level `direction` (which may be `'auto'`) into the two-value + // `GroupedControlDirection` that feeds the root class, every control class, and the + // context. A forced direction wins and ignores the measurement; `'auto'` uses `stacked`. + const resolvedDirection: GroupedControlDirection = + direction === 'auto' ? (stacked ? 'vertical' : 'horizontal') : direction; + + const renderControlSlots = () => + flattenedChildren.map((child, index) => { const key = child && typeof child === 'object' ? (child as Record<'key', unknown>).key : undefined; const position: GroupedControlPosition = controlCount === 1 ? 'only' : index === 0 ? 'first' : index === controlCount - 1 ? 'last' : 'middle'; return (
- {child} + + {child} +
); - })} -
- ); -} + }); + + return ( +
+ {renderControlSlots()} + + {/* + Hidden ghost row, used only to measure the width the controls need as a single + row. It is always a row, `aria-hidden`, inert (set via ref above), and out of + flow, so its width is stable regardless of whether the visible group has stacked. + It duplicates the same children, which is why consumers that probe the DOM by + test id can match both the real and ghost copy. Only rendered in `auto` mode, + since a forced direction needs no measurement. + */} + {direction === 'auto' && ( + + )} +
+ ); + } +); + +export default InternalControlGroup; diff --git a/src/internal/components/control-group/styles.scss b/src/internal/components/control-group/styles.scss index 54c7f93695..988297b13f 100644 --- a/src/internal/components/control-group/styles.scss +++ b/src/internal/components/control-group/styles.scss @@ -9,6 +9,13 @@ .root { @include styles.styles-reset; display: flex; + // `flex-shrink: 0` keeps the group at its natural width in a flex parent (for example + // a `SpaceBetween`), so the parent wraps whole groups to the next line instead of + // squeezing one narrower than its controls — which is what lets the ancestor-walk find + // the real available width. It is inert for a normal block child. + flex-shrink: 0; + // Anchor the absolutely-positioned measurement ghost to the root. + position: relative; &-horizontal { flex-direction: row; @@ -51,3 +58,24 @@ } } } + +// Hidden ghost row, used only to measure the width the controls need as a single row +// (see index.tsx). It is out of flow so it never affects the root's width, and sized to +// its content (`max-content`) so it reports the true natural row width rather than being +// capped by the available space. It must not be `display: none` (that reports zero width) +// and must not be focusable or announced (it is `aria-hidden` + `inert`). It mirrors the +// `.root` row layout so the measurement matches a single inline row. +.ghost { + position: absolute; + inset-block-start: 0; + inset-inline-start: 0; + inline-size: max-content; + visibility: hidden; + pointer-events: none; + block-size: 0; + overflow: hidden; + display: flex; + flex-direction: row; + flex-wrap: nowrap; + align-items: flex-end; +} diff --git a/src/multiselect/__tests__/control-group.test.tsx b/src/multiselect/__tests__/control-group.test.tsx index 06cff025f7..bf97483f64 100644 --- a/src/multiselect/__tests__/control-group.test.tsx +++ b/src/multiselect/__tests__/control-group.test.tsx @@ -13,7 +13,7 @@ const noop = () => {}; describe('Multiselect in control group', () => { test('resets the context for a custom dropdown footer', () => { - const { container, getByTestId } = render( + const { container, getAllByTestId } = render( { ); createWrapper(container).findMultiselect()!.openDropdown(); - expect(getByTestId('probe')).toHaveTextContent('none'); + // The control group renders a hidden measurement ghost that duplicates its children, + // so the dropdown footer renders twice (the real dropdown and the ghost's). The first + // match is the real control's. + expect(getAllByTestId('probe')[0]).toHaveTextContent('none'); }); it('renders inline tokens even if `inlineTokens` is not set', () => { From 4c3222e6655f765f46c31cfbf449e987a073bb0a Mon Sep 17 00:00:00 2001 From: Joan Perals Tresserra Date: Tue, 6 Oct 2026 19:19:12 +0200 Subject: [PATCH 02/29] test: Cover ControlGroup auto-wrap via integ tests Replace the implementation-detail unit tests for the internal ControlGroup with behavioral browser (integ) tests, and retie the dev page to the viewport so setWindowSize can drive the available width. PART A - src/internal/components/control-group/__tests__/control-group.test.tsx Remove the ghost-probing tests (hidden measurement ghost aria-hidden/inert, focus-reaches-only-real-controls) and the measurement-driven class-flipping tests (horizontal fallback while null, stacks when narrow, re-expands when widened, forced direction bypasses measurement), plus the now-unused jsdom harness (useContainerQuery mock, requiredRowWidth/availableWidth/ lastObserverCallback globals, StubResizeObserver, fireResize, the beforeEach/afterEach stubs) and the styles import. Keep the focus-on-reorder test and the position and direction suites, keeping getAllByTestId(...)[0] since the ghost still renders at runtime in auto mode. common.tsx is unchanged (DirectionProbe/PositionProbe still used). PART B - src/internal/components/control-group/__integ__/control-group.test.ts New geometry-based integ test mirroring options-list and the tabs setWindowSize patterns. Navigates to the bare route #/control-group/responsiveness. Asserts ROW (shared top + increasing left) vs STACKED (increasing top) via bounding boxes of the visible controls, never internal classes and never the ghost (filtered out by zero height). Covers: wide -> single row; narrow -> all controls stack; widen -> single row again; auto group inside horizontal SpaceBetween narrow -> stack then widen -> row (flexbox deadlock); forced horizontal stays a row when narrow and forced vertical stays stacked when wide. waitForAssertion lets measurement settle. PART C - pages/control-group/responsiveness.page.tsx Scenario containers are now viewport-driven (inlineSize: '100%' instead of the fixed resizable 480 cap) so setWindowSize changes the available width. Added stable data-testid wrappers: auto-plain, auto-spacebetween (deadlock), forced-horizontal, forced-vertical. Keeps the internal ControlGroup import, real Input/Select with ariaLabels, and the SimplePage template. Verification (run in this worktree): - Build: `npm run quick-build` -> OK. - Unit: `TZ=UTC NODE_OPTIONS=--experimental-vm-modules node_modules/.bin/jest -c jest.unit.config.js src/internal/components/control-group src/input/__tests__/control-group.test.tsx src/multiselect/__tests__/control-group.test.tsx` -> 3 suites, 11 passed. Also autosuggest + select control-group tests -> 2 suites, 2 passed. - Typecheck: `npx tsc --noEmit -p tsconfig.json` -> exit 0. The integ test type-checks clean against the real @cloudscape-design/browser-test-tools types (tsconfig.integ.json fixed to a node16/node mismatch that only `tsc -p` trips, not ts-jest; verified via a temporary override that pairs module=commonjs with moduleResolution=node -> exit 0). - Lint: `eslint` on the three changed files -> exit 0. No scss touched. - Integ not run here (no ChromeDriver/dev server); relies on the draft-PR CI trigger. --- pages/control-group/responsiveness.page.tsx | 52 +++--- .../__integ__/control-group.test.ts | 142 +++++++++++++++ .../__tests__/control-group.test.tsx | 169 +----------------- 3 files changed, 180 insertions(+), 183 deletions(-) create mode 100644 src/internal/components/control-group/__integ__/control-group.test.ts diff --git a/pages/control-group/responsiveness.page.tsx b/pages/control-group/responsiveness.page.tsx index d25313bf06..2a743cd525 100644 --- a/pages/control-group/responsiveness.page.tsx +++ b/pages/control-group/responsiveness.page.tsx @@ -4,7 +4,7 @@ import React, { useState } from 'react'; import Box from '~components/box'; import Input from '~components/input'; -import ControlGroup from '~components/internal/components/control-group'; +import ControlGroup, { InternalControlGroupProps } from '~components/internal/components/control-group'; import Select, { SelectProps } from '~components/select'; import SpaceBetween from '~components/space-between'; @@ -15,27 +15,23 @@ const operators: SelectProps.Option[] = [ { value: '!=', label: '!=' }, ]; -// A resizable container so the group can be narrowed below its required row width (which -// stacks all controls at once) and widened again (which re-expands them). This exercises -// the ancestor-walk + ResizeObserver. -const resizableContainerStyle: React.CSSProperties = { - resize: 'horizontal', - overflow: 'auto', - inlineSize: 480, - minInlineSize: 160, - maxInlineSize: '100%', +// The scenario containers are tied to the viewport width (`inlineSize: '100%'`) so that an +// integ test can drive the available width with `setWindowSize`. Narrowing the viewport below +// the group's required row width stacks all controls at once; widening it re-expands them. +const scenarioContainerStyle: React.CSSProperties = { + inlineSize: '100%', padding: 16, border: '1px dashed var(--awsui-color-border-divider-default, #b6bec9)', borderRadius: 8, }; // A control group of real controls, matching the permutations-page fixtures. -function Group() { +function Group({ direction }: { direction?: InternalControlGroupProps['direction'] }) { const [name, setName] = useState('service'); const [operator, setOperator] = useState(operators[0]); const [value, setValue] = useState('production'); return ( - + setName(e.detail.value)} /> ; @@ -78,8 +14,8 @@ describe('Control group', () => { const { getAllByTestId, rerender } = render({[alpha, beta]}); - // The ghost duplicates the children, so there are two "alpha" nodes; the first is the - // real (focusable) one, the second is in the inert ghost. + // In auto mode the group renders a hidden measurement duplicate of its children, so a + // test id can match more than once; the first match is the real (focusable) control. const alphaInput = getAllByTestId('alpha')[0]; alphaInput.focus(); expect(document.activeElement).toBe(alphaInput); @@ -93,8 +29,8 @@ describe('Control group', () => { }); describe('position', () => { - // The ghost duplicates each child, so every probe matches twice; the first match is - // the real control, the second is the inert ghost copy. + // In auto mode each child is duplicated for measurement, so every probe matches twice; + // the first match is the real control. test('exposes the "only" position to a single child control', () => { const { getAllByTestId } = render( @@ -135,9 +71,8 @@ describe('Control group', () => { }); describe('direction', () => { - // With the default `direction="auto"` and the jsdom harness (`availableWidth = 1000`, - // `requiredRowWidth = null`), the measurement falls back and `resolvedDirection` is - // `'horizontal'`, so the "defaults to horizontal" expectation holds unchanged. + // Without layout (jsdom), auto measurement never stacks, so the default resolves to + // horizontal. test('defaults the direction to "horizontal" and exposes it to each child', () => { const { getAllByTestId } = render( @@ -177,93 +112,3 @@ describe('Control group', () => { }); }); }); - -describe('Control group responsiveness', () => { - function renderGroup() { - return render( - - - - - ); - } - - function getRoot(container: HTMLElement) { - return container.querySelector(`.${styles.root}`)!; - } - - test('renders the hidden measurement ghost: aria-hidden and inert, horizontal by default', () => { - requiredRowWidth = 300; - availableWidth = 1000; - const { container } = renderGroup(); - - const ghost = container.querySelector(`.${styles.ghost}`) as HTMLElement; - expect(ghost).not.toBeNull(); - expect(ghost).toHaveAttribute('aria-hidden', 'true'); - // `inert` is set imperatively after mount so the duplicated controls are not focusable - // and not announced (no phantom tab stops). - expect(ghost.inert).toBe(true); - - // The group fits (available 1000 > required 300), so it resolves to horizontal. - expect(getRoot(container)).toHaveClass(styles['root-horizontal']); - expect(getRoot(container)).not.toHaveClass(styles['root-vertical']); - }); - - test('falls back to horizontal while measurements are null', () => { - // No required width measured yet. - requiredRowWidth = null; - availableWidth = 1000; - const { container } = renderGroup(); - expect(getRoot(container)).toHaveClass(styles['root-horizontal']); - expect(getRoot(container)).not.toHaveClass(styles['root-vertical']); - }); - - test('stacks (vertical) when the available width is below the required row width', () => { - requiredRowWidth = 500; - availableWidth = 200; - const { container } = renderGroup(); - expect(getRoot(container)).toHaveClass(styles['root-vertical']); - expect(getRoot(container)).not.toHaveClass(styles['root-horizontal']); - }); - - test('re-expands (horizontal) when the available width grows back above the required width', () => { - requiredRowWidth = 500; - availableWidth = 200; - const { container } = renderGroup(); - expect(getRoot(container)).toHaveClass(styles['root-vertical']); - - // Widen the available space and fire a resize so the ancestor-walk re-runs. This is - // the deadlock-safe re-expand: the walk re-reads the (now wider) ancestor width. - availableWidth = 1000; - act(() => { - fireResize(); - }); - expect(getRoot(container)).toHaveClass(styles['root-horizontal']); - expect(getRoot(container)).not.toHaveClass(styles['root-vertical']); - }); - - test('focus reaches only the real controls, not the inert ghost duplicates', () => { - requiredRowWidth = 300; - const { container } = renderGroup(); - const ghost = container.querySelector(`.${styles.ghost}`) as HTMLElement; - // The ghost subtree is inert, so its duplicated inputs are not in the tab order. - expect(ghost.inert).toBe(true); - expect(ghost.querySelectorAll('input')).toHaveLength(2); - }); - - test('a forced direction wins and bypasses measurement (no ghost rendered)', () => { - // Even with a tiny available width and a large required width, a forced horizontal - // direction stays horizontal and renders no measurement ghost. - requiredRowWidth = 500; - availableWidth = 50; - const { container } = render( - - - - - ); - expect(getRoot(container)).toHaveClass(styles['root-horizontal']); - expect(getRoot(container)).not.toHaveClass(styles['root-vertical']); - expect(container.querySelector(`.${styles.ghost}`)).toBeNull(); - }); -}); From c9b4733e5c87f7993274c3fc29966f1001295368 Mon Sep 17 00:00:00 2001 From: Joan Perals Tresserra Date: Tue, 6 Oct 2026 19:41:31 +0200 Subject: [PATCH 03/29] test: Assert ControlGroup ghost controls are not focusable Add a keyboard focus-order integ test that tabs from a sentinel before the auto group, through each real control (the Select is a single tab stop), to a sentinel after it. A focusable hidden measurement duplicate would add a phantom tab stop and divert this sequence, so the test fails if the ghost ever becomes keyboard-reachable. Bracket the auto group with focus sentinels on the dev page and address the real controls through the test-utils wrappers (the Select trigger is labelled via aria-labelledby, so it has no direct aria-label). --- pages/control-group/responsiveness.page.tsx | 9 +++++ .../__integ__/control-group.test.ts | 39 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/pages/control-group/responsiveness.page.tsx b/pages/control-group/responsiveness.page.tsx index 2a743cd525..b113a448df 100644 --- a/pages/control-group/responsiveness.page.tsx +++ b/pages/control-group/responsiveness.page.tsx @@ -3,6 +3,7 @@ import React, { useState } from 'react'; import Box from '~components/box'; +import Button from '~components/button'; import Input from '~components/input'; import ControlGroup, { InternalControlGroupProps } from '~components/internal/components/control-group'; import Select, { SelectProps } from '~components/select'; @@ -53,9 +54,17 @@ export default function ControlGroupResponsiveness() { Auto group + {/* + Focusable sentinels bracket the group so an integ test can tab from `focus-before` + through exactly the real controls to `focus-after`. If any hidden ghost duplicate + were keyboard-focusable, the group would absorb extra tab stops and focus would not + reach `focus-after` in the expected number of presses. + */} +
+ diff --git a/src/internal/components/control-group/__integ__/control-group.test.ts b/src/internal/components/control-group/__integ__/control-group.test.ts index 2277add75f..38463a4ac8 100644 --- a/src/internal/components/control-group/__integ__/control-group.test.ts +++ b/src/internal/components/control-group/__integ__/control-group.test.ts @@ -3,6 +3,16 @@ import { BasePageObject } from '@cloudscape-design/browser-test-tools/page-objects'; import useBrowser from '@cloudscape-design/browser-test-tools/use-browser'; +import createWrapper from '../../../../../lib/components/test-utils/selectors'; + +// Scope the finders to the plain auto group so the real controls are addressed through the +// component test-utils rather than raw attribute selectors (the Select trigger is labelled +// via `aria-labelledby`, not `aria-label`, so it has no direct `aria-label` to match). +const autoPlain = createWrapper('[data-testid="auto-plain"]'); +const firstInput = autoPlain.findInput('[aria-label="Label name"]').findNativeInput().toSelector(); +const selectTrigger = autoPlain.findSelect().findTrigger().toSelector(); +const lastInput = autoPlain.findInput('[aria-label="Label value"]').findNativeInput().toSelector(); + // The dev page lays out one control group per scenario, each wrapped in a `data-testid` // container. Each group holds three controls with these aria labels; the auto groups also // render a hidden measurement duplicate, which this test ignores by reading only the boxes @@ -139,4 +149,33 @@ describe('ControlGroup responsiveness', () => { await page.expectStacked('forced-vertical'); }) ); + + test( + 'keyboard focus flows through only the real controls, never a hidden measurement duplicate', + setupTest(async page => { + // The group renders a hidden duplicate of its controls to measure the row width. Those + // duplicates must not be keyboard-focusable, or they would add phantom tab stops. Tab + // from the button before the group: focus must visit each real control once (the Select + // is a single tab stop at its trigger) and then land on the button after the group — a + // total of (controls + 1) presses. An extra, hidden tab stop would divert focus and this + // sequence would fail. + await page.setWindowSize(WIDE); + await page.click('[data-testid="focus-before"]'); + await expect(page.isFocused('[data-testid="focus-before"]')).resolves.toBe(true); + + // Tab across the three real controls in order. The ghost duplicate's controls share the + // same markup, but it is inert and `visibility: hidden`, so focus can never rest on them. + await page.keys(['Tab']); + await expect(page.isFocused(firstInput)).resolves.toBe(true); + await page.keys(['Tab']); + await expect(page.isFocused(selectTrigger)).resolves.toBe(true); + await page.keys(['Tab']); + await expect(page.isFocused(lastInput)).resolves.toBe(true); + + // The next Tab leaves the group entirely, reaching the sentinel after it — proving no + // hidden duplicate sits between the last real control and the following focus target. + await page.keys(['Tab']); + await expect(page.isFocused('[data-testid="focus-after"]')).resolves.toBe(true); + }) + ); }); From 6dfd469b33dd113f04f86fa10cc3b3ed163c8761 Mon Sep 17 00:00:00 2001 From: Joan Perals Tresserra Date: Tue, 6 Oct 2026 19:56:49 +0200 Subject: [PATCH 04/29] chore: Simplify ControlGroup responsiveness comments --- pages/control-group/responsiveness.page.tsx | 19 ++----- src/input/__tests__/control-group.test.tsx | 5 +- .../__integ__/control-group.test.ts | 39 +++++--------- .../__tests__/control-group.test.tsx | 9 ++-- .../components/control-group/index.tsx | 54 ++++++------------- .../components/control-group/styles.scss | 16 ++---- .../__tests__/control-group.test.tsx | 3 +- 7 files changed, 44 insertions(+), 101 deletions(-) diff --git a/pages/control-group/responsiveness.page.tsx b/pages/control-group/responsiveness.page.tsx index b113a448df..d887566867 100644 --- a/pages/control-group/responsiveness.page.tsx +++ b/pages/control-group/responsiveness.page.tsx @@ -16,9 +16,7 @@ const operators: SelectProps.Option[] = [ { value: '!=', label: '!=' }, ]; -// The scenario containers are tied to the viewport width (`inlineSize: '100%'`) so that an -// integ test can drive the available width with `setWindowSize`. Narrowing the viewport below -// the group's required row width stacks all controls at once; widening it re-expands them. +// Full-width so an integ test can drive the available width with `setWindowSize`. const scenarioContainerStyle: React.CSSProperties = { inlineSize: '100%', padding: 16, @@ -54,12 +52,7 @@ export default function ControlGroupResponsiveness() { Auto group - {/* - Focusable sentinels bracket the group so an integ test can tab from `focus-before` - through exactly the real controls to `focus-after`. If any hidden ghost duplicate - were keyboard-focusable, the group would absorb extra tab stops and focus would not - reach `focus-after` in the expected number of presses. - */} + {/* Sentinels for the integ test to tab through the group and out the other side. */}
@@ -70,12 +63,8 @@ export default function ControlGroupResponsiveness() { Auto group inside SpaceBetween (flexbox deadlock) {/* - The group sits inside a horizontal SpaceBetween (a flex row) alongside another - element, itself inside the viewport-constrained container. A naive "measure my - parent" group would deadlock here: once stacked, the shrink-wrapping flex item - reports the collapsed width, so the group would never see the room to re-expand. - The ancestor-walk skips those shrink-wrapping ancestors, so widening the viewport - re-expands the group. + Inside a flex row, a naive "measure my parent" group would stay stuck stacked once + collapsed. The ancestor-walk skips the shrink-wrapping flex item, so it re-expands. */}
diff --git a/src/input/__tests__/control-group.test.tsx b/src/input/__tests__/control-group.test.tsx index a847ee0629..63457da270 100644 --- a/src/input/__tests__/control-group.test.tsx +++ b/src/input/__tests__/control-group.test.tsx @@ -10,9 +10,8 @@ import { PositionProbe } from '../../internal/components/control-group/__tests__ const noop = () => {}; describe('Input in control group', () => { - // The control group renders a hidden measurement ghost that duplicates its children, - // so prefix/suffix content (which renders immediately) appears twice. The first match - // is the real control's; the second is in the inert ghost. + // The measurement ghost duplicates the children, so prefix/suffix content matches twice; + // the first match is the real control's. test('resets the context for prefix content', () => { const { getAllByTestId } = render( diff --git a/src/internal/components/control-group/__integ__/control-group.test.ts b/src/internal/components/control-group/__integ__/control-group.test.ts index 38463a4ac8..133f3b4d94 100644 --- a/src/internal/components/control-group/__integ__/control-group.test.ts +++ b/src/internal/components/control-group/__integ__/control-group.test.ts @@ -5,25 +5,20 @@ import useBrowser from '@cloudscape-design/browser-test-tools/use-browser'; import createWrapper from '../../../../../lib/components/test-utils/selectors'; -// Scope the finders to the plain auto group so the real controls are addressed through the -// component test-utils rather than raw attribute selectors (the Select trigger is labelled -// via `aria-labelledby`, not `aria-label`, so it has no direct `aria-label` to match). +// Address the controls via test-utils rather than raw selectors (the Select trigger has no +// `aria-label` — it's labelled via `aria-labelledby`). const autoPlain = createWrapper('[data-testid="auto-plain"]'); const firstInput = autoPlain.findInput('[aria-label="Label name"]').findNativeInput().toSelector(); const selectTrigger = autoPlain.findSelect().findTrigger().toSelector(); const lastInput = autoPlain.findInput('[aria-label="Label value"]').findNativeInput().toSelector(); -// The dev page lays out one control group per scenario, each wrapped in a `data-testid` -// container. Each group holds three controls with these aria labels; the auto groups also -// render a hidden measurement duplicate, which this test ignores by reading only the boxes -// of visible (non-zero-height) controls. +// The three controls in each scenario group. const CONTROL_LABELS = ['Label name', 'Operator', 'Label value']; const WIDE = { width: 1200, height: 800 }; const NARROW = { width: 360, height: 800 }; -// Field boxes are bottom-aligned within a row (`align-items: flex-end`), so compare `top` -// with a small tolerance rather than exact equality; it also absorbs sub-pixel rounding. +// Tolerance for comparing control tops in a row (bottom-aligned fields, sub-pixel rounding). const ROW_TOP_TOLERANCE = 2; interface Box { @@ -32,9 +27,7 @@ interface Box { } class ControlGroupPage extends BasePageObject { - // Returns the bounding boxes of the real (visible) controls inside the given scenario - // wrapper, left-to-right in DOM order. The hidden measurement ghost is skipped because its - // controls have zero height. + // Boxes of the visible controls in the scenario, in DOM order. Skips the ghost duplicate. getVisibleControlBoxes(testId: string): Promise { return this.browser.execute( (id, labels) => { @@ -47,7 +40,7 @@ class ControlGroupPage extends BasePageObject { const elements = Array.from(container.querySelectorAll(`[aria-label="${label}"]`)); for (const element of elements) { const rect = element.getBoundingClientRect(); - // Skip the hidden ghost duplicate (collapsed to zero height, out of flow). + // The ghost duplicate has zero height; skip it. if (rect.height > 0) { boxes.push({ top: rect.top, left: rect.left }); break; @@ -65,11 +58,10 @@ class ControlGroupPage extends BasePageObject { await this.waitForAssertion(async () => { const boxes = await this.getVisibleControlBoxes(testId); expect(boxes).toHaveLength(CONTROL_LABELS.length); - // All controls share approximately the same top (one row)... + // Same top, increasing left: one row, left-to-right. for (const box of boxes) { expect(Math.abs(box.top - boxes[0].top)).toBeLessThanOrEqual(ROW_TOP_TOLERANCE); } - // ...and are placed left-to-right. for (let i = 1; i < boxes.length; i++) { expect(boxes[i].left).toBeGreaterThan(boxes[i - 1].left); } @@ -131,8 +123,7 @@ describe('ControlGroup responsiveness', () => { await page.expectRow('auto-spacebetween'); await page.setWindowSize(NARROW); await page.expectStacked('auto-spacebetween'); - // The critical case: widening the viewport must break the parent/child deadlock and - // let the group re-expand, instead of staying stuck stacked. + // The critical case: widening must re-expand it, not leave it stuck stacked. await page.setWindowSize(WIDE); await page.expectRow('auto-spacebetween'); }) @@ -153,18 +144,13 @@ describe('ControlGroup responsiveness', () => { test( 'keyboard focus flows through only the real controls, never a hidden measurement duplicate', setupTest(async page => { - // The group renders a hidden duplicate of its controls to measure the row width. Those - // duplicates must not be keyboard-focusable, or they would add phantom tab stops. Tab - // from the button before the group: focus must visit each real control once (the Select - // is a single tab stop at its trigger) and then land on the button after the group — a - // total of (controls + 1) presses. An extra, hidden tab stop would divert focus and this - // sequence would fail. + // Tabbing from the sentinel before the group must reach each real control then the + // sentinel after it. A focusable ghost duplicate would add a tab stop and break this. await page.setWindowSize(WIDE); await page.click('[data-testid="focus-before"]'); await expect(page.isFocused('[data-testid="focus-before"]')).resolves.toBe(true); - // Tab across the three real controls in order. The ghost duplicate's controls share the - // same markup, but it is inert and `visibility: hidden`, so focus can never rest on them. + // Tab across the three real controls in order. await page.keys(['Tab']); await expect(page.isFocused(firstInput)).resolves.toBe(true); await page.keys(['Tab']); @@ -172,8 +158,7 @@ describe('ControlGroup responsiveness', () => { await page.keys(['Tab']); await expect(page.isFocused(lastInput)).resolves.toBe(true); - // The next Tab leaves the group entirely, reaching the sentinel after it — proving no - // hidden duplicate sits between the last real control and the following focus target. + // The next Tab leaves the group for the sentinel after it. await page.keys(['Tab']); await expect(page.isFocused('[data-testid="focus-after"]')).resolves.toBe(true); }) diff --git a/src/internal/components/control-group/__tests__/control-group.test.tsx b/src/internal/components/control-group/__tests__/control-group.test.tsx index 45b458cf94..e2b30da653 100644 --- a/src/internal/components/control-group/__tests__/control-group.test.tsx +++ b/src/internal/components/control-group/__tests__/control-group.test.tsx @@ -14,8 +14,7 @@ describe('Control group', () => { const { getAllByTestId, rerender } = render({[alpha, beta]}); - // In auto mode the group renders a hidden measurement duplicate of its children, so a - // test id can match more than once; the first match is the real (focusable) control. + // The measurement duplicate matches the test id too; the first match is the real control. const alphaInput = getAllByTestId('alpha')[0]; alphaInput.focus(); expect(document.activeElement).toBe(alphaInput); @@ -29,8 +28,7 @@ describe('Control group', () => { }); describe('position', () => { - // In auto mode each child is duplicated for measurement, so every probe matches twice; - // the first match is the real control. + // Each probe matches twice (real + measurement duplicate); the first match is the real one. test('exposes the "only" position to a single child control', () => { const { getAllByTestId } = render( @@ -71,8 +69,7 @@ describe('Control group', () => { }); describe('direction', () => { - // Without layout (jsdom), auto measurement never stacks, so the default resolves to - // horizontal. + // In jsdom there's no layout, so auto never stacks and defaults to horizontal. test('defaults the direction to "horizontal" and exposes it to each child', () => { const { getAllByTestId } = render( diff --git a/src/internal/components/control-group/index.tsx b/src/internal/components/control-group/index.tsx index a3fa991716..0a6f7959bd 100644 --- a/src/internal/components/control-group/index.tsx +++ b/src/internal/components/control-group/index.tsx @@ -17,9 +17,8 @@ import { flattenChildren } from '../../utils/flatten-children'; import styles from './styles.css.js'; -// The prop widens the shared two-value `GroupedControlDirection` with `'auto'`, which only -// exists at the component boundary: `'auto'` measures and resolves to a concrete axis before -// anything reaches context, classes, or SCSS. +// `'auto'` exists only at the prop boundary; it resolves to a concrete axis before reaching +// context, classes, or SCSS. type ControlGroupDirection = GroupedControlDirection | 'auto'; export interface InternalControlGroupProps extends BaseComponentProps { @@ -40,20 +39,14 @@ const InternalControlGroup = forwardRef