fix(dropdown): keep focus on trigger with single-component keyboard navigation#14079
fix(dropdown): keep focus on trigger with single-component keyboard navigation#14079
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates calcite-dropdown keyboard/focus behavior to keep focus on the trigger and represent item navigation via an active-descendant style state, aligning with the “single component” focus model requested in #8206.
Changes:
- Refactors dropdown keyboard handling to track an internal active item index and activate items without moving DOM focus into dropdown items.
- Adds an
activeDescendantstate tocalcite-dropdown-item(with styling) and wires upariaActiveDescendantElementfor the menu. - Updates E2E and browser tests to assert active-descendant behavior instead of
document.activeElementmoving to items.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/components/src/components/dropdown/resources.ts | Removes internal ID helpers previously used for menu/button relationships. |
| packages/components/src/components/dropdown/interfaces.ts | Removes the internal keyboard event detail type no longer used. |
| packages/components/src/components/dropdown/dropdown.tsx | Implements active-descendant navigation and keeps focus on the trigger; adds hover focus-in/out handlers. |
| packages/components/src/components/dropdown/dropdown.e2e.ts | Updates focus assertions to check active-descendant rather than focused item. |
| packages/components/src/components/dropdown/dropdown.browser.e2e.tsx | Updates browser-level tests, including hover behavior expectations. |
| packages/components/src/components/dropdown-item/dropdown-item.tsx | Adds activeDescendant and activateItem(), removes item-level keydown handling, sets item tabIndex to -1. |
| packages/components/src/components/dropdown-item/dropdown-item.scss | Applies focus styling when active-descendant is set. |
| packages/components/src/components/dropdown-item/dropdown-item.e2e.ts | Removes key-activation coverage that depended on item receiving focus. |
| packages/components/src/components/dropdown-item/dropdown-item.browser.e2e.tsx | Replaces generic disabled tests with event-prevention assertions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
💡 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>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
packages/components/src/components/dropdown-item/dropdown-item.tsx:169
calcite-dropdown-itemno longer listens for keyboard activation when it has focus (thekeydownhandler was removed). Since the host can still receive focus programmatically (and sometimes via pointer interaction even withtabIndex=-1), Enter/Space will no longer activate/select the item, which is a regression from prior behavior and can hurt accessibility in edge cases. Consider keeping minimal key handling for Enter/Space (and possibly Escape) when the item itself is focused, even if arrow-key navigation remains managed bycalcite-dropdown.
constructor() {
super();
this.listen("click", this.onClick);
this.listenOn<CustomEvent>(
document.body,
"calciteInternalDropdownItemChange",
this.updateActiveItemOnChange,
);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Related Issue: #8206
Summary
This PR updates
calcite-dropdownkeyboard/focus behavior to keep focus on the trigger and represent item navigation via an active-descendant style state, aligning with the “single component” focus model requested in #8206.Changes:
activeDescendantstate tocalcite-dropdown-item(with styling) and wires upariaActiveDescendantElementfor the menu.document.activeElementmoving to items.