Repository navigation
Conversation
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.
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.
jperals
changed the title
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).
Reduce the responsiveness dev page to the SpaceBetween deadlock scenario, reuse the shared FocusTarget for the focus-order integ test, and add an Auto option to the permutations page direction setting.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5111 +/- ##
=======================================
Coverage 97.70% 97.71%
=======================================
Files 990 991 +1
Lines 31818 31871 +53
Branches 11752 11767 +15
=======================================
+ Hits 31089 31142 +53
Misses 722 722
Partials 7 7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…#5020) Co-authored-by: Andrei Zhaleznichenka <zhalea@amazon.de> Co-authored-by: Maximilian Schöll <20025802+mxschll@users.noreply.github.com> Co-authored-by: Maximilian Schoell <mxschll@users.noreply.github.com> Co-authored-by: Vahab Valiyev <43747437+vvaliyev@users.noreply.github.com> Co-authored-by: Gethin Webster <gethinw@amazon.de> Co-authored-by: Amanuel Abiy <amanabiy@amazon.de> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: revezard <137924264+revezard@users.noreply.github.com> Co-authored-by: at-susie <ienakaai@amazon.de> Co-authored-by: Tanner <101598007+tannerbachelor@users.noreply.github.com> Co-authored-by: test <test@test> Co-authored-by: Harsh Anand Singh <130173825+Harsh-Anand-Singh@users.noreply.github.com> Co-authored-by: Ajay Dharnappa Poojary <ajsuvarna6@gmail.com> Co-authored-by: Ajay Dharnappa Poojary <ajdp@amazon.com> Co-authored-by: Nathnael_D <109980176+NathanZlion@users.noreply.github.com> Co-authored-by: Nathnael Dereje <natidere@amazon.com> Co-authored-by: Ernst Kaese <ernstka@amazon.de>
Add a minimal internal ControlGroupWrapper exposing findControls, which returns only the real controls — the direct-child selector excludes the hidden measurement duplicate. Use it in the integ test instead of filtering the ghost out by bounding-box height.
Resolve each control through findControls().get(index) so the input and select finders can't match the ghost's duplicated aria-labels.
Avoid conflicts with other branches touching the permutations page.
Move the ghost measurement, ancestor-walk, ResizeObserver, and the stacked decision out of index.tsx into useResponsiveDirection, so the component is rendering-only and the measurement logic has a reusable home.
Move the ghost measurement from control-group into a shared src/internal/hooks/use-fits-inline hook. It returns a boolean overflows flag (no GroupedControlDirection coupling) and takes the available-width walk as an overridable function, defaulting to the constraining-ancestor walk. ControlGroup translates overflows into its direction locally.
Add a reusable measurement-ghost mixin (out of flow, hidden but measurable, non-interactive) in internal styles utils. ControlGroup's .ghost includes it and keeps only the single-row layout it measures.
Make the required single-row width deterministic (~3 x 200px) so the group reliably stacks at the narrow viewport and is a row at the wide one, instead of depending on default control sizing.
This reverts commit 27e439f.
Add control-group/test-classes/styles.scss with root and control classes, apply them in the component, and point the test-utils wrapper at them instead of the implementation styles, following the repo test-classes convention.
Stacking is decided asynchronously (ResizeObserver -> state -> relayout), so assert only after waitForJsTimers following each setWindowSize, matching the cards responsive integ test. Fixes the control not appearing stacked.
The flex-shrink:0 group propped the auto-width container (and the whole content column) open to its own min-content, so setWindowSize never narrowed it. Use a viewport-relative width with overflow:hidden so the container tracks the window and stacks at the narrow size.
Give each control a fixed 250px width (~750px required row) so the group can't fit even at the browser's minimum window width. Drop the resize settle helper and the viewport-width container workaround.
Use six controls (no wrapper divs) so the group's required single-row width exceeds the browser's minimum window width and it reliably stacks at the narrow viewport. Update the integ test count and tab sequence.
Unit-test measureConstrainingAncestorWidth with a stubbed ancestor chain, exercising both walk exits (an ancestor wider than the content, and one that clips/scrolls) plus the shrink-wrap skip path.
Wrap the scenario in an overflow:hidden full-width container so the flex-shrink:0 group can't prop it open; its width then tracks the viewport deterministically and the group stacks/re-expands regardless of CI scrollbar differences.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Related links, issue #, if available: n/a
How has this been tested?
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.