-
Notifications
You must be signed in to change notification settings - Fork 18
feat: more accessible keyboard nav focus outlines for menu items #890
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
35b313d
a35b0b4
88d948c
071093b
d973282
bcbd000
3cc7ace
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@clickhouse/click-ui': patch | ||
| --- | ||
|
|
||
| Add visible keyboard focus ring to menu items (Dropdown, Select, ContextMenu) for WCAG SC 2.4.7 and SC 1.4.11 compliance. Introduces `useInputModality` hook and `stroke.focus` theme tokens. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
| import { Arrow, GenericMenuItem, GenericMenuPanel } from '@/components/GenericMenu'; | ||
| import Popover_Arrow from '@/components/Assets/Icons/Popover-Arrow'; | ||
| import { IconWrapper } from '@/components/IconWrapper/IconWrapper'; | ||
| import { useInputModality } from '@/hooks'; | ||
| import type { ArrowProps, ContextMenuItemProps } from './ContextMenu.types'; | ||
|
|
||
| export const ContextMenu = (props: RightMenu.ContextMenuProps) => ( | ||
|
|
@@ -122,19 +123,21 @@ | |
| showArrow, | ||
| // TODO: remove deprecated side and align | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| side, | ||
|
Check warning on line 126 in src/components/ContextMenu/ContextMenu.tsx
|
||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| align, | ||
|
Check warning on line 128 in src/components/ContextMenu/ContextMenu.tsx
|
||
| ...props | ||
| }: ContextMenuContentProps | ContextMenuSubContentProps) => { | ||
| const ContentElement = sub ? RightMenu.SubContent : RightMenu.Content; | ||
| const inputModalityProps = useInputModality(); | ||
| return ( | ||
| <RightMenu.Portal> | ||
| <RightMenuContent | ||
| {...props} | ||
| $type="context-menu" | ||
| $showArrow={showArrow} | ||
| as={ContentElement} | ||
| {...props} | ||
| {...inputModalityProps} | ||
| > | ||
| {showArrow && ( | ||
| <Arrow | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| export { useUpdateEffect } from './useUpdateEffect'; | ||
| export { useInitialTheme } from './useInitialTheme'; | ||
| export { useInputModality } from './useInputModality'; | ||
| export { useCUITheme } from './useCUITheme'; | ||
| export type { CUIThemeType } from './useCUITheme'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| import { render } from '@testing-library/react'; | ||
| import { fireEvent } from '@testing-library/dom'; | ||
| import { useInputModality } from './useInputModality'; | ||
|
|
||
| function TestContainer() { | ||
| const props = useInputModality(); | ||
| return ( | ||
| <div | ||
| data-testid="container" | ||
| {...props} | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| describe('useInputModality', () => { | ||
| function setup() { | ||
| const { getByTestId } = render(<TestContainer />); | ||
| return getByTestId('container') as HTMLElement; | ||
| } | ||
|
|
||
| it('sets keyboard modality for navigation keys', () => { | ||
| const el = setup(); | ||
| for (const key of ['ArrowUp', 'ArrowDown', 'Home', 'End', 'Enter', 'Tab']) { | ||
| fireEvent.keyDown(el, { key }); | ||
| expect(el.dataset.inputModality).toBe('keyboard'); | ||
| delete el.dataset.inputModality; | ||
| } | ||
| }); | ||
|
|
||
| it('sets keyboard modality for typeahead characters', () => { | ||
| const el = setup(); | ||
| for (const key of ['d', 'a', 'z']) { | ||
| fireEvent.keyDown(el, { key }); | ||
| expect(el.dataset.inputModality).toBe('keyboard'); | ||
| delete el.dataset.inputModality; | ||
| } | ||
| }); | ||
|
|
||
| it('sets keyboard modality for Space', () => { | ||
| const el = setup(); | ||
| fireEvent.keyDown(el, { key: ' ' }); | ||
| expect(el.dataset.inputModality).toBe('keyboard'); | ||
| }); | ||
|
|
||
| it('does not set keyboard modality for bare modifier keys', () => { | ||
| const el = setup(); | ||
| for (const key of ['Meta', 'Control', 'Alt', 'Shift', 'CapsLock']) { | ||
| fireEvent.keyDown(el, { key }); | ||
| expect(el.dataset.inputModality).toBeUndefined(); | ||
| } | ||
| }); | ||
|
|
||
| it('sets pointer modality on pointer move', () => { | ||
| const el = setup(); | ||
| fireEvent.keyDown(el, { key: 'ArrowDown' }); | ||
| expect(el.dataset.inputModality).toBe('keyboard'); | ||
|
|
||
| fireEvent.pointerMove(el); | ||
| expect(el.dataset.inputModality).toBe('pointer'); | ||
| }); | ||
|
|
||
| it('sets pointer modality on pointer down', () => { | ||
| const el = setup(); | ||
| fireEvent.keyDown(el, { key: 'ArrowDown' }); | ||
| expect(el.dataset.inputModality).toBe('keyboard'); | ||
|
|
||
| fireEvent.pointerDown(el); | ||
| expect(el.dataset.inputModality).toBe('pointer'); | ||
| }); | ||
|
|
||
| it('seeds modality from global state on focus after keyboard', () => { | ||
| fireEvent.keyDown(document, { key: 'Enter' }); | ||
|
|
||
| const el = setup(); | ||
| fireEvent.focusIn(el); | ||
|
|
||
| expect(el.dataset.inputModality).toBe('keyboard'); | ||
| }); | ||
|
|
||
| it('seeds pointer modality from global state on focus after pointer', () => { | ||
| fireEvent.pointerDown(document); | ||
|
|
||
| const el = setup(); | ||
| fireEvent.focusIn(el); | ||
|
|
||
| expect(el.dataset.inputModality).toBe('pointer'); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| import { | ||
| type FocusEvent, | ||
| type KeyboardEvent, | ||
| type PointerEvent, | ||
| useCallback, | ||
| } from 'react'; | ||
|
|
||
| const MODIFIER_KEYS = new Set(['Meta', 'Control', 'Alt', 'Shift', 'CapsLock']); | ||
|
|
||
| /** | ||
| * WCAG SC 2.4.7 requires a visible keyboard focus indicator, but hover | ||
| * styling is not mandated and adding a focus ring on hover would be | ||
| * visually disruptive. Radix uses a single `data-highlighted` attribute | ||
| * for both hover and keyboard focus, so we track input modality on the | ||
| * container to let CSS distinguish the two. | ||
| * | ||
| * Spread the returned props onto a menu container element. | ||
| */ | ||
|
|
||
| // Global modality tracks the last input method so that menus opened via | ||
| // keyboard can seed their container's data attribute before any per-container | ||
| // event fires (the triggering keydown happens on the trigger, not the content). | ||
| let lastGlobalModality: 'keyboard' | 'pointer' = 'pointer'; | ||
|
|
||
| if (typeof document !== 'undefined') { | ||
| document.addEventListener( | ||
| 'keydown', | ||
| (e: globalThis.KeyboardEvent) => { | ||
| if (!MODIFIER_KEYS.has(e.key)) { | ||
| lastGlobalModality = 'keyboard'; | ||
| } | ||
| }, | ||
| true | ||
| ); | ||
| document.addEventListener( | ||
| 'pointerdown', | ||
| () => { | ||
| lastGlobalModality = 'pointer'; | ||
| }, | ||
| true | ||
| ); | ||
| document.addEventListener( | ||
| 'pointermove', | ||
| () => { | ||
| lastGlobalModality = 'pointer'; | ||
| }, | ||
| true | ||
| ); | ||
| } | ||
|
|
||
| export function useInputModality() { | ||
| const onKeyDownCapture = useCallback((e: KeyboardEvent<HTMLElement>) => { | ||
| if (!MODIFIER_KEYS.has(e.key)) { | ||
| e.currentTarget.dataset.inputModality = 'keyboard'; | ||
| } | ||
| }, []); | ||
|
|
||
| const onPointerMove = useCallback((e: PointerEvent<HTMLElement>) => { | ||
| e.currentTarget.dataset.inputModality = 'pointer'; | ||
| }, []); | ||
|
dustinhealy marked this conversation as resolved.
|
||
|
|
||
| const onPointerDown = useCallback((e: PointerEvent<HTMLElement>) => { | ||
| e.currentTarget.dataset.inputModality = 'pointer'; | ||
| }, []); | ||
|
|
||
| const onFocusCapture = useCallback((e: FocusEvent<HTMLElement>) => { | ||
| e.currentTarget.dataset.inputModality = lastGlobalModality; | ||
| }, []); | ||
|
|
||
| return { onKeyDownCapture, onPointerMove, onPointerDown, onFocusCapture }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2033,6 +2033,7 @@ const theme = { | |
| }, | ||
| stroke: { | ||
| default: '#e6e7e9', | ||
| focus: '#437eef', | ||
| }, | ||
| }, | ||
| format: { | ||
|
|
@@ -2060,6 +2061,10 @@ const theme = { | |
| active: 'rgb(100% 13.725% 13.725% / 0.3)', | ||
| disabled: '#ffffff', | ||
| }, | ||
| stroke: { | ||
| default: 'rgba(0, 0, 0, 0)', | ||
| focus: '#437eef', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dustinhealy I missed this one. For tokens, we have to update them in the Figma file. The process will be revised soon to avoid confusion, but just for your interest, as it seems these values were hard typed, so lost on the latest.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good to know - thanks Helder. In the meantime, feel free to let me know if I need to make any changes / open a new PR to account for this or anything else.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dustinhealy Not really, did a quick tweak for now #810 (started migrating out from styled components). Once I get the semantic versions for the tokens to reduce the file sizes and make it simpler, I'll tag you. Thanks! |
||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.