Fix KeybindingHint key display on non-MacOS/Windows platforms#7908
Fix KeybindingHint key display on non-MacOS/Windows platforms#7908jonrohan wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: 4cb7c0e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
There was a problem hiding this comment.
Pull request overview
This PR updates KeybindingHint platform handling so modifier keys are displayed appropriately on Apple, Windows, and other platforms, addressing Linux/Android Meta key labeling.
Changes:
- Added internal tri-state platform detection and platform override support for Storybook previews.
- Updated
KeybindingHint,TooltipV2, andTreeViewto use platform-aware key naming. - Added accessible-string tests and a Storybook matrix for platform-specific modifier rendering.
Show a summary per file
| File | Description |
|---|---|
packages/react/src/TreeView/TreeView.tsx |
Uses the new platform hook for TreeView shortcut labels. |
packages/react/src/TooltipV2/Tooltip.tsx |
Uses platform-aware accessible shortcut descriptions. |
packages/react/src/KeybindingHint/platform.ts |
Adds platform detection and override context. |
packages/react/src/KeybindingHint/KeybindingHint.tsx |
Updates accessible helper to accept platform values. |
packages/react/src/KeybindingHint/KeybindingHint.test.tsx |
Adds accessible-name coverage for platform-specific keys. |
packages/react/src/KeybindingHint/KeybindingHint.features.stories.tsx |
Adds a platform comparison Storybook story. |
packages/react/src/KeybindingHint/KeybindingHint.features.stories.module.css |
Styles the new platform comparison table. |
packages/react/src/KeybindingHint/key-names.ts |
Implements platform-specific key-name mappings. |
packages/react/src/KeybindingHint/components/Sequence.tsx |
Threads platform values through accessible sequence strings. |
packages/react/src/KeybindingHint/components/Key.tsx |
Renders keys using detected platform mappings. |
packages/react/src/KeybindingHint/components/Chord.tsx |
Threads platform values through accessible chord strings. |
.changeset/keybinding-hint-platform-keys.md |
Adds a patch changeset for the behavior fix. |
Copilot's findings
- Files reviewed: 12/12 changed files
- Comments generated: 1
| shift: '⇧', | ||
| meta: isMacOS ? '⌘' : 'Win', | ||
| mod: isMacOS ? '⌘' : '⌃', | ||
| meta: platform === 'mac' ? '⌘' : platform === 'windows' ? 'Win' : 'Meta', |
|
Uh oh! @jonrohan, at least one image you shared is missing helpful alt text. Check your pull request body to fix the following violations:
Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image. Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.
|
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/21980 |
Closes #7756
Here's a preview of all the modifier keys on different platforms https://primer-11d74e8128-13348165.drafts.github.io/storybook/?path=/story/experimental-components-keybindinghint-features--platforms
KeybindingHintpreviously only distinguished between MacOS and "not MacOS" when displaying theMeta,Alt, andModkeys. As a result, theMetakey was rendered asWin(Windows key) on Linux, Android, and other platforms where that key does not exist.This change introduces a three-way platform detection (
mac/windows/other) so keys are displayed correctly per platform.Changelog
New
usePlatform()hook (internal toKeybindingHint) that returns'mac' | 'windows' | 'other'. Apple platforms (macOS and iOS/iPadOS) map to'mac'.Changed
KeybindingHintnow resolves platform-specific keys based on the detected platform:Meta:⌘/Commandon Apple,Win/Windowson Windows,Metaon all other platforms.Alt:⌥/Optionon Apple,Altelsewhere.Mod:⌘/Commandon Apple,⌃/Controlelsewhere.getAccessibleKeybindingHintStringremains backward compatible — it still accepts the previousbooleanargument (true→'mac',false→'other') in addition to aPlatformvalue.Removed
Rollout strategy
Testing & Reviewing
Unit tests were added covering
Meta,Alt, andModrendering across all three platform categories. ExistingKeybindingHint,Tooltip, andTreeViewtests pass.Merge checklist