From 997d1522b8ce4b9e90c06cfe85dad4acdabf3c6a Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Thu, 22 May 2025 08:17:00 +1000 Subject: [PATCH 1/5] fix: TreeView inadvertent focus trap --- packages/@react-aria/tree/src/useTreeItem.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@react-aria/tree/src/useTreeItem.ts b/packages/@react-aria/tree/src/useTreeItem.ts index b94313429c1..0608da863d6 100644 --- a/packages/@react-aria/tree/src/useTreeItem.ts +++ b/packages/@react-aria/tree/src/useTreeItem.ts @@ -15,8 +15,8 @@ import {AriaGridListItemOptions, GridListItemAria, useGridListItem} from '@react import {DOMAttributes, FocusableElement, Node, RefObject} from '@react-types/shared'; // @ts-ignore import intlMessages from '../intl/*.json'; -import {isAndroid, useLabels} from '@react-aria/utils'; import {TreeState} from '@react-stately/tree'; +import {useLabels} from '@react-aria/utils'; import {useLocalizedStringFormatter} from '@react-aria/i18n'; export interface AriaTreeItemOptions extends Omit { @@ -59,7 +59,7 @@ export function useTreeItem(props: AriaTreeItemOptions, state: TreeState, state.selectionManager.setFocusedKey(node.key); } }, - tabIndex: isAndroid() ? -1 : null, + excludeFromTabOrder: true, 'data-react-aria-prevent-focus': true, ...labelProps }; From 09ec7c27f4c97351fc62c12e13a43974a711a945 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Tue, 3 Jun 2025 06:33:24 +1000 Subject: [PATCH 2/5] add test stories --- .../s2/stories/TreeView.stories.tsx | 56 ++++++++ .../tree/stories/TreeView.stories.tsx | 71 ++++++++++ .../stories/Tree.stories.tsx | 126 +++++++++++++++++- 3 files changed, 247 insertions(+), 6 deletions(-) diff --git a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx index f514dbd80bd..12be6e18f43 100644 --- a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx +++ b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx @@ -185,6 +185,62 @@ export const Example = { } }; +const TreeExampleStaticNoActions = (args) => ( +
+ + + + Photos + + + + + + Projects + + + + + Projects-1 + + + + + Projects-1A + + + + + + + Projects-2 + + + + + + Projects-3 + + + + + +
+); + +export const ExampleNoActions = { + render: TreeExampleStaticNoActions, + args: { + selectionMode: 'multiple' + } +}; + + let rows = [ {id: 'projects', name: 'Projects', icon: , childItems: [ {id: 'project-1', name: 'Project 1 Level 1', icon: }, diff --git a/packages/@react-spectrum/tree/stories/TreeView.stories.tsx b/packages/@react-spectrum/tree/stories/TreeView.stories.tsx index 466da584255..40646928174 100644 --- a/packages/@react-spectrum/tree/stories/TreeView.stories.tsx +++ b/packages/@react-spectrum/tree/stories/TreeView.stories.tsx @@ -174,6 +174,77 @@ TreeExampleStatic.story = { } }; +export const TreeExampleStaticNoActions = (args: SpectrumTreeViewProps) => ( +
+ + + + Photos + + + + + + Projects + + + + + Projects-1 + + + + + Projects-1A + + + + + + + Projects-2 + + + + + + Projects-3 + + + + + +
+); + +export const ExampleNoActions = { + render: TreeExampleStaticNoActions, + args: { + selectionMode: 'none', + selectionStyle: 'checkbox', + disabledBehavior: 'selection' + }, + argTypes: { + selectionMode: { + control: 'radio', + options: ['none', 'single', 'multiple'] + }, + selectionStyle: { + control: 'radio', + options: ['checkbox', 'highlight'] + }, + disabledBehavior: { + control: 'radio', + options: ['selection', 'all'] + }, + disallowEmptySelection: { + control: { + type: 'boolean' + } + } + } +}; + let rows = [ {id: 'projects', name: 'Projects', icon: , childItems: [ {id: 'project-1', name: 'Project 1', icon: }, diff --git a/packages/react-aria-components/stories/Tree.stories.tsx b/packages/react-aria-components/stories/Tree.stories.tsx index 8c6e94b1c1a..d821fc062d4 100644 --- a/packages/react-aria-components/stories/Tree.stories.tsx +++ b/packages/react-aria-components/stories/Tree.stories.tsx @@ -100,6 +100,44 @@ const StaticTreeItem = (props: StaticTreeItemProps) => { ); }; +const StaticTreeItemNoActions = (props: StaticTreeItemProps) => { + return ( + classNames(styles, 'tree-item', { + focused: isFocused, + 'focus-visible': isFocusVisible, + selected: isSelected, + hovered: isHovered + })}> + + {({isExpanded, hasChildItems, level, selectionMode, selectionBehavior}) => ( + <> + {selectionMode !== 'none' && selectionBehavior === 'toggle' && ( + + )} +
+ {hasChildItems && ( + + )} + {props.title || props.children} +
+ + )} + + {props.title && props.children} + + ); +}; + const TreeExampleStaticRender = (args) => ( Photos @@ -147,6 +185,53 @@ const TreeExampleStaticRender = (args) => ( ); +const TreeExampleStaticNoActionsRender = (args) => ( + + Photos + + + + Projects-1A + + + + Projects-2 + + + Projects-3 + + + classNames(styles, 'tree-item', { + focused: isFocused, + 'focus-visible': isFocusVisible, + selected: isSelected, + hovered: isHovered + })}> + + Reports + + + classNames(styles, 'tree-item', { + focused: isFocused, + 'focus-visible': isFocusVisible, + selected: isSelected, + hovered: isHovered + })}> + + {({isFocused}) => ( + {`${isFocused} Tests`} + )} + + + +); + export const TreeExampleStatic = { render: TreeExampleStaticRender, args: { @@ -176,6 +261,35 @@ export const TreeExampleStatic = { } }; +export const TreeExampleStaticNoActions = { + render: TreeExampleStaticNoActionsRender, + args: { + selectionMode: 'none', + selectionBehavior: 'toggle', + disabledBehavior: 'selection', + disallowClearAll: false + }, + argTypes: { + selectionMode: { + control: 'radio', + options: ['none', 'single', 'multiple'] + }, + selectionBehavior: { + control: 'radio', + options: ['toggle', 'replace'] + }, + disabledBehavior: { + control: 'radio', + options: ['selection', 'all'] + } + }, + parameters: { + description: { + data: 'Note that the last two items are just to test bare minimum TreeItem and thus dont have the checkbox or any of the other contents that the other items have. The last item tests the isFocused renderProp. This story specifically tests tab behaviour when there are no additional actions in the tree.' + } + } +}; + let rows = [ {id: 'projects', name: 'Projects', childItems: [ {id: 'project-1', name: 'Project 1'}, @@ -392,7 +506,7 @@ function LoadingStoryDepOnCollection(args) { getKey: item => item.id, getChildren: item => item.childItems }); - + return ( @@ -659,11 +773,11 @@ function SecondTree(args) { }); return ( - 'Drop items here'}> {(item: any) => ( From d31d6f7c42dd3e13bd1ed72a3aeb2e9d0205d433 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Tue, 3 Jun 2025 10:19:26 +1000 Subject: [PATCH 3/5] apply same fix to S2 --- packages/@react-spectrum/s2/src/TreeView.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index 59f4cdfcf32..bd65c40a84f 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -422,8 +422,7 @@ function ExpandableRowChevron(props: ExpandableRowChevronProps) { {...props} ref={ref} slot="chevron" - // Override tabindex so that grid keyboard nav skips over it. Needs -1 so android talkback can actually "focus" it - excludeFromTabOrder={isAndroid() && !isDisabled} + excludeFromTabOrder={!isDisabled} preventFocusOnPress className={renderProps => expandButton({...renderProps, isExpanded, isRTL: direction === 'rtl', scale, isHidden})}> Date: Tue, 3 Jun 2025 10:21:07 +1000 Subject: [PATCH 4/5] fix lint --- packages/@react-spectrum/s2/src/TreeView.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index bd65c40a84f..f792f4dba8e 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -33,7 +33,6 @@ import {colorMix, focusRing, fontRelative, lightDark, style} from '../style' wit import {DOMRef, Key} from '@react-types/shared'; import {getAllowedOverrides, StylesPropWithHeight, UnsafeStyles} from './style-utils' with {type: 'macro'}; import {IconContext} from './Icon'; -import {isAndroid} from '@react-aria/utils'; import {raw} from '../style/style-macro' with {type: 'macro'}; import React, {createContext, forwardRef, JSXElementConstructor, ReactElement, ReactNode, useContext, useRef} from 'react'; import {TextContext} from './Content'; From 13853c0de9459d35c62f08297d69ce6432c39e4f Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Wed, 4 Jun 2025 07:43:17 +1000 Subject: [PATCH 5/5] add prevent focus on press to RAC --- packages/@react-aria/tree/src/useTreeItem.ts | 1 + packages/@react-spectrum/s2/src/TreeView.tsx | 5 +---- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/@react-aria/tree/src/useTreeItem.ts b/packages/@react-aria/tree/src/useTreeItem.ts index 0608da863d6..f16c2d8b43c 100644 --- a/packages/@react-aria/tree/src/useTreeItem.ts +++ b/packages/@react-aria/tree/src/useTreeItem.ts @@ -60,6 +60,7 @@ export function useTreeItem(props: AriaTreeItemOptions, state: TreeState, } }, excludeFromTabOrder: true, + preventFocusOnPress: true, 'data-react-aria-prevent-focus': true, ...labelProps }; diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index f792f4dba8e..9c6062710a9 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -412,17 +412,14 @@ const expandButton = style({ function ExpandableRowChevron(props: ExpandableRowChevronProps) { let expandButtonRef = useRef(null); let [fullProps, ref] = useContextProps({...props, slot: 'chevron'}, expandButtonRef, ButtonContext); - let {isExpanded, isDisabled, scale, isHidden} = fullProps; + let {isExpanded, scale, isHidden} = fullProps; let {direction} = useLocale(); - isDisabled = isDisabled || isHidden; return (