Skip to content

chore: document useKeyboard shortcuts#10192

Open
snowystinger wants to merge 1 commit into
mainfrom
shortcuts-documentation
Open

chore: document useKeyboard shortcuts#10192
snowystinger wants to merge 1 commit into
mainfrom
shortcuts-documentation

Conversation

@snowystinger

Copy link
Copy Markdown
Member

Closes

Also swaps the order of props.onKeyDown and our shortcut handler so that in the future it'll be possible to prevent the aria defaults.
Note this came with a change that I needed to pass the shortcuts object to a menu item, not just the handlers, otherwise continuePropagation was being called when it shouldn't be.

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

@rspbot

rspbot commented Jun 12, 2026

Copy link
Copy Markdown

@rspbot

rspbot commented Jun 12, 2026

Copy link
Copy Markdown
## API Changes

@react-aria/menu

/@react-aria/menu:AriaMenuItemProps

 AriaMenuItemProps {
   aria-controls?: string
   aria-describedby?: string
   aria-expanded?: boolean | 'true' | 'false'
   aria-haspopup?: 'menu' | 'dialog'
   aria-label?: string
   id?: string
   isVirtualized?: boolean
   key: Key
   onBlur?: (FocusEvent<Target>) => void
   onClick?: (MouseEvent<FocusableElement>) => void
   onFocus?: (FocusEvent<Target>) => void
   onFocusChange?: (boolean) => void
   onHoverChange?: (boolean) => void
   onHoverEnd?: (HoverEvent) => void
   onHoverStart?: (HoverEvent) => void
   onKeyDown?: (KeyboardEvent) => void
   onKeyUp?: (KeyboardEvent) => void
   onPress?: (PressEvent) => void
   onPressChange?: (boolean) => void
   onPressEnd?: (PressEvent) => void
   onPressStart?: (PressEvent) => void
   onPressUp?: (PressEvent) => void
   selectionManager?: SelectionManager
+  shortcuts?: KeyboardShortcutBindings
   shouldCloseOnSelect?: boolean
 }

@rspbot

rspbot commented Jun 12, 2026

Copy link
Copy Markdown

Agent Skills Changes

Modified (2)
Install

React Spectrum S2:

npx skills add https://d1pzu54gtk2aed.cloudfront.net/pr/68fb7959d7b69a8683241d8dc3a0079d84131545/

React Aria:

npx skills add https://d5iwopk28bdhl.cloudfront.net/pr/68fb7959d7b69a8683241d8dc3a0079d84131545/

selectionManager?: SelectionManager;

/** Keyboard shortcuts to handle. */
shortcuts?: KeyboardShortcutBindings;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

probably shouldn't add this as a prop

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, i was unsure the best way to handle this. The issue is that if shortcuts doesn't handle it, we automatically call continuePropagation on it. The issue though is that useMenuItem and useSubmenuTrigger are attaching keyboard handlers to the same element, through the same useKeyboard because we pass onKeyDown/onKeyUp straight through on the props.
I'll try something new on monday to see if i can undo this.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yeah fwiw daniel ran into the same thing in combobox I think, one of the reasons we reverted for this release. he had some ideas but ultimately it seemed too risky for now

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yeah, i chatted with him about it, makes sense

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants