chore: expand support for custom trees#7817
Conversation
…eeitem' and 'tree'
🦋 Changeset detectedLatest commit: de4b18a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
…ct into chore/expand-actionlist-roles
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/20288 |
joshblack
left a comment
There was a problem hiding this comment.
Could we add tests/stories for the new functionality?
There was a problem hiding this comment.
Pull request overview
This PR expands support for tree-like structures by broadening ActionList’s supported ARIA roles and by making TreeView’s roving-tabindex behavior reusable via a new public useRovingTabIndex export.
Changes:
- Expanded
ActionList.Itemrole handling to includetreeitemandtree. - Enhanced
useRovingTabIndexwith opt-in configuration (e.g. wrap-around, preventScroll, focus-out behavior) and exported it from the package entrypoint. - Exposed
ActionListContainerContextviaActionList.ContainerContextand added a changeset for a minor release.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/TreeView/useRovingTabIndex.ts | Extends roving tabindex logic with new options (wrap-around, focus-out behavior, preventScroll) and improved element resolution. |
| packages/react/src/index.ts | Re-exports useRovingTabIndex from the public API. |
| packages/react/src/ActionList/Item.tsx | Expands selectable/list role allowlists to include treeitem/tree. |
| packages/react/src/ActionList/index.ts | Adds ActionList.ContainerContext static for advanced composition use cases. |
| packages/react/src/tests/snapshots/exports.test.ts.snap | Updates export snapshot to include useRovingTabIndex. |
| .changeset/expand-actionlist-roles.md | Declares a minor bump and describes the new exports/role support. |
Copilot's findings
- Files reviewed: 6/6 changed files
- Comments generated: 3
| export function useRovingTabIndex( | ||
| { | ||
| containerRef, | ||
| bindKeys: | ||
| FocusKeys.ArrowVertical | | ||
| FocusKeys.ArrowHorizontal | | ||
| FocusKeys.HomeAndEnd | | ||
| FocusKeys.Backspace | | ||
| FocusKeys.PageUpDown, | ||
| preventScroll: true, | ||
| getNextFocusable: (direction, from, event) => { | ||
| if (!(from instanceof HTMLElement)) return | ||
|
|
||
| // Skip elements within a modal dialog | ||
| // This need to be in a try/catch to avoid errors in | ||
| // non-supported browsers | ||
| try { | ||
| if (from.closest('dialog:modal')) { | ||
| return | ||
| mouseDownRef, | ||
| focusOutBehavior, | ||
| preventScroll = true, | ||
| wrapAround = false, | ||
| }: { | ||
| containerRef: React.RefObject<HTMLElement | null> | ||
| mouseDownRef?: React.RefObject<boolean> | ||
| preventScroll?: boolean | ||
| focusOutBehavior?: 'stop' | 'wrap' | ||
| wrapAround?: boolean | ||
| }, | ||
| dependencies: React.DependencyList = [], | ||
| ) { |
There was a problem hiding this comment.
Updated in 5ba0289. I changed useRovingTabIndex to pass [preventScroll, focusOutBehavior, wrapAround, ...dependencies] into useFocusZone, so those options now re-initialize the focus zone when they change while still allowing additional caller-provided dependencies.
…ct into chore/expand-actionlist-roles
|
@joshblack added tests in ActionList that test the selection part when role="tree", and added a test file to useRovingTabIndex with general tests as well as the new opt-in features. I don't think it's worth the effort // we get much value out of creating stories for "ActionList as tree" here. Can definitely add if you feel otherwise! |
|
@francinelucca my thought is that functionality that folks can use should be documented in storybook so that they can discover it either in storybook or through the docs site. Let me know what you think 👀 |
|
🤖 Lint issues have been automatically fixed and committed to this PR. |
Co-authored-by: francinelucca <40550942+francinelucca@users.noreply.github.com>
…ct into chore/expand-actionlist-roles
Uh oh!
There was an error while loading. Please reload this page.