From 7a7da0047d1920868ef95ce93663f24baa5bc29a Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Fri, 29 May 2026 22:29:49 +0000 Subject: [PATCH 1/2] Fix KeybindingHint key display on non-MacOS/Windows platforms --- .changeset/keybinding-hint-platform-keys.md | 5 ++ .../KeybindingHint/KeybindingHint.test.tsx | 23 +++++++++ .../src/KeybindingHint/KeybindingHint.tsx | 8 +++- .../src/KeybindingHint/components/Chord.tsx | 5 +- .../src/KeybindingHint/components/Key.tsx | 8 ++-- .../KeybindingHint/components/Sequence.tsx | 5 +- .../react/src/KeybindingHint/key-names.ts | 25 +++++----- packages/react/src/KeybindingHint/platform.ts | 48 +++++++++++++++++++ packages/react/src/TooltipV2/Tooltip.tsx | 7 +-- packages/react/src/TreeView/TreeView.tsx | 8 ++-- 10 files changed, 115 insertions(+), 27 deletions(-) create mode 100644 .changeset/keybinding-hint-platform-keys.md create mode 100644 packages/react/src/KeybindingHint/platform.ts diff --git a/.changeset/keybinding-hint-platform-keys.md b/.changeset/keybinding-hint-platform-keys.md new file mode 100644 index 00000000000..e8fb56afe8d --- /dev/null +++ b/.changeset/keybinding-hint-platform-keys.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +`KeybindingHint`: display the `Meta` key correctly on platforms other than macOS and Windows. The `Meta`, `Alt`, and `Mod` keys are now resolved based on the detected platform: Apple platforms (macOS and iOS) show `⌘`/`⌥`, Windows shows `Win`, and all other platforms show `Meta`/`Alt`. diff --git a/packages/react/src/KeybindingHint/KeybindingHint.test.tsx b/packages/react/src/KeybindingHint/KeybindingHint.test.tsx index 65af945c564..e9f06089a26 100644 --- a/packages/react/src/KeybindingHint/KeybindingHint.test.tsx +++ b/packages/react/src/KeybindingHint/KeybindingHint.test.tsx @@ -107,4 +107,27 @@ describe('getAccessibleKeybindingHintString', () => { it('returns "control" for "mod" on non-MacOS', () => expect(getAccessibleKeybindingHintString('Mod+x', false)).toBe('control x')) + + it('returns "command" for "mod" on Apple platforms', () => + expect(getAccessibleKeybindingHintString('Mod+x', 'mac')).toBe('command x')) + + it('returns "control" for "mod" on non-Apple platforms', () => { + expect(getAccessibleKeybindingHintString('Mod+x', 'windows')).toBe('control x') + expect(getAccessibleKeybindingHintString('Mod+x', 'other')).toBe('control x') + }) + + it('returns "command" for "meta" on Apple platforms', () => + expect(getAccessibleKeybindingHintString('Meta+x', 'mac')).toBe('command x')) + + it('returns "Windows" for "meta" on Windows', () => + expect(getAccessibleKeybindingHintString('Meta+x', 'windows')).toBe('Windows x')) + + it('returns "meta" for "meta" on other platforms', () => + expect(getAccessibleKeybindingHintString('Meta+x', 'other')).toBe('meta x')) + + it('returns "option" for "alt" on Apple platforms and "alt" elsewhere', () => { + expect(getAccessibleKeybindingHintString('Alt+x', 'mac')).toBe('option x') + expect(getAccessibleKeybindingHintString('Alt+x', 'windows')).toBe('alt x') + expect(getAccessibleKeybindingHintString('Alt+x', 'other')).toBe('alt x') + }) }) diff --git a/packages/react/src/KeybindingHint/KeybindingHint.tsx b/packages/react/src/KeybindingHint/KeybindingHint.tsx index 6668799704f..8cd0418c149 100644 --- a/packages/react/src/KeybindingHint/KeybindingHint.tsx +++ b/packages/react/src/KeybindingHint/KeybindingHint.tsx @@ -3,6 +3,7 @@ import {memo} from 'react' import Text from '../Text' import type {KeybindingHintProps} from './props' import {accessibleSequenceString, Sequence} from './components/Sequence' +import type {Platform} from './platform' import classes from './KeybindingHint.module.css' import {clsx} from 'clsx' @@ -37,5 +38,10 @@ KeybindingHint.displayName = 'KeybindingHint' * * NOTE that this string should _only_ be used when building `aria-label` or `aria-description` props (never rendered * visibly) and should nearly always also be paired with a visible hint for sighted users. + * + * The `platform` argument controls how platform-specific keys (such as `Meta`, `Alt`, and `Mod`) are named. For + * backwards compatibility, a `boolean` may be passed instead, where `true` is treated as `'mac'` and `false` as + * `'other'`. */ -export const getAccessibleKeybindingHintString = accessibleSequenceString +export const getAccessibleKeybindingHintString = (sequence: string, platform: Platform | boolean) => + accessibleSequenceString(sequence, typeof platform === 'boolean' ? (platform ? 'mac' : 'other') : platform) diff --git a/packages/react/src/KeybindingHint/components/Chord.tsx b/packages/react/src/KeybindingHint/components/Chord.tsx index ad8ded2debf..87de9189c57 100644 --- a/packages/react/src/KeybindingHint/components/Chord.tsx +++ b/packages/react/src/KeybindingHint/components/Chord.tsx @@ -3,6 +3,7 @@ import Text from '../../Text' import type {KeybindingHintProps} from '../props' import {Key} from './Key' import {accessibleKeyName} from '../key-names' +import type {Platform} from '../platform' import {clsx} from 'clsx' import classes from './Chord.module.css' @@ -56,7 +57,7 @@ export const Chord = ({keys, format = 'condensed', variant = 'normal', size = 'n ) /** Plain string version of `Chord` for use in `aria` string attributes. */ -export const accessibleChordString = (chord: string, isMacOS: boolean) => +export const accessibleChordString = (chord: string, platform: Platform) => splitChord(chord) - .map(key => accessibleKeyName(key, isMacOS)) + .map(key => accessibleKeyName(key, platform)) .join(' ') diff --git a/packages/react/src/KeybindingHint/components/Key.tsx b/packages/react/src/KeybindingHint/components/Key.tsx index ce85bcdcb5f..f17703c15bc 100644 --- a/packages/react/src/KeybindingHint/components/Key.tsx +++ b/packages/react/src/KeybindingHint/components/Key.tsx @@ -1,7 +1,7 @@ import VisuallyHidden from '../../_VisuallyHidden' import {accessibleKeyName, condensedKeyName, fullKeyName} from '../key-names' import type {KeybindingHintFormat} from '../props' -import {useIsMacOS} from '../../hooks/useIsMacOS' +import {usePlatform} from '../platform' interface KeyProps { name: string @@ -10,12 +10,12 @@ interface KeyProps { /** Renders a single key with accessible alternative text. */ export const Key = ({name, format}: KeyProps) => { - const isMacOS = useIsMacOS() + const platform = usePlatform() return ( <> - {accessibleKeyName(name, isMacOS)} - {format === 'condensed' ? condensedKeyName(name, isMacOS) : fullKeyName(name, isMacOS)} + {accessibleKeyName(name, platform)} + {format === 'condensed' ? condensedKeyName(name, platform) : fullKeyName(name, platform)} ) } diff --git a/packages/react/src/KeybindingHint/components/Sequence.tsx b/packages/react/src/KeybindingHint/components/Sequence.tsx index ff16fb8675d..d69d216ba89 100644 --- a/packages/react/src/KeybindingHint/components/Sequence.tsx +++ b/packages/react/src/KeybindingHint/components/Sequence.tsx @@ -2,6 +2,7 @@ import {Fragment} from 'react' import type {KeybindingHintProps} from '../props' import VisuallyHidden from '../../_VisuallyHidden' import {accessibleChordString, Chord} from './Chord' +import type {Platform} from '../platform' const splitSequence = (sequence: string) => sequence.split(' ') @@ -21,7 +22,7 @@ export const Sequence = ({keys, ...chordProps}: KeybindingHintProps) => )) /** Plain string version of `Sequence` for use in `aria` string attributes. */ -export const accessibleSequenceString = (sequence: string, isMacOS: boolean) => +export const accessibleSequenceString = (sequence: string, platform: Platform) => splitSequence(sequence) - .map(chord => accessibleChordString(chord, isMacOS)) + .map(chord => accessibleChordString(chord, platform)) .join(' then ') diff --git a/packages/react/src/KeybindingHint/key-names.ts b/packages/react/src/KeybindingHint/key-names.ts index 6d0543656b4..e5b55839aca 100644 --- a/packages/react/src/KeybindingHint/key-names.ts +++ b/packages/react/src/KeybindingHint/key-names.ts @@ -1,3 +1,5 @@ +import type {Platform} from './platform' + /** Converts the first character of the string to upper case and the remaining to lower case. */ // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition const capitalize = ([first, ...rest]: string) => (first?.toUpperCase() ?? '') + rest.join('').toLowerCase() @@ -9,13 +11,13 @@ const capitalize = ([first, ...rest]: string) => (first?.toUpperCase() ?? '') + /** * Short-form iconic versions of keys. These should be intuitive (not archaic) and match icons on keyboards. */ -export const condensedKeyName = (key: string, isMacOS: boolean) => +export const condensedKeyName = (key: string, platform: Platform) => ({ - alt: isMacOS ? '⌥' : 'Alt', // the alt key _is_ the option key on MacOS - in the browser there is no "option" key + alt: platform === 'mac' ? '⌥' : 'Alt', // the alt key _is_ the option key on MacOS - in the browser there is no "option" key control: '⌃', shift: '⇧', - meta: isMacOS ? '⌘' : 'Win', - mod: isMacOS ? '⌘' : '⌃', + meta: platform === 'mac' ? '⌘' : platform === 'windows' ? 'Win' : 'Meta', + mod: platform === 'mac' ? '⌘' : '⌃', pageup: 'PgUp', pagedown: 'PgDn', arrowup: '↑', @@ -39,10 +41,11 @@ export const condensedKeyName = (key: string, isMacOS: boolean) => * Specific key displays for 'full' format. We still do show some icons (ie punctuation) * because that's more intuitive, but for the rest of keys we show the standard key name. */ -export const fullKeyName = (key: string, isMacOS: boolean) => +export const fullKeyName = (key: string, platform: Platform) => ({ - alt: isMacOS ? 'Option' : 'Alt', - mod: isMacOS ? 'Command' : 'Control', + alt: platform === 'mac' ? 'Option' : 'Alt', + meta: platform === 'mac' ? 'Command' : platform === 'windows' ? 'Windows' : 'Meta', + mod: platform === 'mac' ? 'Command' : 'Control', '+': 'Plus', pageup: 'Page Up', pagedown: 'Page Down', @@ -59,11 +62,11 @@ export const fullKeyName = (key: string, isMacOS: boolean) => * readers from expressing punctuation in speech, ie, reading a long pause instead of the * word "period". */ -export const accessibleKeyName = (key: string, isMacOS: boolean) => +export const accessibleKeyName = (key: string, platform: Platform) => ({ - alt: isMacOS ? 'option' : 'alt', - meta: isMacOS ? 'command' : 'Windows', - mod: isMacOS ? 'command' : 'control', + alt: platform === 'mac' ? 'option' : 'alt', + meta: platform === 'mac' ? 'command' : platform === 'windows' ? 'Windows' : 'meta', + mod: platform === 'mac' ? 'command' : 'control', // Screen readers may not be able to pronounce concatenated words - this provides a better experience pageup: 'page up', pagedown: 'page down', diff --git a/packages/react/src/KeybindingHint/platform.ts b/packages/react/src/KeybindingHint/platform.ts new file mode 100644 index 00000000000..3be3bf2b9f6 --- /dev/null +++ b/packages/react/src/KeybindingHint/platform.ts @@ -0,0 +1,48 @@ +import {isMacOS as ssrUnsafeIsMacOS} from '@primer/behaviors/utils' +import {useSyncExternalStore} from 'react' + +/** + * The platform categories that affect how keyboard shortcut keys are displayed. + * + * - `mac`: Apple platforms (macOS and iOS/iPadOS), which use the Command and Option keys. + * - `windows`: Windows, which uses the Windows (Meta) key. + * - `other`: Any other platform (e.g. Linux, Android), where the Meta key does not map to a + * consistent label. + */ +export type Platform = 'mac' | 'windows' | 'other' + +// No-op. The platform never changes at runtime, so there is nothing to +// subscribe to. Hoisted to avoid creating a new function on every call. +const subscribe = () => () => {} + +/** SSR-unsafe detection of iOS/iPadOS (in addition to macOS, which is detected separately). */ +const ssrUnsafeIsIOS = () => { + if (typeof navigator === 'undefined') return false + return /iphone|ipad|ipod/i.test(navigator.platform) || /iphone|ipad|ipod/i.test(navigator.userAgent) +} + +/** SSR-unsafe detection of Windows. */ +const ssrUnsafeIsWindows = () => { + if (typeof navigator === 'undefined') return false + return /^win/i.test(navigator.platform) +} + +const getSnapshot = (): Platform => { + if (ssrUnsafeIsMacOS() || ssrUnsafeIsIOS()) return 'mac' + if (ssrUnsafeIsWindows()) return 'windows' + return 'other' +} + +// Safe default for SSR since we can't detect the platform on the server. +const getServerSnapshot = (): Platform => 'other' + +/** + * SSR-safe hook for determining the current platform category used when displaying + * keyboard shortcut keys. + * + * Mirrors the approach of `useIsMacOS`: on the client it reads the real value immediately, + * and on the server it returns a safe default (`'other'`). + */ +export function usePlatform(): Platform { + return useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot) +} diff --git a/packages/react/src/TooltipV2/Tooltip.tsx b/packages/react/src/TooltipV2/Tooltip.tsx index ad347d927b4..cb264360c98 100644 --- a/packages/react/src/TooltipV2/Tooltip.tsx +++ b/packages/react/src/TooltipV2/Tooltip.tsx @@ -1,5 +1,5 @@ import React, {Children, useEffect, useRef, useState, useMemo, type ForwardRefExoticComponent} from 'react' -import {useId, useProvidedRefOrCreate, useOnEscapePress, useIsMacOS} from '../hooks' +import {useId, useProvidedRefOrCreate, useOnEscapePress} from '../hooks' import {invariant} from '../utils/invariant' import {warning} from '../utils/warning' import {getAnchoredPosition} from '@primer/behaviors' @@ -8,6 +8,7 @@ import {isSupported, apply} from '@oddbird/popover-polyfill/fn' import {clsx} from 'clsx' import classes from './Tooltip.module.css' import {getAccessibleKeybindingHintString, KeybindingHint, type KeybindingHintProps} from '../KeybindingHint' +import {usePlatform} from '../KeybindingHint/platform' import VisuallyHidden from '../_VisuallyHidden' import useSafeTimeout from '../hooks/useSafeTimeout' import type {SlotMarker} from '../utils/types' @@ -272,7 +273,7 @@ export const Tooltip: ForwardRefExoticComponent< [isPopoverOpen], ) - const isMacOS = useIsMacOS() + const platform = usePlatform() const hasAriaLabel = 'aria-label' in rest // Normalize keybindingHint to an array for uniform rendering @@ -370,7 +371,7 @@ export const Tooltip: ForwardRefExoticComponent< to duplicate the symbols and key names. To work around this, we exclude the hint from being part of the label and instead render the plain keybinding description string. */} - ({keybindingHints.map(hint => getAccessibleKeybindingHintString(hint, isMacOS)).join(' or ')}) + ({keybindingHints.map(hint => getAccessibleKeybindingHintString(hint, platform)).join(' or ')}) ( const [isSubTreeEmpty, setIsSubTreeEmpty] = React.useState(!hasSubTree) const [actionCommandPressed, setActionCommandPressed] = React.useState(false) const [isFocused, setIsFocused] = React.useState(false) - const isMacOS = useIsMacOS() + const platform = usePlatform() // Set the expanded state and cache it const setIsExpandedWithCache = React.useCallback( @@ -335,8 +335,8 @@ const Item = React.forwardRef( slots.trailingVisual ? trailingVisualId : null, ].filter(Boolean) - const shortcut = `Shift+${isMacOS ? 'Meta' : 'Control'}+U` - const trailingActionShortcutText = `Press (${getAccessibleKeybindingHintString(shortcut, isMacOS)}) for more actions.` + const shortcut = `Shift+${platform === 'mac' ? 'Meta' : 'Control'}+U` + const trailingActionShortcutText = `Press (${getAccessibleKeybindingHintString(shortcut, platform)}) for more actions.` return ( Date: Fri, 29 May 2026 23:05:33 +0000 Subject: [PATCH 2/2] Add story demonstrating modifier keys across platforms --- ...KeybindingHint.features.stories.module.css | 16 ++++++ .../KeybindingHint.features.stories.tsx | 57 +++++++++++++++++++ packages/react/src/KeybindingHint/platform.ts | 18 +++++- 3 files changed, 89 insertions(+), 2 deletions(-) diff --git a/packages/react/src/KeybindingHint/KeybindingHint.features.stories.module.css b/packages/react/src/KeybindingHint/KeybindingHint.features.stories.module.css index 4e208bfc240..291d6fc3b87 100644 --- a/packages/react/src/KeybindingHint/KeybindingHint.features.stories.module.css +++ b/packages/react/src/KeybindingHint/KeybindingHint.features.stories.module.css @@ -7,3 +7,19 @@ background-color: var(--button-primary-bgColor-rest); padding: var(--base-size-12); } + +.PlatformTable { + /* stylelint-disable-next-line primer/borders -- collapse is a layout keyword, not a border size */ + border-collapse: collapse; +} + +.PlatformCell { + padding: var(--base-size-8) var(--base-size-12); + text-align: left; + border: var(--borderWidth-thin) solid var(--borderColor-default); +} + +.PlatformHeader { + color: var(--fgColor-muted); + font-weight: var(--base-text-weight-semibold); +} diff --git a/packages/react/src/KeybindingHint/KeybindingHint.features.stories.tsx b/packages/react/src/KeybindingHint/KeybindingHint.features.stories.tsx index 3fadc129df9..e250eab0233 100644 --- a/packages/react/src/KeybindingHint/KeybindingHint.features.stories.tsx +++ b/packages/react/src/KeybindingHint/KeybindingHint.features.stories.tsx @@ -1,5 +1,6 @@ import type {Meta, StoryObj} from '@storybook/react-vite' import {KeybindingHint, type KeybindingHintProps} from '.' +import {PlatformOverrideProvider, type Platform} from './platform' import classes from './KeybindingHint.features.stories.module.css' export default { @@ -38,3 +39,59 @@ export const OnPrimary: StoryObj = { } export const Small = {args: {keys: chord, size: 'small'}} + +const platforms: Array<{platform: Platform; label: string}> = [ + {platform: 'mac', label: 'Apple (macOS / iOS)'}, + {platform: 'windows', label: 'Windows'}, + {platform: 'other', label: 'Other (Linux, Android, …)'}, +] + +const modifierKeys = ['Mod', 'Meta', 'Alt', 'Control', 'Shift'] + +/** + * Demonstrates how each modifier key renders across platforms. The platform is faked via + * `PlatformOverrideProvider` so all variations can be previewed regardless of the device + * actually running Storybook. + */ +export const Platforms: StoryObj = { + render: ({format = 'full', ...args}) => ( + + + + + {modifierKeys.map(key => ( + + ))} + + + + {platforms.map(({platform, label}) => ( + + + {modifierKeys.map(key => ( + + ))} + + ))} + +
+ Platform + + {key} +
+ {label} + + + + +
+ ), + args: {format: 'full'}, + argTypes: { + format: { + control: 'radio', + options: ['condensed', 'full'], + }, + keys: {table: {disable: true}}, + }, +} diff --git a/packages/react/src/KeybindingHint/platform.ts b/packages/react/src/KeybindingHint/platform.ts index 3be3bf2b9f6..153a3b1fce4 100644 --- a/packages/react/src/KeybindingHint/platform.ts +++ b/packages/react/src/KeybindingHint/platform.ts @@ -1,5 +1,5 @@ import {isMacOS as ssrUnsafeIsMacOS} from '@primer/behaviors/utils' -import {useSyncExternalStore} from 'react' +import {createContext, useContext, useSyncExternalStore} from 'react' /** * The platform categories that affect how keyboard shortcut keys are displayed. @@ -36,13 +36,27 @@ const getSnapshot = (): Platform => { // Safe default for SSR since we can't detect the platform on the server. const getServerSnapshot = (): Platform => 'other' +/** + * Allows overriding the detected platform. This is primarily intended for testing and + * Storybook, where we want to preview how keyboard hints render on platforms other than the + * one actually running. A `null` value (the default) means "use the detected platform". + */ +const PlatformOverrideContext = createContext(null) + +export const PlatformOverrideProvider = PlatformOverrideContext.Provider + /** * SSR-safe hook for determining the current platform category used when displaying * keyboard shortcut keys. * * Mirrors the approach of `useIsMacOS`: on the client it reads the real value immediately, * and on the server it returns a safe default (`'other'`). + * + * If a `PlatformOverrideProvider` is present with a non-null value, that value is used + * instead of the detected platform. */ export function usePlatform(): Platform { - return useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot) + const override = useContext(PlatformOverrideContext) + const detected = useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot) + return override ?? detected }