[scanner] fix: add keyboard navigation to dropdown menus#19414
Conversation
Adds arrow key navigation, Enter/Space activation, and Escape to close for dropdown menus in PayloadCard and UpdateSettingsForm components using the existing useDropdownKeyNav hook. - PayloadCard: Replaced manual Escape handler with useDropdownKeyNav - UpdateSettingsForm: Added keyboard nav to channel selection dropdown Fixes #19410 Signed-off-by: GitHub Actions Bot <actions@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: GitHub Actions Bot <actions@github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
|
🐝 Hi @clubanderson! I'm Trusted users — org members and contributors with write access — can mention Automation may take a moment to start, and follow-up happens through workflow activity rather than chat replies. |
✅ Test Coverage CheckAll new source files in this PR have corresponding test files. Checked |
Auto Test GeneratorThe following new files have no corresponding test file:
Please add tests or apply the |
There was a problem hiding this comment.
Pull request overview
This PR addresses the keyboard navigation gaps flagged by the scanner for two high-priority dropdowns by wiring them to the shared useDropdownKeyNav hook, aiming to support Arrow key navigation and Escape-to-close behavior.
Changes:
- Updated PayloadCard priority dropdown to use
useDropdownKeyNavinstead of a document-level Escape listener. - Updated UpdateSettingsForm channel dropdown to add a
useDropdownKeyNavkeydown handler to the listbox container.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
web/src/components/mission-control/PayloadCard.tsx |
Replaces manual Escape handling with useDropdownKeyNav and adds menu semantics for the priority dropdown. |
web/src/components/settings/UpdateSettingsForm.tsx |
Adds useDropdownKeyNav handling for the update channel dropdown listbox. |
Comments suppressed due to low confidence (1)
web/src/components/settings/UpdateSettingsForm.tsx:203
- The channel dropdown uses
role="listbox", but its items are plain buttons withoutrole="option"/aria-selected, which makes the listbox semantics incomplete for screen readers.
Also, because keyboard handling is attached to the listbox, focus should move into the list when it opens; otherwise Arrow/Escape won’t typically hit onKeyDown (focus stays on the trigger). Autofocusing the currently-selected option is a simple way to make the new key handler actually reachable.
<div role="listbox" aria-labelledby="updates-channel-label" onKeyDown={channelDropdownKeyNav} className="absolute z-dropdown mt-2 w-full rounded-lg bg-card border border-border shadow-xl">
{visibleChannels.map((option) => (
<button
key={option.value}
onClick={() => handleSelectChannel(option.value)}
| const priorityBtnRef = useRef<HTMLButtonElement>(null) | ||
|
|
||
| // issue 6743 — Dismiss the priority dropdown when the user presses Escape. Without | ||
| // this, keyboard users had no way to close the menu once opened. | ||
| useEffect(() => { | ||
| if (!showPriorityMenu) return | ||
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| if (e.key === 'Escape') { | ||
| e.stopPropagation() | ||
| setShowPriorityMenu(false) | ||
| } | ||
| } | ||
| document.addEventListener('keydown', handleKeyDown) | ||
| return () => document.removeEventListener('keydown', handleKeyDown) | ||
| }, [showPriorityMenu]) | ||
| const priorityMenuKeyNav = useDropdownKeyNav(() => setShowPriorityMenu(false)) |
| @@ -276,6 +264,8 @@ export function PayloadCard({ project, onRemove, onUpdatePriority, onHover, onCl | |||
| <div className="fixed inset-0 z-[9998]" role="presentation" aria-hidden="true" onClick={() => setShowPriorityMenu(false)} /> | |||
| role="menu" | ||
| onKeyDown={priorityMenuKeyNav} |
| ] | ||
| const failCount = prereqChecks.filter((check) => !check).length | ||
|
|
||
| const channelDropdownKeyNav = useDropdownKeyNav(channelDropdown.close) |
| </button> | ||
| {channelDropdown.isOpen && ( | ||
| <div role="listbox" aria-labelledby="updates-channel-label" className="absolute z-dropdown mt-2 w-full rounded-lg bg-card border border-border shadow-xl"> | ||
| <div role="listbox" aria-labelledby="updates-channel-label" onKeyDown={channelDropdownKeyNav} className="absolute z-dropdown mt-2 w-full rounded-lg bg-card border border-border shadow-xl"> |
| document.addEventListener('keydown', handleKeyDown) | ||
| return () => document.removeEventListener('keydown', handleKeyDown) | ||
| }, [showPriorityMenu]) | ||
| const priorityMenuKeyNav = useDropdownKeyNav(() => setShowPriorityMenu(false)) |
Signed-off-by: clubanderson <club.anderson@gmail.com>
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
✅ Post-Merge Verification: passedCommit: |
|
Post-merge build verification passed ✅ Both Go and frontend builds compiled successfully against merge commit |
Fixes #19410
Adds arrow key navigation, Enter/Space activation, and Escape to close for dropdown menus in 2 high-priority components using the existing
useDropdownKeyNavhook.Changes
useDropdownKeyNavfor priority dropdown menuImplementation
Both components now use the
useDropdownKeyNavhook which provides:Notes
This is a partial fix addressing the 2 highest-priority dropdown components with actual keyboard navigation gaps. The other 3 files in the issue either have no dropdown menus (DashboardGrid, MiniDashboard) or use filter buttons rather than dropdown menus (Marketplace).
Build and lint validation will be handled by CI.