From 9104157e6aa285a6159889608ddfb6bd1c9825c6 Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Thu, 18 Jul 2024 16:41:53 +0000 Subject: [PATCH 01/21] Add `KeybindingHint` component --- docs/content/KeybindingHint.mdx | 138 ++++++++ .../src/@primer/gatsby-theme-doctocat/nav.yml | 2 + .../KeybindingHint/KeybindingHint.docs.json | 28 ++ .../KeybindingHint/KeybindingHint.stories.tsx | 28 ++ .../src/KeybindingHint/KeybindingHint.tsx | 306 ++++++++++++++++++ packages/react/src/KeybindingHint/index.ts | 1 + .../src/__tests__/KeybindingHint.test.tsx | 89 +++++ packages/react/src/index.ts | 2 + 8 files changed, 594 insertions(+) create mode 100644 docs/content/KeybindingHint.mdx create mode 100644 packages/react/src/KeybindingHint/KeybindingHint.docs.json create mode 100644 packages/react/src/KeybindingHint/KeybindingHint.stories.tsx create mode 100644 packages/react/src/KeybindingHint/KeybindingHint.tsx create mode 100644 packages/react/src/KeybindingHint/index.ts create mode 100644 packages/react/src/__tests__/KeybindingHint.test.tsx diff --git a/docs/content/KeybindingHint.mdx b/docs/content/KeybindingHint.mdx new file mode 100644 index 00000000000..88d6039f047 --- /dev/null +++ b/docs/content/KeybindingHint.mdx @@ -0,0 +1,138 @@ +--- +title: KeybindingHint +componentId: keybinding_hint +status: Alpha +source: https://github.com/primer/react/tree/main/packages/react/src/KeybindingHint +storybook: '/react/storybook?path=/story/components-keybindinghint' +description: Indicates the presence of a keybinding available for an action. +--- + +import data from '../../packages/react/src/KeybindingHint/KeybindingHint.docs.json' +import {ActionList, KeybindingHint, Button, Text, Box} from '@primer/react' +import {TrashIcon} from "@primer/octicons-react" + +Use `KeybindingHint` to make keyboard shortcuts discoverable. Can render visual keybinding hints in condensed (abbreviated) form or expanded form, and provides accessible alternative text for screen reader users. + + + + + Move down + + + + Unsubscribe + + + + + Delete + + + + + +## Examples + +### Single keys + +Use the [full names of the keys as returned by `KeyboardEvent.key`](https://developer.mozilla.org/en-US/docs/Web/API/UI_Events/Keyboard_event_key_values). Key names are case-insensitive. + +```javascript live noinline +render(<> +
+
+
+ +) +``` + +#### Special key names + +Because the `+` and ` ` (space) characters are used to build chords and sequences as described below, their names must be spelled out to be used as keys. + +```javascript live noinline +render(<> +
+ +) +``` + +### Chords + +_Chords_ are multiple keys that are pressed at the same time. Combine keys in a chord with `+`. Keys are automatically sorted into a standardized order so that modifiers come first. + +```javascript live noinline +render(<> +
+
+
+ +) +``` + +#### Platform-dependent modifier + +Typical chords use `Command` on MacOS and `Control` on other devices. To automatically render `Command` or `Control` based on the user's operating system, use the special key name `Mod`. + +```javascript live noinline +render() +``` + +### Sequences + +_Sequences_ are keys or chords that are pressed one after the other. Combine elements in a sequence with ` `. For example, `a b` means "press a, then press b". + +```javascript live noinline +render(<> +
+ +) +``` + +### Full display format + +The default `condensed` format should be used on UI elements like buttons, menuitems, and inputs. In long-form text (prose), the `full` variant can be used instead to help the text flow better. + +```javascript live noinline +render( + Press to submit the form. +) +``` + +### `onEmphasis` variant + +When rendering on 'emphasis' colors, use the `onEmphasis` variant. + +```javascript live noinline +const CmdEnterHint = () => + +render( + +) +``` + +## Props + + + +## Status + + + diff --git a/docs/src/@primer/gatsby-theme-doctocat/nav.yml b/docs/src/@primer/gatsby-theme-doctocat/nav.yml index 3d4c3327b64..e2c5e49d499 100644 --- a/docs/src/@primer/gatsby-theme-doctocat/nav.yml +++ b/docs/src/@primer/gatsby-theme-doctocat/nav.yml @@ -82,6 +82,8 @@ url: /Heading - title: IconButton url: /IconButton + - title: KeybindingHint + url: /KeybindingHint - title: Label url: /Label - title: LabelGroup diff --git a/packages/react/src/KeybindingHint/KeybindingHint.docs.json b/packages/react/src/KeybindingHint/KeybindingHint.docs.json new file mode 100644 index 00000000000..67042d19bc8 --- /dev/null +++ b/packages/react/src/KeybindingHint/KeybindingHint.docs.json @@ -0,0 +1,28 @@ +{ + "id": "KeybindingHint", + "name": "KeybindingHint", + "status": "alpha", + "a11yReviewed": false, + "stories": [], + "importPath": "@primer/react", + "props": [ + { + "name": "keys", + "type": "string", + "description": "The keys involved in this keybinding." + }, + { + "name": "format", + "type": "'condensed' | 'full'", + "defaultValue": "'condensed'", + "description": "Control the display format." + }, + { + "name": "variant", + "type": "'normal' | 'onEmphasis'", + "defaultValue": "'normal'", + "description": "Set to `onEmphasis` for display on 'emphasis' colors." + } + ], + "subcomponents": [] +} diff --git a/packages/react/src/KeybindingHint/KeybindingHint.stories.tsx b/packages/react/src/KeybindingHint/KeybindingHint.stories.tsx new file mode 100644 index 00000000000..fe5ebd59685 --- /dev/null +++ b/packages/react/src/KeybindingHint/KeybindingHint.stories.tsx @@ -0,0 +1,28 @@ +import type React from 'react' +import type {Meta} from '@storybook/react' +import {KeybindingHint} from './KeybindingHint' + +type KeybindingHintProps = React.ComponentProps + +const args = { + keys: 'Mod+Shift+K', +} satisfies KeybindingHintProps + +const meta = { + title: 'Components/KeybindingHint', + component: KeybindingHint, +} satisfies Meta + +export default meta + +export const Condensed = {args} + +export const Full = {args: {...args, format: 'full'}} + +const sequenceArgs = { + keys: 'Mod+x y z', +} satisfies KeybindingHintProps + +export const SequenceCondensed = {args: sequenceArgs} + +export const SequenceFull = {args: {...sequenceArgs, format: 'full'}} diff --git a/packages/react/src/KeybindingHint/KeybindingHint.tsx b/packages/react/src/KeybindingHint/KeybindingHint.tsx new file mode 100644 index 00000000000..d4cd56e7d3d --- /dev/null +++ b/packages/react/src/KeybindingHint/KeybindingHint.tsx @@ -0,0 +1,306 @@ +import Text from '../Text' +import type {ReactNode} from 'react' +import React, {Fragment, memo} from 'react' +import VisuallyHidden from '../_VisuallyHidden' + +import {isMacOS} from '@primer/behaviors/utils' + +export interface KeybindingHintProps { + /** + * The keys involved in this keybinding. These should be the full names of the keys as would + * be returned by `KeyboardEvent.key` (e.g. "Control", "Shift", "ArrowUp", "a", etc.). + * + * Combine keys with the "+" character to form chords. To represent the "+" key, use "Plus". + * + * Combine chords/keys with " " to form sequences that should be pressed one after the other. For example, "a b" + * represents "a then b". To represent the " " key, use "Space". + * + * The fake key name "Mod" can be used to represent "Command" on macOS and "Control" on other platforms. + * + * See https://github.com/github/hotkey for format details. + */ + keys: string + /** + * Control the display format. Condensed is most useful in menus and tooltips, while + * the full form is better for prose. + * @default "condensed" + */ + format?: KeybindingHintFormat + /** + * Set to `onEmphasis` for display on emphasis colors. + */ + variant?: KeybindingHintVariant +} + +type KeybindingHintFormat = 'condensed' | 'full' + +type KeybindingHintVariant = 'normal' | 'onEmphasis' + +// In the below records, we don't intend to cover every single possible key - only those that +// would be realistically used in shortcuts. For example, the Pause/Break key is not necessary +// because it is not found on many keyboards. + +/** + * Short-form iconic versions of keys. These should be intuitive and match icons on keyboards. + */ +const condensedKeys = (): Record => ({ + alt: isMacOS() ? '⌥' : '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() ? '⌘' : '⌃', + pageup: 'PgUp', + pagedown: 'PgDn', + arrowup: '↑', + arrowdown: '↓', + arrowleft: '←', + arrowright: '→', + plus: '+', // needed to allow +-separated key names + backspace: '⌫', + delete: 'Del', + space: '␣', // allow consumers to use the word "Space" even though it's not the browser key name, because it's more readable in props + tab: '⇥', + enter: '⏎', + escape: 'Esc', + function: 'Fn', + capslock: 'CapsLock', + insert: 'Ins', + printscreen: 'PrtScn', +}) + +/** + * 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. + */ +const fullKeys = (): Record => ({ + alt: isMacOS() ? 'Option' : 'Alt', + mod: isMacOS() ? 'Command' : 'Control', + '+': 'Plus', + pageup: 'Page Up', + pagedown: 'Page Down', + arrowup: 'Up Arrow', + arrowdown: 'Down Arrow', + arrowleft: 'Left Arrow', + arrowright: 'Right Arrow', + capslock: 'Caps Lock', + printscreen: 'Print Screen', +}) + +/** + * Accessible key names intended to be read by a screen reader. This prevents screen + * readers from expressing punctuation in speech, ie, reading a long pause instead of the + * word "period". + */ +const keyDescriptions = (): Record => ({ + alt: isMacOS() ? 'option' : 'alt', + meta: isMacOS() ? 'command' : 'Windows', + mod: isMacOS() ? 'command' : 'control', + // Screen readers may not be able to pronounce concatenated words - this provides a better experience + pageup: 'page up', + pagedown: 'page down', + arrowup: 'up arrow', + arrowdown: 'down arrow', + arrowleft: 'left arrow', + arrowright: 'right arrow', + capslock: 'caps lock', + printscreen: 'print screen', + // We don't need to represent _every_ symbol - only those found on standard keyboards. + // Other symbols should be avoided as keyboard shortcuts anyway. + // These should match the colloqiual names of the keys, not the names of the symbols. Ie, + // "Equals" not "Equal Sign", "Dash" not "Minus", "Period" not "Dot", etc. + '`': 'backtick', + '~': 'tilde', + '!': 'exclamation point', + '@': 'at', + '#': 'hash', + $: 'dollar sign', + '%': 'percent', + '^': 'caret', + '&': 'ampersand', + '*': 'asterisk', + '(': 'left parenthesis', + ')': 'right parenthesis', + _: 'underscore', + '-': 'dash', + '+': 'plus', + '=': 'equals', + '[': 'left bracket', + '{': 'left curly brace', + ']': 'right bracket', + '}': 'right curly brace', + '\\': 'backslash', + '|': 'pipe', + ';': 'semicolon', + ':': 'colon', + "'": 'single quote', + '"': 'double quote', + ',': 'comma', + '<': 'left angle bracket', + '.': 'period', + '>': 'right angle bracket', + '/': 'forward slash', + '?': 'question mark', + ' ': 'space', +}) + +/** + * Consistent sort order for modifier keys. There should never be more than one non-modifier + * key in a shortcut, so we don't need to worry about sorting those - we just put them at + * the end. + */ +const keySortPriorities = { + control: 1, + meta: 2, + alt: 3, + option: 4, + shift: 5, + function: 6, + /** Maximum value for pushing other keys to end. */ + DEFAULT: 7, +} as const + +function isValidKeySortPriority(priority: string): priority is keyof typeof keySortPriorities { + return priority in keySortPriorities +} + +function getKeySortPriorityValue(priority: string) { + if (isValidKeySortPriority(priority)) { + return keySortPriorities[priority] + } + return keySortPriorities.DEFAULT +} + +/** `kbd` element with style resets. */ +const Kbd = ({children}: {children: ReactNode}) => ( + + {children} + +) + +interface KeyProps { + name: string + format: KeybindingHintFormat +} + +/** + * 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() + +const keyToAccessibleString = (name: string) => keyDescriptions()[name] || name + +const Key = ({name, format}: KeyProps) => ( + // We represent each individual key as a inside a single container element. + // This requires a bit more styling to override the defaults but is the most semantic way + // to do it: + // + // > To describe an input comprised of multiple keystrokes, you can nest multiple + // > elements, with an outer element representing the overall input and each + // > individual keystroke or component of the input enclosed within its own . + // > (https://developer.mozilla.org/en-US/docs/Web/HTML/Element/kbd#representing_keystrokes_within_an_input) + <> + {keyToAccessibleString(name)} + {/* eslint-disable-next-line @typescript-eslint/no-unnecessary-condition */} + {(format === 'condensed' ? condensedKeys()[name] : fullKeys()[name]) ?? capitalize(name)} + +) + +/** Split and sort the chord keys in standard order. */ +const splitChord = (chord: string) => + chord + .split('+') + .map(k => k.toLowerCase()) + .sort(compareLowercaseKeys) + +const compareLowercaseKeys = (a: string, b: string) => getKeySortPriorityValue(a) - getKeySortPriorityValue(b) + +const chordToAccessibleString = (chord: string) => splitChord(chord).map(keyToAccessibleString).join(' ') + +const Chord = ({keys, format = 'condensed', variant = 'normal'}: KeybindingHintProps) => ( + + {splitChord(keys).map((k, i) => ( + + {i > 0 && format === 'full' ? ( + + // hiding the plus sign helps screen readers be more concise + ) : ( + ' ' // space is nonvisual due to flex layout but critical for labelling / screen readers + )} + + + + ))} + +) + +const splitSequence = (sequence: string) => sequence.split(' ') + +const sequenceToAccessibleString = (sequence: string) => + splitSequence(sequence).map(chordToAccessibleString).join(', then ') + +/** + * Indicates the presence of a keybinding available for an action. + */ +// KeybindingHint is a good candidate for memoizing since props will almost never change +export const KeybindingHint = memo(({keys, format = 'condensed', variant}: KeybindingHintProps) => ( + + {splitSequence(keys).map((c, i) => ( + + { + // Since we audibly separate individual keys in chord with space, we need some other separator for chords in a sequence + i > 0 && ( + <> + , then{' '} + + ) + } + + + ))} + +)) +KeybindingHint.displayName = 'KeybindingHint' + +/** + * AVOID: `KeybindingHint` is nearly always sufficient for providing both visible and accessible keyboard hints, and + * will result in a good screen reader experience when used as the target for `aria-describedby` and `aria-labelledby`. + * However, there may be cases where we need a plain string version, such as when building `aria-label` or + * `aria-description`. In that case, this plain string builder can be used instead. + * + * 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. + */ +export const getAccessibleKeybindingHintString = (keys: string) => sequenceToAccessibleString(keys) diff --git a/packages/react/src/KeybindingHint/index.ts b/packages/react/src/KeybindingHint/index.ts new file mode 100644 index 00000000000..93d08a8257d --- /dev/null +++ b/packages/react/src/KeybindingHint/index.ts @@ -0,0 +1 @@ +export * from './KeybindingHint' diff --git a/packages/react/src/__tests__/KeybindingHint.test.tsx b/packages/react/src/__tests__/KeybindingHint.test.tsx new file mode 100644 index 00000000000..1358c735d3f --- /dev/null +++ b/packages/react/src/__tests__/KeybindingHint.test.tsx @@ -0,0 +1,89 @@ +import React from 'react' +import {render, screen} from '@testing-library/react' + +import {KeybindingHint, getAccessibleKeybindingHintString} from '../KeybindingHint' + +describe('KeybindingHint', () => { + it('renders condensed keys by default', () => { + render() + for (const icon of ['⇧', '⌃', 'Fn', 'PgUp']) { + const el = screen.getByText(icon) + expect(el).toBeVisible() + expect(el).toHaveAttribute('aria-hidden') + } + }) + + it('renders accessible key descriptions', () => { + render() + for (const name of ['control', 'shift', 'left curly brace']) { + const el = screen.getByText(name) + expect(el).toBeInTheDocument() + expect(el).not.toHaveAttribute('aria-hidden') + } + }) + + it('renders key names in full format', () => { + render() + for (const name of ['Shift', 'Control', 'Function', 'Up Arrow']) { + const el = screen.getByText(name) + expect(el).toBeVisible() + expect(el).toHaveAttribute('aria-hidden') + } + }) + + it('sorts modifier keys', () => { + render() + const namesInOrder = ['Control', 'Shift', 'Function', 'Page Up'] + const names = screen.getAllByText(text => namesInOrder.includes(text)).map(el => el.textContent) + expect(names).toEqual(namesInOrder) + }) + + it('capitalizes other keys', () => { + render() + for (const key of ['⌃', 'A']) expect(screen.getByText(key)).toBeInTheDocument() + }) + + it.each([ + ['Plus', '+'], + ['Space', '␣'], + ])('renders %s as symbol in condensed mode', (name, symbol) => { + render() + expect(screen.getByText(symbol)).toBeInTheDocument() + }) + + it.each(['Plus', 'Space'])('renders %s as name in full format', name => { + render() + expect(screen.getByText(name)).toBeInTheDocument() + }) + + it('does not render plus signs in condensed mode', () => { + render() + expect(screen.queryByText('+')).not.toBeInTheDocument() + }) + + it('renders plus signs between keys in full format', () => { + render() + const plus = screen.getByText('+') + expect(plus).toBeVisible() + expect(plus).toHaveAttribute('aria-hidden') + }) + + it('renders sequences separated by hidden "then"', () => { + render() + const el = screen.getByText(', then') + expect(el).toBeInTheDocument() + expect(el).not.toHaveAttribute('aria-hidden') + }) +}) + +describe('getAccessibleKeybindingHintString', () => { + it('returns full readable key names', () => expect(getAccessibleKeybindingHintString('{')).toBe('left curly brace')) + + it('joins keys in a chord with space', () => expect(getAccessibleKeybindingHintString('Command+U')).toBe('command u')) + + it('sorts modifiers in standard order', () => + expect(getAccessibleKeybindingHintString('Alt+Shift+Command+%')).toBe('alt shift command percent')) + + it('joins chords in a sequence with "then"', () => + expect(getAccessibleKeybindingHintString('Alt+9 x y')).toBe('alt 9, then x, then y')) +}) diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index 30f2d09c796..4587082a5dd 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -196,6 +196,8 @@ export type {UnderlineNavProps, UnderlineNavItemProps} from './UnderlineNav' export {ActionBar} from './ActionBar' export type {ActionBarProps} from './ActionBar' +export * from './KeybindingHint' + // eslint-disable-next-line no-restricted-imports export {SSRProvider, useSSRSafeId} from './utils/ssr' export {default as sx, merge} from './sx' From 66dc77e3c21c9d190396aa37302d7d6994b584a3 Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Thu, 18 Jul 2024 16:56:55 +0000 Subject: [PATCH 02/21] Split file and refactor a bit --- .../src/KeybindingHint/KeybindingHint.tsx | 138 +----------------- .../react/src/KeybindingHint/key-names.ts | 117 +++++++++++++++ 2 files changed, 124 insertions(+), 131 deletions(-) create mode 100644 packages/react/src/KeybindingHint/key-names.ts diff --git a/packages/react/src/KeybindingHint/KeybindingHint.tsx b/packages/react/src/KeybindingHint/KeybindingHint.tsx index d4cd56e7d3d..361ede3c5a2 100644 --- a/packages/react/src/KeybindingHint/KeybindingHint.tsx +++ b/packages/react/src/KeybindingHint/KeybindingHint.tsx @@ -3,7 +3,7 @@ import type {ReactNode} from 'react' import React, {Fragment, memo} from 'react' import VisuallyHidden from '../_VisuallyHidden' -import {isMacOS} from '@primer/behaviors/utils' +import {accessibleKeyName, condensedKeyName, fullKeyName} from './key-names' export interface KeybindingHintProps { /** @@ -36,113 +36,6 @@ type KeybindingHintFormat = 'condensed' | 'full' type KeybindingHintVariant = 'normal' | 'onEmphasis' -// In the below records, we don't intend to cover every single possible key - only those that -// would be realistically used in shortcuts. For example, the Pause/Break key is not necessary -// because it is not found on many keyboards. - -/** - * Short-form iconic versions of keys. These should be intuitive and match icons on keyboards. - */ -const condensedKeys = (): Record => ({ - alt: isMacOS() ? '⌥' : '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() ? '⌘' : '⌃', - pageup: 'PgUp', - pagedown: 'PgDn', - arrowup: '↑', - arrowdown: '↓', - arrowleft: '←', - arrowright: '→', - plus: '+', // needed to allow +-separated key names - backspace: '⌫', - delete: 'Del', - space: '␣', // allow consumers to use the word "Space" even though it's not the browser key name, because it's more readable in props - tab: '⇥', - enter: '⏎', - escape: 'Esc', - function: 'Fn', - capslock: 'CapsLock', - insert: 'Ins', - printscreen: 'PrtScn', -}) - -/** - * 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. - */ -const fullKeys = (): Record => ({ - alt: isMacOS() ? 'Option' : 'Alt', - mod: isMacOS() ? 'Command' : 'Control', - '+': 'Plus', - pageup: 'Page Up', - pagedown: 'Page Down', - arrowup: 'Up Arrow', - arrowdown: 'Down Arrow', - arrowleft: 'Left Arrow', - arrowright: 'Right Arrow', - capslock: 'Caps Lock', - printscreen: 'Print Screen', -}) - -/** - * Accessible key names intended to be read by a screen reader. This prevents screen - * readers from expressing punctuation in speech, ie, reading a long pause instead of the - * word "period". - */ -const keyDescriptions = (): Record => ({ - alt: isMacOS() ? 'option' : 'alt', - meta: isMacOS() ? 'command' : 'Windows', - mod: isMacOS() ? 'command' : 'control', - // Screen readers may not be able to pronounce concatenated words - this provides a better experience - pageup: 'page up', - pagedown: 'page down', - arrowup: 'up arrow', - arrowdown: 'down arrow', - arrowleft: 'left arrow', - arrowright: 'right arrow', - capslock: 'caps lock', - printscreen: 'print screen', - // We don't need to represent _every_ symbol - only those found on standard keyboards. - // Other symbols should be avoided as keyboard shortcuts anyway. - // These should match the colloqiual names of the keys, not the names of the symbols. Ie, - // "Equals" not "Equal Sign", "Dash" not "Minus", "Period" not "Dot", etc. - '`': 'backtick', - '~': 'tilde', - '!': 'exclamation point', - '@': 'at', - '#': 'hash', - $: 'dollar sign', - '%': 'percent', - '^': 'caret', - '&': 'ampersand', - '*': 'asterisk', - '(': 'left parenthesis', - ')': 'right parenthesis', - _: 'underscore', - '-': 'dash', - '+': 'plus', - '=': 'equals', - '[': 'left bracket', - '{': 'left curly brace', - ']': 'right bracket', - '}': 'right curly brace', - '\\': 'backslash', - '|': 'pipe', - ';': 'semicolon', - ':': 'colon', - "'": 'single quote', - '"': 'double quote', - ',': 'comma', - '<': 'left angle bracket', - '.': 'period', - '>': 'right angle bracket', - '/': 'forward slash', - '?': 'question mark', - ' ': 'space', -}) - /** * Consistent sort order for modifier keys. There should never be more than one non-modifier * key in a shortcut, so we don't need to worry about sorting those - we just put them at @@ -197,27 +90,10 @@ interface KeyProps { format: KeybindingHintFormat } -/** - * 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() - -const keyToAccessibleString = (name: string) => keyDescriptions()[name] || name - const Key = ({name, format}: KeyProps) => ( - // We represent each individual key as a inside a single container element. - // This requires a bit more styling to override the defaults but is the most semantic way - // to do it: - // - // > To describe an input comprised of multiple keystrokes, you can nest multiple - // > elements, with an outer element representing the overall input and each - // > individual keystroke or component of the input enclosed within its own . - // > (https://developer.mozilla.org/en-US/docs/Web/HTML/Element/kbd#representing_keystrokes_within_an_input) <> - {keyToAccessibleString(name)} - {/* eslint-disable-next-line @typescript-eslint/no-unnecessary-condition */} - {(format === 'condensed' ? condensedKeys()[name] : fullKeys()[name]) ?? capitalize(name)} + {accessibleKeyName(name)} + {format === 'condensed' ? condensedKeyName(name) : fullKeyName(name)} ) @@ -230,7 +106,7 @@ const splitChord = (chord: string) => const compareLowercaseKeys = (a: string, b: string) => getKeySortPriorityValue(a) - getKeySortPriorityValue(b) -const chordToAccessibleString = (chord: string) => splitChord(chord).map(keyToAccessibleString).join(' ') +const accessibleChordString = (chord: string) => splitChord(chord).map(accessibleKeyName).join(' ') const Chord = ({keys, format = 'condensed', variant = 'normal'}: KeybindingHintProps) => ( sequence.split(' ') -const sequenceToAccessibleString = (sequence: string) => - splitSequence(sequence).map(chordToAccessibleString).join(', then ') +const accessibleSequenceString = (sequence: string) => + splitSequence(sequence).map(accessibleChordString).join(', then ') /** * Indicates the presence of a keybinding available for an action. @@ -303,4 +179,4 @@ 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. */ -export const getAccessibleKeybindingHintString = (keys: string) => sequenceToAccessibleString(keys) +export const getAccessibleKeybindingHintString = (keys: string) => accessibleSequenceString(keys) diff --git a/packages/react/src/KeybindingHint/key-names.ts b/packages/react/src/KeybindingHint/key-names.ts new file mode 100644 index 00000000000..9b250aed1fe --- /dev/null +++ b/packages/react/src/KeybindingHint/key-names.ts @@ -0,0 +1,117 @@ +// In the below records, we don't intend to cover every single possible key - only those that +// would be realistically used in shortcuts. For example, the Pause/Break key is not necessary +// because it is not found on many keyboards. + +import {isMacOS} from '@primer/behaviors/utils' + +// These are methods instead of plain objects to delay querying isMacOS and avoid SSR issues + +/** 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() + +/** + * Short-form iconic versions of keys. These should be intuitive and match icons on keyboards. + */ +export const condensedKeyName = (key: string) => + ({ + alt: isMacOS() ? '⌥' : '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() ? '⌘' : '⌃', + pageup: 'PgUp', + pagedown: 'PgDn', + arrowup: '↑', + arrowdown: '↓', + arrowleft: '←', + arrowright: '→', + plus: '+', // needed to allow +-separated key names + backspace: '⌫', + delete: 'Del', + space: '␣', // allow consumers to use the word "Space" even though it's not the browser key name, because it's more readable in props + tab: '⇥', + enter: '⏎', + escape: 'Esc', + function: 'Fn', + capslock: 'CapsLock', + insert: 'Ins', + printscreen: 'PrtScn', + })[key] ?? capitalize(key) + +/** + * 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) => + ({ + alt: isMacOS() ? 'Option' : 'Alt', + mod: isMacOS() ? 'Command' : 'Control', + '+': 'Plus', + pageup: 'Page Up', + pagedown: 'Page Down', + arrowup: 'Up Arrow', + arrowdown: 'Down Arrow', + arrowleft: 'Left Arrow', + arrowright: 'Right Arrow', + capslock: 'Caps Lock', + printscreen: 'Print Screen', + })[key] ?? capitalize(key) + +/** + * Accessible key names intended to be read by a screen reader. This prevents screen + * readers from expressing punctuation in speech, ie, reading a long pause instead of the + * word "period". + */ +export const accessibleKeyName = (key: string) => + ({ + alt: isMacOS() ? 'option' : 'alt', + meta: isMacOS() ? 'command' : 'Windows', + mod: isMacOS() ? 'command' : 'control', + // Screen readers may not be able to pronounce concatenated words - this provides a better experience + pageup: 'page up', + pagedown: 'page down', + arrowup: 'up arrow', + arrowdown: 'down arrow', + arrowleft: 'left arrow', + arrowright: 'right arrow', + capslock: 'caps lock', + printscreen: 'print screen', + // We don't need to represent _every_ symbol - only those found on standard keyboards. + // Other symbols should be avoided as keyboard shortcuts anyway. + // These should match the colloqiual names of the keys, not the names of the symbols. Ie, + // "Equals" not "Equal Sign", "Dash" not "Minus", "Period" not "Dot", etc. + '`': 'backtick', + '~': 'tilde', + '!': 'exclamation point', + '@': 'at', + '#': 'hash', + $: 'dollar sign', + '%': 'percent', + '^': 'caret', + '&': 'ampersand', + '*': 'asterisk', + '(': 'left parenthesis', + ')': 'right parenthesis', + _: 'underscore', + '-': 'dash', + '+': 'plus', + '=': 'equals', + '[': 'left bracket', + '{': 'left curly brace', + ']': 'right bracket', + '}': 'right curly brace', + '\\': 'backslash', + '|': 'pipe', + ';': 'semicolon', + ':': 'colon', + "'": 'single quote', + '"': 'double quote', + ',': 'comma', + '<': 'left angle bracket', + '.': 'period', + '>': 'right angle bracket', + '/': 'forward slash', + '?': 'question mark', + ' ': 'space', + })[key] ?? key.toLowerCase() From 04904cdcca3bc59bd388d0f4adc1cbb3de404735 Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Thu, 18 Jul 2024 17:22:30 +0000 Subject: [PATCH 03/21] Split components out into individual files --- .../src/KeybindingHint/KeybindingHint.tsx | 153 ++---------------- .../src/KeybindingHint/components/Chord.tsx | 67 ++++++++ .../src/KeybindingHint/components/Key.tsx | 17 ++ .../KeybindingHint/components/Sequence.tsx | 25 +++ .../react/src/KeybindingHint/key-names.ts | 10 +- packages/react/src/KeybindingHint/props.ts | 30 ++++ 6 files changed, 154 insertions(+), 148 deletions(-) create mode 100644 packages/react/src/KeybindingHint/components/Chord.tsx create mode 100644 packages/react/src/KeybindingHint/components/Key.tsx create mode 100644 packages/react/src/KeybindingHint/components/Sequence.tsx create mode 100644 packages/react/src/KeybindingHint/props.ts diff --git a/packages/react/src/KeybindingHint/KeybindingHint.tsx b/packages/react/src/KeybindingHint/KeybindingHint.tsx index 361ede3c5a2..c6aec174649 100644 --- a/packages/react/src/KeybindingHint/KeybindingHint.tsx +++ b/packages/react/src/KeybindingHint/KeybindingHint.tsx @@ -1,67 +1,10 @@ +import React, {type ReactNode} from 'react' +import {memo} from 'react' import Text from '../Text' -import type {ReactNode} from 'react' -import React, {Fragment, memo} from 'react' -import VisuallyHidden from '../_VisuallyHidden' -import {accessibleKeyName, condensedKeyName, fullKeyName} from './key-names' - -export interface KeybindingHintProps { - /** - * The keys involved in this keybinding. These should be the full names of the keys as would - * be returned by `KeyboardEvent.key` (e.g. "Control", "Shift", "ArrowUp", "a", etc.). - * - * Combine keys with the "+" character to form chords. To represent the "+" key, use "Plus". - * - * Combine chords/keys with " " to form sequences that should be pressed one after the other. For example, "a b" - * represents "a then b". To represent the " " key, use "Space". - * - * The fake key name "Mod" can be used to represent "Command" on macOS and "Control" on other platforms. - * - * See https://github.com/github/hotkey for format details. - */ - keys: string - /** - * Control the display format. Condensed is most useful in menus and tooltips, while - * the full form is better for prose. - * @default "condensed" - */ - format?: KeybindingHintFormat - /** - * Set to `onEmphasis` for display on emphasis colors. - */ - variant?: KeybindingHintVariant -} - -type KeybindingHintFormat = 'condensed' | 'full' - -type KeybindingHintVariant = 'normal' | 'onEmphasis' - -/** - * Consistent sort order for modifier keys. There should never be more than one non-modifier - * key in a shortcut, so we don't need to worry about sorting those - we just put them at - * the end. - */ -const keySortPriorities = { - control: 1, - meta: 2, - alt: 3, - option: 4, - shift: 5, - function: 6, - /** Maximum value for pushing other keys to end. */ - DEFAULT: 7, -} as const - -function isValidKeySortPriority(priority: string): priority is keyof typeof keySortPriorities { - return priority in keySortPriorities -} - -function getKeySortPriorityValue(priority: string) { - if (isValidKeySortPriority(priority)) { - return keySortPriorities[priority] - } - return keySortPriorities.DEFAULT -} +import {Chord} from './components/Chord' +import type {KeybindingHintProps} from './props' +import {accessibleSequenceString} from './components/Sequence' /** `kbd` element with style resets. */ const Kbd = ({children}: {children: ReactNode}) => ( @@ -85,87 +28,11 @@ const Kbd = ({children}: {children: ReactNode}) => ( ) -interface KeyProps { - name: string - format: KeybindingHintFormat -} - -const Key = ({name, format}: KeyProps) => ( - <> - {accessibleKeyName(name)} - {format === 'condensed' ? condensedKeyName(name) : fullKeyName(name)} - -) - -/** Split and sort the chord keys in standard order. */ -const splitChord = (chord: string) => - chord - .split('+') - .map(k => k.toLowerCase()) - .sort(compareLowercaseKeys) - -const compareLowercaseKeys = (a: string, b: string) => getKeySortPriorityValue(a) - getKeySortPriorityValue(b) - -const accessibleChordString = (chord: string) => splitChord(chord).map(accessibleKeyName).join(' ') - -const Chord = ({keys, format = 'condensed', variant = 'normal'}: KeybindingHintProps) => ( - - {splitChord(keys).map((k, i) => ( - - {i > 0 && format === 'full' ? ( - + // hiding the plus sign helps screen readers be more concise - ) : ( - ' ' // space is nonvisual due to flex layout but critical for labelling / screen readers - )} - - - - ))} - -) - -const splitSequence = (sequence: string) => sequence.split(' ') - -const accessibleSequenceString = (sequence: string) => - splitSequence(sequence).map(accessibleChordString).join(', then ') - -/** - * Indicates the presence of a keybinding available for an action. - */ -// KeybindingHint is a good candidate for memoizing since props will almost never change -export const KeybindingHint = memo(({keys, format = 'condensed', variant}: KeybindingHintProps) => ( +/** Indicates the presence of an available keybinding. */ +// KeybindingHint is a good candidate for memoizing since props will rarely change +export const KeybindingHint = memo((props: KeybindingHintProps) => ( - {splitSequence(keys).map((c, i) => ( - - { - // Since we audibly separate individual keys in chord with space, we need some other separator for chords in a sequence - i > 0 && ( - <> - , then{' '} - - ) - } - - - ))} + )) KeybindingHint.displayName = 'KeybindingHint' @@ -179,4 +46,4 @@ 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. */ -export const getAccessibleKeybindingHintString = (keys: string) => accessibleSequenceString(keys) +export const getAccessibleKeybindingHintString = accessibleSequenceString diff --git a/packages/react/src/KeybindingHint/components/Chord.tsx b/packages/react/src/KeybindingHint/components/Chord.tsx new file mode 100644 index 00000000000..b1eb89287af --- /dev/null +++ b/packages/react/src/KeybindingHint/components/Chord.tsx @@ -0,0 +1,67 @@ +import React, {Fragment} from 'react' +import Text from '../../Text' +import type {KeybindingHintProps} from '../props' +import {Key} from './Key' +import {accessibleKeyName} from '../key-names' + +/** + * Consistent sort order for modifier keys. There should never be more than one non-modifier + * key in a chord, so we don't need to worry about sorting those - we just put them at + * the end. + */ +const keySortPriorities: Partial> = { + control: 1, + meta: 2, + alt: 3, + option: 4, + shift: 5, + function: 6, +} + +const keySortPriority = (priority: string) => keySortPriorities[priority] ?? Infinity + +const compareLowercaseKeys = (a: string, b: string) => keySortPriority(a) - keySortPriority(b) + +/** Split and sort the chord keys in standard order. */ +const splitChord = (chord: string) => + chord + .split('+') + .map(k => k.toLowerCase()) + .sort(compareLowercaseKeys) + +export const Chord = ({keys, format = 'condensed', variant = 'normal'}: KeybindingHintProps) => ( + + {splitChord(keys).map((k, i) => ( + + {i > 0 && format === 'full' ? ( + + // hiding the plus sign helps screen readers be more concise + ) : ( + ' ' // space is nonvisual due to flex layout but critical for labelling / screen readers + )} + + + + ))} + +) + +/** Plain string version of `Chord` for use in `aria` string attributes. */ +export const accessibleChordString = (chord: string) => splitChord(chord).map(accessibleKeyName).join(' ') diff --git a/packages/react/src/KeybindingHint/components/Key.tsx b/packages/react/src/KeybindingHint/components/Key.tsx new file mode 100644 index 00000000000..7ae3775954c --- /dev/null +++ b/packages/react/src/KeybindingHint/components/Key.tsx @@ -0,0 +1,17 @@ +import React from 'react' +import VisuallyHidden from '../../_VisuallyHidden' +import {accessibleKeyName, condensedKeyName, fullKeyName} from '../key-names' +import type {KeybindingHintFormat} from '../props' + +interface KeyProps { + name: string + format: KeybindingHintFormat +} + +/** Renders a single key with accessible alternative text. */ +export const Key = ({name, format}: KeyProps) => ( + <> + {accessibleKeyName(name)} + {format === 'condensed' ? condensedKeyName(name) : fullKeyName(name)} + +) diff --git a/packages/react/src/KeybindingHint/components/Sequence.tsx b/packages/react/src/KeybindingHint/components/Sequence.tsx new file mode 100644 index 00000000000..2fe7f5e43da --- /dev/null +++ b/packages/react/src/KeybindingHint/components/Sequence.tsx @@ -0,0 +1,25 @@ +import React, {Fragment} from 'react' +import type {KeybindingHintProps} from '../props' +import VisuallyHidden from '../../_VisuallyHidden' +import {accessibleChordString, Chord} from './Chord' + +const splitSequence = (sequence: string) => sequence.split(' ') + +export const Sequence = ({keys, format = 'condensed', variant = 'normal'}: KeybindingHintProps) => + splitSequence(keys).map((c, i) => ( + + { + // Since we audibly separate individual keys in chord with space, we need some other separator for chords in a sequence + i > 0 && ( + <> + , then{' '} + + ) + } + + + )) + +/** Plain string version of `Sequence` for use in `aria` string attributes. */ +export const accessibleSequenceString = (sequence: string) => + splitSequence(sequence).map(accessibleChordString).join(', then ') diff --git a/packages/react/src/KeybindingHint/key-names.ts b/packages/react/src/KeybindingHint/key-names.ts index 9b250aed1fe..80f5ad65e03 100644 --- a/packages/react/src/KeybindingHint/key-names.ts +++ b/packages/react/src/KeybindingHint/key-names.ts @@ -4,14 +4,14 @@ import {isMacOS} from '@primer/behaviors/utils' -// These are methods instead of plain objects to delay querying isMacOS and avoid SSR issues - /** 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() +// These are methods instead of plain objects to delay calling isMacOS (which depends on `window.navigator`) and avoid SSR issues + /** - * Short-form iconic versions of keys. These should be intuitive and match icons on keyboards. + * Short-form iconic versions of keys. These should be intuitive (not archaic) and match icons on keyboards. */ export const condensedKeyName = (key: string) => ({ @@ -26,10 +26,10 @@ export const condensedKeyName = (key: string) => arrowdown: '↓', arrowleft: '←', arrowright: '→', - plus: '+', // needed to allow +-separated key names + plus: '+', // needed to allow +-separated chords backspace: '⌫', delete: 'Del', - space: '␣', // allow consumers to use the word "Space" even though it's not the browser key name, because it's more readable in props + space: '␣', // needed to allow space-separated sequences tab: '⇥', enter: '⏎', escape: 'Esc', diff --git a/packages/react/src/KeybindingHint/props.ts b/packages/react/src/KeybindingHint/props.ts new file mode 100644 index 00000000000..72584086435 --- /dev/null +++ b/packages/react/src/KeybindingHint/props.ts @@ -0,0 +1,30 @@ +export type KeybindingHintFormat = 'condensed' | 'full' + +export type KeybindingHintVariant = 'normal' | 'onEmphasis' + +export interface KeybindingHintProps { + /** + * The keys involved in this keybinding. These should be the full names of the keys as would + * be returned by `KeyboardEvent.key` (e.g. "Control", "Shift", "ArrowUp", "a", etc.). + * + * Combine keys with the "+" character to form chords. To represent the "+" key, use "Plus". + * + * Combine chords/keys with " " to form sequences that should be pressed one after the other. For example, "a b" + * represents "a then b". To represent the " " key, use "Space". + * + * The fake key name "Mod" can be used to represent "Command" on macOS and "Control" on other platforms. + * + * See https://github.com/github/hotkey for format details. + */ + keys: string + /** + * Control the display format. Condensed is most useful in menus and tooltips, while + * the full form is better for prose. + * @default "condensed" + */ + format?: KeybindingHintFormat + /** + * Set to `onEmphasis` for display on emphasis colors. + */ + variant?: KeybindingHintVariant +} From cec4e50fd7ce949bf06abdde6c0cc88f34a08d87 Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Thu, 18 Jul 2024 17:24:12 +0000 Subject: [PATCH 04/21] Update comments --- packages/react/src/KeybindingHint/key-names.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/react/src/KeybindingHint/key-names.ts b/packages/react/src/KeybindingHint/key-names.ts index 80f5ad65e03..eab7f860e56 100644 --- a/packages/react/src/KeybindingHint/key-names.ts +++ b/packages/react/src/KeybindingHint/key-names.ts @@ -1,14 +1,15 @@ -// In the below records, we don't intend to cover every single possible key - only those that -// would be realistically used in shortcuts. For example, the Pause/Break key is not necessary -// because it is not found on many keyboards. - import {isMacOS} from '@primer/behaviors/utils' /** 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() -// These are methods instead of plain objects to delay calling isMacOS (which depends on `window.navigator`) and avoid SSR issues +// In the below records, we don't intend to cover every single possible key - only those that +// would be realistically used in shortcuts. For example, the Pause/Break key is not necessary +// because it is not found on many keyboards. + +// These are methods instead of plain objects to delay calling isMacOS (which depends on +// `window.navigator`) and avoid SSR issues /** * Short-form iconic versions of keys. These should be intuitive (not archaic) and match icons on keyboards. From 8a37a6fade2052feafe6f64cf283c1382bcc04cd Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Thu, 18 Jul 2024 17:51:47 +0000 Subject: [PATCH 05/21] Create `useIsMacOS` hook for SSR support --- .../src/KeybindingHint/components/Chord.tsx | 5 +++- .../src/KeybindingHint/components/Key.tsx | 17 +++++++----- .../KeybindingHint/components/Sequence.tsx | 6 +++-- .../react/src/KeybindingHint/key-names.ts | 27 ++++++++----------- .../src/__tests__/KeybindingHint.test.tsx | 16 ++++++++--- packages/react/src/hooks/index.ts | 1 + packages/react/src/hooks/useIsMacOS.ts | 16 +++++++++++ 7 files changed, 59 insertions(+), 29 deletions(-) create mode 100644 packages/react/src/hooks/useIsMacOS.ts diff --git a/packages/react/src/KeybindingHint/components/Chord.tsx b/packages/react/src/KeybindingHint/components/Chord.tsx index b1eb89287af..9bc7dbcde05 100644 --- a/packages/react/src/KeybindingHint/components/Chord.tsx +++ b/packages/react/src/KeybindingHint/components/Chord.tsx @@ -64,4 +64,7 @@ export const Chord = ({keys, format = 'condensed', variant = 'normal'}: Keybindi ) /** Plain string version of `Chord` for use in `aria` string attributes. */ -export const accessibleChordString = (chord: string) => splitChord(chord).map(accessibleKeyName).join(' ') +export const accessibleChordString = (chord: string, isMacOS: boolean) => + splitChord(chord) + .map(key => accessibleKeyName(key, isMacOS)) + .join(' ') diff --git a/packages/react/src/KeybindingHint/components/Key.tsx b/packages/react/src/KeybindingHint/components/Key.tsx index 7ae3775954c..eb803924f09 100644 --- a/packages/react/src/KeybindingHint/components/Key.tsx +++ b/packages/react/src/KeybindingHint/components/Key.tsx @@ -2,6 +2,7 @@ import React from 'react' import VisuallyHidden from '../../_VisuallyHidden' import {accessibleKeyName, condensedKeyName, fullKeyName} from '../key-names' import type {KeybindingHintFormat} from '../props' +import {useIsMacOS} from '../../hooks/useIsMacOS' interface KeyProps { name: string @@ -9,9 +10,13 @@ interface KeyProps { } /** Renders a single key with accessible alternative text. */ -export const Key = ({name, format}: KeyProps) => ( - <> - {accessibleKeyName(name)} - {format === 'condensed' ? condensedKeyName(name) : fullKeyName(name)} - -) +export const Key = ({name, format}: KeyProps) => { + const isMacOS = useIsMacOS() + + return ( + <> + {accessibleKeyName(name, isMacOS)} + {format === 'condensed' ? condensedKeyName(name, isMacOS) : fullKeyName(name, isMacOS)} + + ) +} diff --git a/packages/react/src/KeybindingHint/components/Sequence.tsx b/packages/react/src/KeybindingHint/components/Sequence.tsx index 2fe7f5e43da..ce4dd5297f1 100644 --- a/packages/react/src/KeybindingHint/components/Sequence.tsx +++ b/packages/react/src/KeybindingHint/components/Sequence.tsx @@ -21,5 +21,7 @@ export const Sequence = ({keys, format = 'condensed', variant = 'normal'}: Keybi )) /** Plain string version of `Sequence` for use in `aria` string attributes. */ -export const accessibleSequenceString = (sequence: string) => - splitSequence(sequence).map(accessibleChordString).join(', then ') +export const accessibleSequenceString = (sequence: string, isMacOS: boolean) => + splitSequence(sequence) + .map(chord => accessibleChordString(chord, isMacOS)) + .join(', then ') diff --git a/packages/react/src/KeybindingHint/key-names.ts b/packages/react/src/KeybindingHint/key-names.ts index eab7f860e56..de5eb5e95bb 100644 --- a/packages/react/src/KeybindingHint/key-names.ts +++ b/packages/react/src/KeybindingHint/key-names.ts @@ -1,5 +1,3 @@ -import {isMacOS} from '@primer/behaviors/utils' - /** 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() @@ -8,19 +6,16 @@ const capitalize = ([first, ...rest]: string) => (first?.toUpperCase() ?? '') + // would be realistically used in shortcuts. For example, the Pause/Break key is not necessary // because it is not found on many keyboards. -// These are methods instead of plain objects to delay calling isMacOS (which depends on -// `window.navigator`) and avoid SSR issues - /** * Short-form iconic versions of keys. These should be intuitive (not archaic) and match icons on keyboards. */ -export const condensedKeyName = (key: string) => +export const condensedKeyName = (key: string, isMacOS: boolean) => ({ - alt: isMacOS() ? '⌥' : 'Alt', // the alt key _is_ the option key on MacOS - in the browser there is no "option" key + alt: isMacOS ? '⌥' : '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: isMacOS ? '⌘' : 'Win', + mod: isMacOS ? '⌘' : '⌃', pageup: 'PgUp', pagedown: 'PgDn', arrowup: '↑', @@ -44,10 +39,10 @@ export const condensedKeyName = (key: string) => * 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) => +export const fullKeyName = (key: string, isMacOS: boolean) => ({ - alt: isMacOS() ? 'Option' : 'Alt', - mod: isMacOS() ? 'Command' : 'Control', + alt: isMacOS ? 'Option' : 'Alt', + mod: isMacOS ? 'Command' : 'Control', '+': 'Plus', pageup: 'Page Up', pagedown: 'Page Down', @@ -64,11 +59,11 @@ export const fullKeyName = (key: string) => * readers from expressing punctuation in speech, ie, reading a long pause instead of the * word "period". */ -export const accessibleKeyName = (key: string) => +export const accessibleKeyName = (key: string, isMacOS: boolean) => ({ - alt: isMacOS() ? 'option' : 'alt', - meta: isMacOS() ? 'command' : 'Windows', - mod: isMacOS() ? 'command' : 'control', + alt: isMacOS ? 'option' : 'alt', + meta: isMacOS ? 'command' : 'Windows', + mod: isMacOS ? '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/__tests__/KeybindingHint.test.tsx b/packages/react/src/__tests__/KeybindingHint.test.tsx index 1358c735d3f..bee45ff08e5 100644 --- a/packages/react/src/__tests__/KeybindingHint.test.tsx +++ b/packages/react/src/__tests__/KeybindingHint.test.tsx @@ -77,13 +77,21 @@ describe('KeybindingHint', () => { }) describe('getAccessibleKeybindingHintString', () => { - it('returns full readable key names', () => expect(getAccessibleKeybindingHintString('{')).toBe('left curly brace')) + it('returns full readable key names', () => + expect(getAccessibleKeybindingHintString('{', false)).toBe('left curly brace')) - it('joins keys in a chord with space', () => expect(getAccessibleKeybindingHintString('Command+U')).toBe('command u')) + it('joins keys in a chord with space', () => + expect(getAccessibleKeybindingHintString('Command+U', false)).toBe('command u')) it('sorts modifiers in standard order', () => - expect(getAccessibleKeybindingHintString('Alt+Shift+Command+%')).toBe('alt shift command percent')) + expect(getAccessibleKeybindingHintString('Alt+Shift+Command+%', false)).toBe('alt shift command percent')) it('joins chords in a sequence with "then"', () => - expect(getAccessibleKeybindingHintString('Alt+9 x y')).toBe('alt 9, then x, then y')) + expect(getAccessibleKeybindingHintString('Alt+9 x y', false)).toBe('alt 9, then x, then y')) + + it('returns "command" for "mod" on MacOS', () => + expect(getAccessibleKeybindingHintString('Mod+x', true)).toBe('command x')) + + it('returns "control" for "mod" on non-MacOS', () => + expect(getAccessibleKeybindingHintString('Mod+x', false)).toBe('control x')) }) diff --git a/packages/react/src/hooks/index.ts b/packages/react/src/hooks/index.ts index 01d1d97c4aa..d8259f5fd96 100644 --- a/packages/react/src/hooks/index.ts +++ b/packages/react/src/hooks/index.ts @@ -15,3 +15,4 @@ export {useMenuKeyboardNavigation} from './useMenuKeyboardNavigation' export {useMnemonics} from './useMnemonics' export {useRefObjectAsForwardedRef} from './useRefObjectAsForwardedRef' export {useId} from './useId' +export {useIsMacOS} from './useIsMacOS' diff --git a/packages/react/src/hooks/useIsMacOS.ts b/packages/react/src/hooks/useIsMacOS.ts new file mode 100644 index 00000000000..daa51906290 --- /dev/null +++ b/packages/react/src/hooks/useIsMacOS.ts @@ -0,0 +1,16 @@ +import {isMacOS as ssrUnsafeIsMacOS} from '@primer/behaviors/utils' +import {useEffect, useState} from 'react' + +/** + * SSR-safe hook for determining if the current platform is MacOS. When rendering + * server-side, will default to non-MacOS and then re-render in an effect if the + * client turns out to be a MacOS device. + */ +export function useIsMacOS() { + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + const [isMacOS, setIsMacOS] = useState(() => (window !== undefined ? ssrUnsafeIsMacOS() : false)) + + useEffect(() => setIsMacOS(ssrUnsafeIsMacOS()), []) + + return isMacOS +} From b0e1e7a8a132c217091c2726f49da18775479526 Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Thu, 18 Jul 2024 18:00:42 +0000 Subject: [PATCH 06/21] Add changelog --- .changeset/short-boats-cover.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/short-boats-cover.md diff --git a/.changeset/short-boats-cover.md b/.changeset/short-boats-cover.md new file mode 100644 index 00000000000..2fcf2d6f408 --- /dev/null +++ b/.changeset/short-boats-cover.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Add `KeybindingHint` component for indicating an available keyboard shortcut From 46c01352fcddfc19c832a837470db00cec506000 Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Thu, 18 Jul 2024 18:04:54 +0000 Subject: [PATCH 07/21] Format --- docs/content/KeybindingHint.mdx | 75 +++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 28 deletions(-) diff --git a/docs/content/KeybindingHint.mdx b/docs/content/KeybindingHint.mdx index 88d6039f047..bdc84d7fbaa 100644 --- a/docs/content/KeybindingHint.mdx +++ b/docs/content/KeybindingHint.mdx @@ -9,7 +9,7 @@ description: Indicates the presence of a keybinding available for an action. import data from '../../packages/react/src/KeybindingHint/KeybindingHint.docs.json' import {ActionList, KeybindingHint, Button, Text, Box} from '@primer/react' -import {TrashIcon} from "@primer/octicons-react" +import {TrashIcon} from '@primer/octicons-react' Use `KeybindingHint` to make keyboard shortcuts discoverable. Can render visual keybinding hints in condensed (abbreviated) form or expanded form, and provides accessible alternative text for screen reader users. @@ -17,16 +17,24 @@ Use `KeybindingHint` to make keyboard shortcuts discoverable. Can render visual Move down - + + + Unsubscribe - + + + - + + + Delete - + + + @@ -38,12 +46,14 @@ Use `KeybindingHint` to make keyboard shortcuts discoverable. Can render visual Use the [full names of the keys as returned by `KeyboardEvent.key`](https://developer.mozilla.org/en-US/docs/Web/API/UI_Events/Keyboard_event_key_values). Key names are case-insensitive. ```javascript live noinline -render(<> -
-
-
- -) +render( + <> +
+
+
+ + , +) ``` #### Special key names @@ -51,10 +61,12 @@ render(<> Because the `+` and ` ` (space) characters are used to build chords and sequences as described below, their names must be spelled out to be used as keys. ```javascript live noinline -render(<> -
- -) +render( + <> +
+ + , +) ``` ### Chords @@ -62,12 +74,14 @@ render(<> _Chords_ are multiple keys that are pressed at the same time. Combine keys in a chord with `+`. Keys are automatically sorted into a standardized order so that modifiers come first. ```javascript live noinline -render(<> -
-
-
- -) +render( + <> +
+
+
+ + , +) ``` #### Platform-dependent modifier @@ -83,10 +97,12 @@ render() _Sequences_ are keys or chords that are pressed one after the other. Combine elements in a sequence with ` `. For example, `a b` means "press a, then press b". ```javascript live noinline -render(<> -
- -) +render( + <> +
+ + , +) ``` ### Full display format @@ -95,7 +111,9 @@ The default `condensed` format should be used on UI elements like buttons, menui ```javascript live noinline render( - Press to submit the form. + + Press to submit the form. + , ) ``` @@ -107,7 +125,9 @@ When rendering on 'emphasis' colors, use the `onEmphasis` variant. const CmdEnterHint = () => render( - + , ) ``` @@ -135,4 +155,3 @@ render( hasFigmaComponent: false, }} /> - From 9b131f3dcc615327c8541d5ee3e797988f1a4813 Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Thu, 18 Jul 2024 18:29:13 +0000 Subject: [PATCH 08/21] Replace space with "space" --- docs/content/KeybindingHint.mdx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/content/KeybindingHint.mdx b/docs/content/KeybindingHint.mdx index bdc84d7fbaa..e3d47b017c0 100644 --- a/docs/content/KeybindingHint.mdx +++ b/docs/content/KeybindingHint.mdx @@ -58,7 +58,7 @@ render( #### Special key names -Because the `+` and ` ` (space) characters are used to build chords and sequences as described below, their names must be spelled out to be used as keys. +Because the `+` and space characters are used to build chords and sequences as described below, their names must be spelled out to be used as keys. ```javascript live noinline render( @@ -94,7 +94,7 @@ render() ### Sequences -_Sequences_ are keys or chords that are pressed one after the other. Combine elements in a sequence with ` `. For example, `a b` means "press a, then press b". +_Sequences_ are keys or chords that are pressed one after the other. Combine elements in a sequence with a space. For example, `a b` means "press a, then press b". ```javascript live noinline render( From e0345aafd2baa0a992dc6e0d25fa6cc1d8bcd5f7 Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Thu, 18 Jul 2024 18:30:54 +0000 Subject: [PATCH 09/21] Update exports snapshot --- packages/react/src/__tests__/__snapshots__/exports.test.ts.snap | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap index 015068cc21d..21d2bdebb21 100644 --- a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap +++ b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap @@ -71,6 +71,7 @@ exports[`@primer/react should not update exports without a semver change 1`] = ` "type FocusTrapHookSettings", "type FocusZoneHookSettings", "FormControl", + "getAccessibleKeybindingHintString", "Header", "type HeaderItemProps", "type HeaderLinkProps", @@ -80,6 +81,7 @@ exports[`@primer/react should not update exports without a semver change 1`] = ` "IconButton", "type IconButtonProps", "IssueLabelToken", + "KeybindingHint", "Label", "LabelGroup", "type LabelGroupProps", From d7eb853b0206fb004b7e502f952a567425c8787d Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Thu, 18 Jul 2024 18:42:28 +0000 Subject: [PATCH 10/21] derp, fix my dumb mistakes --- packages/react/src/KeybindingHint/KeybindingHint.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/react/src/KeybindingHint/KeybindingHint.tsx b/packages/react/src/KeybindingHint/KeybindingHint.tsx index c6aec174649..534f6665991 100644 --- a/packages/react/src/KeybindingHint/KeybindingHint.tsx +++ b/packages/react/src/KeybindingHint/KeybindingHint.tsx @@ -2,9 +2,8 @@ import React, {type ReactNode} from 'react' import {memo} from 'react' import Text from '../Text' -import {Chord} from './components/Chord' import type {KeybindingHintProps} from './props' -import {accessibleSequenceString} from './components/Sequence' +import {accessibleSequenceString, Sequence} from './components/Sequence' /** `kbd` element with style resets. */ const Kbd = ({children}: {children: ReactNode}) => ( @@ -32,7 +31,7 @@ const Kbd = ({children}: {children: ReactNode}) => ( // KeybindingHint is a good candidate for memoizing since props will rarely change export const KeybindingHint = memo((props: KeybindingHintProps) => ( - + )) KeybindingHint.displayName = 'KeybindingHint' From daea8e665bc55b84b9ff4ebc570aa42bfa97b1ee Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Thu, 18 Jul 2024 19:31:30 +0000 Subject: [PATCH 11/21] Try `canUseDOM` instead of `window !== undefined` --- packages/react/src/hooks/useIsMacOS.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/react/src/hooks/useIsMacOS.ts b/packages/react/src/hooks/useIsMacOS.ts index daa51906290..d4982ee7fe8 100644 --- a/packages/react/src/hooks/useIsMacOS.ts +++ b/packages/react/src/hooks/useIsMacOS.ts @@ -1,14 +1,13 @@ import {isMacOS as ssrUnsafeIsMacOS} from '@primer/behaviors/utils' import {useEffect, useState} from 'react' - +import {canUseDOM} from '../utils/environment' /** * SSR-safe hook for determining if the current platform is MacOS. When rendering * server-side, will default to non-MacOS and then re-render in an effect if the * client turns out to be a MacOS device. */ export function useIsMacOS() { - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - const [isMacOS, setIsMacOS] = useState(() => (window !== undefined ? ssrUnsafeIsMacOS() : false)) + const [isMacOS, setIsMacOS] = useState(() => (canUseDOM ? ssrUnsafeIsMacOS() : false)) useEffect(() => setIsMacOS(ssrUnsafeIsMacOS()), []) From 035d5e9ace6266c302d6034002cd84dc1af43ac2 Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Thu, 25 Jul 2024 14:16:47 +0000 Subject: [PATCH 12/21] Move to draft status --- .vscode/settings.json | 4 ++-- docs/content/KeybindingHint.mdx | 2 +- packages/react/src/KeybindingHint/KeybindingHint.docs.json | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 89c7eb34f7f..eb183dfaed1 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -13,11 +13,11 @@ "json.schemas": [ { "fileMatch": ["*.docs.json"], - "url": "./script/components-json/component.schema.json" + "url": "./packages/react/script/components-json/component.schema.json" }, { "fileMatch": ["generated/components.json"], - "url": "./script/components-json/output.schema.json" + "url": "./packages/react/script/components-json/output.schema.json" } ] } diff --git a/docs/content/KeybindingHint.mdx b/docs/content/KeybindingHint.mdx index e3d47b017c0..6b2a171958c 100644 --- a/docs/content/KeybindingHint.mdx +++ b/docs/content/KeybindingHint.mdx @@ -1,7 +1,7 @@ --- title: KeybindingHint componentId: keybinding_hint -status: Alpha +status: Draft source: https://github.com/primer/react/tree/main/packages/react/src/KeybindingHint storybook: '/react/storybook?path=/story/components-keybindinghint' description: Indicates the presence of a keybinding available for an action. diff --git a/packages/react/src/KeybindingHint/KeybindingHint.docs.json b/packages/react/src/KeybindingHint/KeybindingHint.docs.json index 67042d19bc8..136c41954ef 100644 --- a/packages/react/src/KeybindingHint/KeybindingHint.docs.json +++ b/packages/react/src/KeybindingHint/KeybindingHint.docs.json @@ -1,7 +1,7 @@ { "id": "KeybindingHint", "name": "KeybindingHint", - "status": "alpha", + "status": "draft", "a11yReviewed": false, "stories": [], "importPath": "@primer/react", From 69aa6965f9a4ed56f55d010f2f845c9d6642e0e7 Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Tue, 6 Aug 2024 16:17:56 +0000 Subject: [PATCH 13/21] Move export to drafts --- packages/react/src/drafts/index.ts | 2 ++ packages/react/src/index.ts | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react/src/drafts/index.ts b/packages/react/src/drafts/index.ts index 16a78befb4a..c57847c7ef1 100644 --- a/packages/react/src/drafts/index.ts +++ b/packages/react/src/drafts/index.ts @@ -85,3 +85,5 @@ export {UnderlinePanels} from './UnderlinePanels' export type {UnderlinePanelsProps, UnderlinePanelsTabProps, UnderlinePanelsPanelProps} from './UnderlinePanels' export {SkeletonBox, SkeletonText, SkeletonAvatar} from './Skeleton' + +export * from '../KeybindingHint' diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index d8d7a35eb89..2fea9ba96a8 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -196,8 +196,6 @@ export type {UnderlineNavProps, UnderlineNavItemProps} from './UnderlineNav' export {ActionBar} from './ActionBar' export type {ActionBarProps} from './ActionBar' -export * from './KeybindingHint' - export {Stack} from './Stack' export type {StackProps, StackItemProps} from './Stack' From 253b9b3aef88ebc439024fe666f3341445eec959 Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Tue, 6 Aug 2024 16:28:07 +0000 Subject: [PATCH 14/21] Separate out `features` stories and add `onEmphasis` story --- .../KeybindingHint.features.stories.tsx | 28 +++++++++++++++++++ .../KeybindingHint/KeybindingHint.stories.tsx | 23 ++------------- .../src/KeybindingHint/KeybindingHint.tsx | 1 - packages/react/src/KeybindingHint/index.ts | 2 ++ 4 files changed, 32 insertions(+), 22 deletions(-) create mode 100644 packages/react/src/KeybindingHint/KeybindingHint.features.stories.tsx diff --git a/packages/react/src/KeybindingHint/KeybindingHint.features.stories.tsx b/packages/react/src/KeybindingHint/KeybindingHint.features.stories.tsx new file mode 100644 index 00000000000..52a34cf9082 --- /dev/null +++ b/packages/react/src/KeybindingHint/KeybindingHint.features.stories.tsx @@ -0,0 +1,28 @@ +import React from 'react' +import type {Meta, StoryFn} from '@storybook/react' +import {KeybindingHint, type KeybindingHintProps} from '.' +import Box from '../Box' + +export default { + title: 'Components/KeybindingHint/Features', + component: KeybindingHint, +} satisfies Meta + +const chord = 'Mod+Shift+K' + +export const Condensed = {args: {keys: chord}} + +export const Full = {args: {keys: chord, format: 'full'}} + +const sequence = 'Mod+x y z' + +export const SequenceCondensed = {args: {keys: sequence}} + +export const SequenceFull = {args: {keys: sequence, format: 'full'}} + +export const OnEmphasis: StoryFn = args => ( + + + +) +OnEmphasis.args = {keys: chord, variant: 'onEmphasis'} diff --git a/packages/react/src/KeybindingHint/KeybindingHint.stories.tsx b/packages/react/src/KeybindingHint/KeybindingHint.stories.tsx index fe5ebd59685..71869983bfa 100644 --- a/packages/react/src/KeybindingHint/KeybindingHint.stories.tsx +++ b/packages/react/src/KeybindingHint/KeybindingHint.stories.tsx @@ -1,28 +1,9 @@ -import type React from 'react' import type {Meta} from '@storybook/react' import {KeybindingHint} from './KeybindingHint' -type KeybindingHintProps = React.ComponentProps - -const args = { - keys: 'Mod+Shift+K', -} satisfies KeybindingHintProps - -const meta = { +export default { title: 'Components/KeybindingHint', component: KeybindingHint, } satisfies Meta -export default meta - -export const Condensed = {args} - -export const Full = {args: {...args, format: 'full'}} - -const sequenceArgs = { - keys: 'Mod+x y z', -} satisfies KeybindingHintProps - -export const SequenceCondensed = {args: sequenceArgs} - -export const SequenceFull = {args: {...sequenceArgs, format: 'full'}} +export const Default = {args: {keys: 'Mod+Shift+K'}} diff --git a/packages/react/src/KeybindingHint/KeybindingHint.tsx b/packages/react/src/KeybindingHint/KeybindingHint.tsx index 534f6665991..ef835e8439f 100644 --- a/packages/react/src/KeybindingHint/KeybindingHint.tsx +++ b/packages/react/src/KeybindingHint/KeybindingHint.tsx @@ -1,7 +1,6 @@ import React, {type ReactNode} from 'react' import {memo} from 'react' import Text from '../Text' - import type {KeybindingHintProps} from './props' import {accessibleSequenceString, Sequence} from './components/Sequence' diff --git a/packages/react/src/KeybindingHint/index.ts b/packages/react/src/KeybindingHint/index.ts index 93d08a8257d..dc5f4cb31d5 100644 --- a/packages/react/src/KeybindingHint/index.ts +++ b/packages/react/src/KeybindingHint/index.ts @@ -1 +1,3 @@ export * from './KeybindingHint' + +export type {KeybindingHintProps} from './props' From acbd25b70501a7c62276d2d3a59100281abcae27 Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Tue, 6 Aug 2024 16:43:36 +0000 Subject: [PATCH 15/21] Add examples stories --- .../KeybindingHint.examples.stories.tsx | 64 +++++++++++++++++++ .../KeybindingHint.features.stories.tsx | 16 +++-- 2 files changed, 73 insertions(+), 7 deletions(-) create mode 100644 packages/react/src/KeybindingHint/KeybindingHint.examples.stories.tsx diff --git a/packages/react/src/KeybindingHint/KeybindingHint.examples.stories.tsx b/packages/react/src/KeybindingHint/KeybindingHint.examples.stories.tsx new file mode 100644 index 00000000000..fde9eb9a65a --- /dev/null +++ b/packages/react/src/KeybindingHint/KeybindingHint.examples.stories.tsx @@ -0,0 +1,64 @@ +import React from 'react' +import type {Meta, StoryObj} from '@storybook/react' +import {KeybindingHint, type KeybindingHintProps} from '.' +import {Button, ActionList, FormControl, TextInput} from '..' + +export default { + title: 'Components/KeybindingHint/Examples', + component: KeybindingHint, +} satisfies Meta + +export const ButtonExample: StoryObj = { + render: args => , + args: {keys: 'g p'}, + name: 'Button', +} + +export const PrimaryButton: StoryObj = { + render: args => ( + + ), + args: {keys: 'Mod+Enter', variant: 'onEmphasis'}, +} + +export const ActionListExample: StoryObj = { + render: args => ( + + Add comment + + Copy text{' '} + + + + + Cancel + + ), + args: {keys: 'Mod+c'}, + name: 'ActionList', +} + +export const Prose: StoryObj = { + render: args => ( +

+ Press to toggle between write and preview modes. +

+ ), + args: { + keys: 'Mod+Shift+P', + format: 'full', + }, +} + +export const TextInputExample: StoryObj = { + render: args => ( + + Search + } placeholder="Search" /> + + ), + args: {keys: '/'}, + name: 'TextInput', +} diff --git a/packages/react/src/KeybindingHint/KeybindingHint.features.stories.tsx b/packages/react/src/KeybindingHint/KeybindingHint.features.stories.tsx index 52a34cf9082..135f8087f58 100644 --- a/packages/react/src/KeybindingHint/KeybindingHint.features.stories.tsx +++ b/packages/react/src/KeybindingHint/KeybindingHint.features.stories.tsx @@ -1,5 +1,5 @@ import React from 'react' -import type {Meta, StoryFn} from '@storybook/react' +import type {Meta, StoryObj} from '@storybook/react' import {KeybindingHint, type KeybindingHintProps} from '.' import Box from '../Box' @@ -20,9 +20,11 @@ export const SequenceCondensed = {args: {keys: sequence}} export const SequenceFull = {args: {keys: sequence, format: 'full'}} -export const OnEmphasis: StoryFn = args => ( - - - -) -OnEmphasis.args = {keys: chord, variant: 'onEmphasis'} +export const OnEmphasis: StoryObj = { + render: args => ( + + + + ), + args: {keys: chord, variant: 'onEmphasis'}, +} From 0562fa253708c977bd2229cb7f0ddb00bebd5e9d Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Tue, 6 Aug 2024 16:45:23 +0000 Subject: [PATCH 16/21] Tweak styles --- packages/react/src/KeybindingHint/KeybindingHint.tsx | 1 + packages/react/src/KeybindingHint/components/Chord.tsx | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/react/src/KeybindingHint/KeybindingHint.tsx b/packages/react/src/KeybindingHint/KeybindingHint.tsx index ef835e8439f..82338b748ca 100644 --- a/packages/react/src/KeybindingHint/KeybindingHint.tsx +++ b/packages/react/src/KeybindingHint/KeybindingHint.tsx @@ -20,6 +20,7 @@ const Kbd = ({children}: {children: ReactNode}) => ( position: 'relative', overflow: 'visible', verticalAlign: 'baseline', + textWrap: 'nowrap', }} > {children} diff --git a/packages/react/src/KeybindingHint/components/Chord.tsx b/packages/react/src/KeybindingHint/components/Chord.tsx index 9bc7dbcde05..d12ce8f44ff 100644 --- a/packages/react/src/KeybindingHint/components/Chord.tsx +++ b/packages/react/src/KeybindingHint/components/Chord.tsx @@ -33,7 +33,7 @@ export const Chord = ({keys, format = 'condensed', variant = 'normal'}: Keybindi Date: Tue, 6 Aug 2024 16:56:28 +0000 Subject: [PATCH 17/21] Remove comma between chords --- packages/react/src/KeybindingHint/components/Sequence.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react/src/KeybindingHint/components/Sequence.tsx b/packages/react/src/KeybindingHint/components/Sequence.tsx index ce4dd5297f1..aaa4ba00f0f 100644 --- a/packages/react/src/KeybindingHint/components/Sequence.tsx +++ b/packages/react/src/KeybindingHint/components/Sequence.tsx @@ -12,7 +12,7 @@ export const Sequence = ({keys, format = 'condensed', variant = 'normal'}: Keybi // Since we audibly separate individual keys in chord with space, we need some other separator for chords in a sequence i > 0 && ( <> - , then{' '} + then{' '} ) } @@ -24,4 +24,4 @@ export const Sequence = ({keys, format = 'condensed', variant = 'normal'}: Keybi export const accessibleSequenceString = (sequence: string, isMacOS: boolean) => splitSequence(sequence) .map(chord => accessibleChordString(chord, isMacOS)) - .join(', then ') + .join(' then ') From 6d0aedf82efba64854d09c027c5c85b5fd2a260c Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Tue, 6 Aug 2024 17:17:35 +0000 Subject: [PATCH 18/21] Update import in docs --- docs/content/KeybindingHint.mdx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/content/KeybindingHint.mdx b/docs/content/KeybindingHint.mdx index 6b2a171958c..154e79fe0fb 100644 --- a/docs/content/KeybindingHint.mdx +++ b/docs/content/KeybindingHint.mdx @@ -8,7 +8,8 @@ description: Indicates the presence of a keybinding available for an action. --- import data from '../../packages/react/src/KeybindingHint/KeybindingHint.docs.json' -import {ActionList, KeybindingHint, Button, Text, Box} from '@primer/react' +import {ActionList, Button, Text, Box} from '@primer/react' +import {KeybindingHint} from "@primer/react/drafts" import {TrashIcon} from '@primer/octicons-react' Use `KeybindingHint` to make keyboard shortcuts discoverable. Can render visual keybinding hints in condensed (abbreviated) form or expanded form, and provides accessible alternative text for screen reader users. @@ -39,6 +40,10 @@ Use `KeybindingHint` to make keyboard shortcuts discoverable. Can render visual +```js +import {KeybindingHint} from "@primer/react/drafts" +``` + ## Examples ### Single keys From 7ca0d772b7a5732d6ca07c90c36874332fdae587 Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Fri, 9 Aug 2024 14:45:02 +0000 Subject: [PATCH 19/21] Form & update tests --- docs/content/KeybindingHint.mdx | 4 ++-- packages/react/src/__tests__/KeybindingHint.test.tsx | 4 ++-- .../src/__tests__/__snapshots__/exports.test.ts.snap | 8 ++++++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/docs/content/KeybindingHint.mdx b/docs/content/KeybindingHint.mdx index 154e79fe0fb..a1e4bdd9080 100644 --- a/docs/content/KeybindingHint.mdx +++ b/docs/content/KeybindingHint.mdx @@ -9,7 +9,7 @@ description: Indicates the presence of a keybinding available for an action. import data from '../../packages/react/src/KeybindingHint/KeybindingHint.docs.json' import {ActionList, Button, Text, Box} from '@primer/react' -import {KeybindingHint} from "@primer/react/drafts" +import {KeybindingHint} from '@primer/react/drafts' import {TrashIcon} from '@primer/octicons-react' Use `KeybindingHint` to make keyboard shortcuts discoverable. Can render visual keybinding hints in condensed (abbreviated) form or expanded form, and provides accessible alternative text for screen reader users. @@ -41,7 +41,7 @@ Use `KeybindingHint` to make keyboard shortcuts discoverable. Can render visual ```js -import {KeybindingHint} from "@primer/react/drafts" +import {KeybindingHint} from '@primer/react/drafts' ``` ## Examples diff --git a/packages/react/src/__tests__/KeybindingHint.test.tsx b/packages/react/src/__tests__/KeybindingHint.test.tsx index bee45ff08e5..79281964828 100644 --- a/packages/react/src/__tests__/KeybindingHint.test.tsx +++ b/packages/react/src/__tests__/KeybindingHint.test.tsx @@ -70,7 +70,7 @@ describe('KeybindingHint', () => { it('renders sequences separated by hidden "then"', () => { render() - const el = screen.getByText(', then') + const el = screen.getByText('then') expect(el).toBeInTheDocument() expect(el).not.toHaveAttribute('aria-hidden') }) @@ -87,7 +87,7 @@ describe('getAccessibleKeybindingHintString', () => { expect(getAccessibleKeybindingHintString('Alt+Shift+Command+%', false)).toBe('alt shift command percent')) it('joins chords in a sequence with "then"', () => - expect(getAccessibleKeybindingHintString('Alt+9 x y', false)).toBe('alt 9, then x, then y')) + expect(getAccessibleKeybindingHintString('Alt+9 x y', false)).toBe('alt 9 then x then y')) it('returns "command" for "mod" on MacOS', () => expect(getAccessibleKeybindingHintString('Mod+x', true)).toBe('command x')) diff --git a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap index 61e2092b05b..c8f7f5df416 100644 --- a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap +++ b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap @@ -71,7 +71,6 @@ exports[`@primer/react should not update exports without a semver change 1`] = ` "type FocusTrapHookSettings", "type FocusZoneHookSettings", "FormControl", - "getAccessibleKeybindingHintString", "Header", "type HeaderItemProps", "type HeaderLinkProps", @@ -81,7 +80,6 @@ exports[`@primer/react should not update exports without a semver change 1`] = ` "IconButton", "type IconButtonProps", "IssueLabelToken", - "KeybindingHint", "Label", "LabelGroup", "type LabelGroupProps", @@ -291,6 +289,7 @@ exports[`@primer/react/drafts should not update exports without a semver change "type Emoji", "type FileType", "type FileUploadResult", + "getAccessibleKeybindingHintString", "Hidden", "type HiddenProps", "InlineAutocomplete", @@ -298,6 +297,8 @@ exports[`@primer/react/drafts should not update exports without a semver change "InlineMessage", "type InlineMessageProps", "type InteractiveMarkdownViewerProps", + "KeybindingHint", + "type KeybindingHintProps", "MarkdownEditor", "type MarkdownEditorHandle", "type MarkdownEditorProps", @@ -407,6 +408,7 @@ exports[`@primer/react/experimental should not update exports without a semver c "type FeatureFlagsProps", "type FileType", "type FileUploadResult", + "getAccessibleKeybindingHintString", "Hidden", "type HiddenProps", "InlineAutocomplete", @@ -414,6 +416,8 @@ exports[`@primer/react/experimental should not update exports without a semver c "InlineMessage", "type InlineMessageProps", "type InteractiveMarkdownViewerProps", + "KeybindingHint", + "type KeybindingHintProps", "MarkdownEditor", "type MarkdownEditorHandle", "type MarkdownEditorProps", From c03d046cd33df17f1e6b8b8895d40603491c2f30 Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Mon, 12 Aug 2024 15:12:07 +0000 Subject: [PATCH 20/21] Update snapshots, again --- packages/react/src/__tests__/__snapshots__/exports.test.ts.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap index 5c440e124ba..e32e439a90a 100644 --- a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap +++ b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap @@ -408,9 +408,9 @@ exports[`@primer/react/experimental should not update exports without a semver c "type FeatureFlagsProps", "type FileType", "type FileUploadResult", - "getAccessibleKeybindingHintString", "FilteredActionList", "type FilteredActionListProps", + "getAccessibleKeybindingHintString", "Hidden", "type HiddenProps", "InlineAutocomplete", From c378c1e43711ad6fe987febe08eeddfb2108881a Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Tue, 13 Aug 2024 14:23:38 +0000 Subject: [PATCH 21/21] Move stories to Drafts --- .../src/KeybindingHint/KeybindingHint.examples.stories.tsx | 2 +- .../src/KeybindingHint/KeybindingHint.features.stories.tsx | 2 +- packages/react/src/KeybindingHint/KeybindingHint.stories.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react/src/KeybindingHint/KeybindingHint.examples.stories.tsx b/packages/react/src/KeybindingHint/KeybindingHint.examples.stories.tsx index fde9eb9a65a..f7c152df371 100644 --- a/packages/react/src/KeybindingHint/KeybindingHint.examples.stories.tsx +++ b/packages/react/src/KeybindingHint/KeybindingHint.examples.stories.tsx @@ -4,7 +4,7 @@ import {KeybindingHint, type KeybindingHintProps} from '.' import {Button, ActionList, FormControl, TextInput} from '..' export default { - title: 'Components/KeybindingHint/Examples', + title: 'Drafts/Components/KeybindingHint/Examples', component: KeybindingHint, } satisfies Meta diff --git a/packages/react/src/KeybindingHint/KeybindingHint.features.stories.tsx b/packages/react/src/KeybindingHint/KeybindingHint.features.stories.tsx index 135f8087f58..75448e75e41 100644 --- a/packages/react/src/KeybindingHint/KeybindingHint.features.stories.tsx +++ b/packages/react/src/KeybindingHint/KeybindingHint.features.stories.tsx @@ -4,7 +4,7 @@ import {KeybindingHint, type KeybindingHintProps} from '.' import Box from '../Box' export default { - title: 'Components/KeybindingHint/Features', + title: 'Drafts/Components/KeybindingHint/Features', component: KeybindingHint, } satisfies Meta diff --git a/packages/react/src/KeybindingHint/KeybindingHint.stories.tsx b/packages/react/src/KeybindingHint/KeybindingHint.stories.tsx index 71869983bfa..2452f5be70e 100644 --- a/packages/react/src/KeybindingHint/KeybindingHint.stories.tsx +++ b/packages/react/src/KeybindingHint/KeybindingHint.stories.tsx @@ -2,7 +2,7 @@ import type {Meta} from '@storybook/react' import {KeybindingHint} from './KeybindingHint' export default { - title: 'Components/KeybindingHint', + title: 'Drafts/Components/KeybindingHint', component: KeybindingHint, } satisfies Meta