Skip to content

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

Open
jonrohan wants to merge 2 commits into
mainfrom
fix-keybinding-hint-platform-keys
Open

Fix KeybindingHint key display on non-MacOS/Windows platforms#7908
jonrohan wants to merge 2 commits into
mainfrom
fix-keybinding-hint-platform-keys

Conversation

@jonrohan
Copy link
Copy Markdown
Member

@jonrohan jonrohan commented May 29, 2026

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@2x CleanShot 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
Copy link
Copy Markdown

changeset-bot Bot commented May 29, 2026

🦋 Changeset detected

Latest commit: 4cb7c0e

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

This PR includes changesets to release 1 package
Name Type
@primer/react Patch

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 github-actions Bot 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 owner May 29, 2026 23:05
@jonrohan jonrohan requested review from TylerJDev and Copilot May 29, 2026 23:05
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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
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',
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 29, 2026

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 iansan5653 May 29, 2026 23:18
@jonrohan jonrohan 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Canary Release Apply this label when you want CI to create a canary release of the current PR integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm staff Author 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

2 participants