fix(dropdown-select): don't steal focus outline when dropdown is closed#2492
Open
er-asm wants to merge 2 commits into
Open
fix(dropdown-select): don't steal focus outline when dropdown is closed#2492er-asm wants to merge 2 commits into
er-asm wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an accessibility regression in scale-dropdown-select where the steal-focus CSS part was applied on initial render for pre-selected (non-first) values, suppressing the visible focus outline when tabbing to the combobox (issue #2428).
Changes:
- Stop deriving
currentIndexfromvaluechanges on initial render / while closed; managecurrentIndexviasetOpen()and interaction handlers instead. - Add unit tests asserting
steal-focusis not present while the dropdown is closed, regardless of the initially selected item. - Re-enable dropdown-select visual tests and update visual-test infrastructure to bind the Storybook server to
0.0.0.0(Docker reachability), with new snapshot baselines.
Reviewed changes
Copilot reviewed 4 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/components/src/components/dropdown-select/dropdown-select.tsx | Adjusts currentIndex lifecycle so steal-focus is not applied while closed; initializes index on open and resets on close. |
| packages/components/src/components/dropdown-select/dropdown-select.spec.ts | Adds unit tests covering the missing-outline regression by asserting steal-focus is absent while closed. |
| packages/visual-tests/src/dropdown-select.visual.spec.js | Un-skips the dropdown-select visual test suite. |
| packages/visual-tests/setup.js | Ensures the visual-test Storybook server binds to 0.0.0.0 for Docker access and awaits server readiness. |
| packages/visual-tests/src/image_snapshots/dropdown-select-*.png | Adds/updates snapshot baselines for the re-enabled visual tests across themes and states. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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
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.
Problem
When
scale-dropdown-selectwas initialized with a pre-selected value that was not the first item (e.g.value="cedric"withcasparbeing first), thesteal-focusCSS part was incorrectly applied to the wrapper element on load. This suppressed the focus outline (outline: var(--focus-outline)) when the user tabbed to the combobox, breaking keyboard accessibility.Fixes #2428
Root cause
currentIndexwas being set whenever thevalueprop changed (including on initial render via@Watch('value')). IngetBasePartMap(),currentIndex > -1adds thesteal-focuspart, which CSS uses to suppress the focus outline. SincecurrentIndexequalled the index of the pre-selected item while the dropdown was closed, the outline was always suppressed.Solution
currentIndexis no longer set whenvaluechanges via@Watchwhile the dropdown is closedsetOpen(true)initializescurrentIndexto the selected item's index (correct starting point for keyboard navigation)setOpen(false)resetscurrentIndexto-1(removingsteal-focuswhen closed)connectedCallback()intentionally left without settingcurrentIndexChanges
packages/components/src/components/dropdown-select/dropdown-select.tsx— fix logicpackages/components/src/components/dropdown-select/dropdown-select.spec.ts— 3 new unit tests covering issue scale-dropdown-select: no outline on focus #2428packages/visual-tests/src/dropdown-select.visual.spec.js— un-skipped visual test suitepackages/visual-tests/src/__image_snapshots__/dropdown-select-*— 16 new visual snapshots (light/dark × standard/disabled/error)packages/visual-tests/setup.js— fix Express server to bind on0.0.0.0so Docker container can reach storybookTesting
yarn workspace @telekom/scale-components test --spec)yarn workspace @telekom/scale-visual-tests jest --forceExit -u)cc @maomaoZH @amir-ba