fix(SelectPanel): do not bubble up keyboard events #6900
Conversation
Co-authored-by: Tyler Jones <[email protected]>
🦋 Changeset detectedLatest commit: b34a87e The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
the
integration-tests: recommended
|
👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks! |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes keyboard event bubbling issues in the SelectPanel component by preventing typeahead keyboard events from propagating outside the component. The fix addresses unwanted behavior where alphabet key presses in the SelectPanel would bubble up to parent components.
- Extract the
isAlphabetKeyutility function fromuseMnemonicshook for reuse - Add keyboard event handling to SelectPanel's overlay to prevent event bubbling for typeahead keys
- Preserve modifier key shortcuts and skip prevention when text inputs are focused
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/react/src/hooks/useMnemonics.ts | Exports isAlphabetKey utility function for shared use |
| packages/react/src/SelectPanel/SelectPanel.tsx | Adds keyboard event handler to prevent bubbling of typeahead events |
github-actions
bot
requested a deployment
to
storybook-preview-6900
…eact into fix/select-panel-global-actions
github-actions
bot
added
integration-tests: failing
github-actions
bot
added
integration-tests: passing
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/3155 |
|
🟢 ci completed with status |
| } | ||
| : {}), | ||
| } as React.CSSProperties, | ||
| onKeyDown: preventBubbling(overlayProps?.onKeyDown), |
There was a problem hiding this comment.
Should we feature flag this to only exist if the primer_react_select_panel_remove_active_descendant flag is activated?
There was a problem hiding this comment.
We definitely can, I thought you said this was reproducible without it though so I thought it my be beneficial to keep it in regardless? 👀
There was a problem hiding this comment.
To be honest, it would let us test it a bit more when staffshipped alongside the FF. I don't anticipate there being any issues with preventing events from bubbling, but I'm curious if anything would come up 🤔
| customOnKeyDown?.(event) | ||
|
|
||
| const activeElement = document.activeElement as HTMLElement | ||
| if (activeElement.tagName === 'INPUT' || activeElement.tagName === 'TEXTAREA') return |
Relates to https://github.com/github/primer/issues/5882
Changelog
New
Rollout strategy
Testing & Reviewing
Merge checklist