Skip to content

Fix KeybindingHint key display on non-MacOS/Windows platforms - #7908

Merged
jonrohan merged 4 commits into
mainfrom
fix-keybinding-hint-platform-keys
Jun 1, 2026
Merged

Fix KeybindingHint key display on non-MacOS/Windows platforms#7908
jonrohan merged 4 commits into
mainfrom
fix-keybinding-hint-platform-keys

Conversation

@jonrohan

@jonrohanjonrohan commented May 29, 2026

Copy link
Copy Markdown
Member

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

CleanShot 2026-05-29 at 16 17 42@2xCleanShot 2026-05-29 at 16 19 01@2x

KeybindingHint previously only distinguished between MacOS and "not MacOS" when displaying the Meta, Alt, and Mod keys. As a result, the Meta key was rendered as Win (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

  • Added an SSR-safe usePlatform() hook (internal to KeybindingHint) that returns 'mac' | 'windows' | 'other'. Apple platforms (macOS and iOS/iPadOS) map to 'mac'.

Changed

  • KeybindingHint now resolves platform-specific keys based on the detected platform:
    • Meta: /Command on Apple, Win/Windows on Windows, Meta on all other platforms.
    • Alt: /Option on Apple, Alt elsewhere.
    • Mod: /Command on Apple, /Control elsewhere.
  • getAccessibleKeybindingHintString remains backward compatible — it still accepts the previous boolean argument (true'mac', false'other') in addition to a Platform value.

Removed

  • N/A

Rollout strategy

  • Patch release

Testing & Reviewing

Unit tests were added covering Meta, Alt, and Mod rendering across all three platform categories. Existing KeybindingHint, Tooltip, and TreeView tests pass.

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Added/updated previews (Storybook)
  • Changes are SSR compatible
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge
  • (GitHub staff only) Integration tests pass at github/github-ui

@changeset-bot

changeset-botBot commented May 29, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: a1789a3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
NameType
@primer/reactPatch

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-actionsgithub-actionsBot added staff Author is a staff member integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels May 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Action required

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Check the integration testing docs for step-by-step instructions. Or, apply the integration-tests: skipped manually label to skip these checks.

To publish a canary release for integration testing, apply the Canary Release label to this PR.

@jonrohan
jonrohan marked this pull request as ready for review May 29, 2026 23:05
@jonrohan
jonrohan requested a review from a team as a code ownerMay 29, 2026 23:05
@jonrohan
jonrohan requested review from TylerJDev and CopilotMay 29, 2026 23:05

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, and TreeView to use platform-aware key naming.
  • Added accessible-string tests and a Storybook matrix for platform-specific modifier rendering.
Show a summary per file
FileDescription
packages/react/src/TreeView/TreeView.tsxUses the new platform hook for TreeView shortcut labels.
packages/react/src/TooltipV2/Tooltip.tsxUses platform-aware accessible shortcut descriptions.
packages/react/src/KeybindingHint/platform.tsAdds platform detection and override context.
packages/react/src/KeybindingHint/KeybindingHint.tsxUpdates accessible helper to accept platform values.
packages/react/src/KeybindingHint/KeybindingHint.test.tsxAdds accessible-name coverage for platform-specific keys.
packages/react/src/KeybindingHint/KeybindingHint.features.stories.tsxAdds a platform comparison Storybook story.
packages/react/src/KeybindingHint/KeybindingHint.features.stories.module.cssStyles the new platform comparison table.
packages/react/src/KeybindingHint/key-names.tsImplements platform-specific key-name mappings.
packages/react/src/KeybindingHint/components/Sequence.tsxThreads platform values through accessible sequence strings.
packages/react/src/KeybindingHint/components/Key.tsxRenders keys using detected platform mappings.
packages/react/src/KeybindingHint/components/Chord.tsxThreads platform values through accessible chord strings.
.changeset/keybinding-hint-platform-keys.mdAdds 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',
@github-actions

github-actionsBot commented May 29, 2026

Copy link
Copy Markdown
Contributor

Uh oh! @jonrohan, at least one image you shared is missing helpful alt text. Check your pull request body to fix the following violations:

  • Images should have meaningful alternative text (alt text) at line 5
  • Images should have meaningful alternative text (alt text) at line 7

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.

🤖 Beep boop! This comment was added automatically by github/accessibility-alt-text-bot.

@jonrohan
jonrohan requested a review from iansan5653May 29, 2026 23:18
@jonrohanjonrohan added the Canary Release Apply this label when you want CI to create a canary release of the current PR label May 29, 2026
@primer-integration

Copy link
Copy Markdown

👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/21980

@primer-integration

Copy link
Copy Markdown

Integration test results from github/github-ui:

Passed CI  Passed
Running VRT  Running
Passed Projects  Passed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useIsMacOS is a common, shared utility hook. Should usePlatform live in the same place? Or maybe we should just update that hook to be more powerful? Seems like this could be useful for other components.

* `'other'`.
*/
export const getAccessibleKeybindingHintString = accessibleSequenceString
export const getAccessibleKeybindingHintString = (sequence: string, platform: Platform | boolean) =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A followup task after release should be to update consumers in github/github to use the new Platform type instead of boolean

Comment threadpackages/react/src/KeybindingHint/platform.ts Outdated

@TylerJDevTylerJDev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! +1 to Ian's comments, though approving ahead of time.

@jonrohanjonrohan added the integration-tests: skipped manually Changes in this PR do not require an integration test label Jun 1, 2026
@jonrohan
jonrohan added this pull request to the merge queueJun 1, 2026
@github-actions
github-actionsBottemporarily deployed to storybook-preview-7908 June 1, 2026 21:27 Inactive
Merged via the queue into main with commit e9a2254Jun 1, 2026
59 of 60 checks passed
@jonrohan
jonrohan deleted the fix-keybinding-hint-platform-keys branch June 1, 2026 21:34
@primerprimerBot mentioned this pull request Jun 1, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Canary ReleaseApply this label when you want CI to create a canary release of the current PRintegration-tests: recommendedThis change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpmintegration-tests: skipped manuallyChanges in this PR do not require an integration teststaffAuthor is a staff member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

KeybindingHint only considers MacOS and Windows when displaying key names

4 participants

@jonrohan@iansan5653@TylerJDev