Skip to content

Commit db1b174

Browse files
snowystingersnowystinger
authored
fix: TreeView inadvertent focus trap (#8277)
* fix: TreeView inadvertent focus trap * add test stories * apply same fix to S2 * fix lint * add prevent focus on press to RAC
1 parent 61f6f42 commit db1b174

File tree

5 files changed

+251
-14
lines changed

5 files changed

+251
-14
lines changed

‎packages/@react-aria/tree/src/useTreeItem.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ import {AriaGridListItemOptions, GridListItemAria, useGridListItem} from '@react
1515
import {DOMAttributes, FocusableElement, Node, RefObject} from '@react-types/shared';
1616
// @ts-ignore
1717
import intlMessages from '../intl/*.json';
18-
import {isAndroid, useLabels} from '@react-aria/utils';
1918
import {TreeState} from '@react-stately/tree';
19+
import {useLabels} from '@react-aria/utils';
2020
import {useLocalizedStringFormatter} from '@react-aria/i18n';
2121

2222
export interface AriaTreeItemOptions extends Omit<AriaGridListItemOptions, 'isVirtualized'> {
@@ -59,7 +59,8 @@ export function useTreeItem<T>(props: AriaTreeItemOptions, state: TreeState<T>,
5959
state.selectionManager.setFocusedKey(node.key);
6060
}
6161
},
62-
tabIndex: isAndroid() ? -1 : null,
62+
excludeFromTabOrder: true,
63+
preventFocusOnPress: true,
6364
'data-react-aria-prevent-focus': true,
6465
...labelProps
6566
};

‎packages/@react-spectrum/s2/src/TreeView.tsx

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ import {colorMix, focusRing, fontRelative, lightDark, style} from '../style' wit
3333
import {DOMRef, Key} from '@react-types/shared';
3434
import {getAllowedOverrides, StylesPropWithHeight, UnsafeStyles} from './style-utils' with {type: 'macro'};
3535
import {IconContext} from './Icon';
36-
import {isAndroid} from '@react-aria/utils';
3736
import {raw} from '../style/style-macro' with {type: 'macro'};
3837
import React, {createContext, forwardRef, JSXElementConstructor, ReactElement, ReactNode, useContext, useRef} from 'react';
3938
import {TextContext} from './Content';
@@ -413,18 +412,14 @@ const expandButton = style<ExpandableRowChevronProps>({
413412
function ExpandableRowChevron(props: ExpandableRowChevronProps) {
414413
let expandButtonRef = useRef<HTMLButtonElement>(null);
415414
let [fullProps, ref] = useContextProps({...props, slot: 'chevron'}, expandButtonRef, ButtonContext);
416-
let {isExpanded, isDisabled, scale, isHidden} = fullProps;
415+
let {isExpanded, scale, isHidden} = fullProps;
417416
let {direction} = useLocale();
418-
isDisabled = isDisabled || isHidden;
419417

420418
return (
421419
<Button
422420
{...props}
423421
ref={ref}
424422
slot="chevron"
425-
// Override tabindex so that grid keyboard nav skips over it. Needs -1 so android talkback can actually "focus" it
426-
excludeFromTabOrder={isAndroid() && !isDisabled}
427-
preventFocusOnPress
428423
className={renderProps => expandButton({...renderProps, isExpanded, isRTL: direction === 'rtl', scale, isHidden})}>
429424
<Chevron
430425
className={style({

‎packages/@react-spectrum/s2/stories/TreeView.stories.tsx

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,62 @@ export const Example = {
185185
}
186186
};
187187

188+
const TreeExampleStaticNoActions = (args) => (
189+
<div style={{width: '300px', resize: 'both', height: '320px', overflow: 'auto'}}>
190+
<TreeView
191+
{...args}
192+
disabledKeys={['projects-1']}
193+
aria-label="test static tree"
194+
onExpandedChange={action('onExpandedChange')}
195+
onSelectionChange={action('onSelectionChange')}>
196+
<TreeViewItem id="Photos" textValue="Photos">
197+
<TreeViewItemContent>
198+
<Text>Photos</Text>
199+
<Folder />
200+
</TreeViewItemContent>
201+
</TreeViewItem>
202+
<TreeViewItem id="projects" textValue="Projects">
203+
<TreeViewItemContent>
204+
<Text>Projects</Text>
205+
<Folder />
206+
</TreeViewItemContent>
207+
<TreeViewItem id="projects-1" textValue="Projects-1">
208+
<TreeViewItemContent>
209+
<Text>Projects-1</Text>
210+
<Folder />
211+
</TreeViewItemContent>
212+
<TreeViewItem id="projects-1A" textValue="Projects-1A">
213+
<TreeViewItemContent>
214+
<Text>Projects-1A</Text>
215+
<FileTxt />
216+
</TreeViewItemContent>
217+
</TreeViewItem>
218+
</TreeViewItem>
219+
<TreeViewItem id="projects-2" textValue="Projects-2">
220+
<TreeViewItemContent>
221+
<Text>Projects-2</Text>
222+
<FileTxt />
223+
</TreeViewItemContent>
224+
</TreeViewItem>
225+
<TreeViewItem id="projects-3" textValue="Projects-3">
226+
<TreeViewItemContent>
227+
<Text>Projects-3</Text>
228+
<FileTxt />
229+
</TreeViewItemContent>
230+
</TreeViewItem>
231+
</TreeViewItem>
232+
</TreeView>
233+
</div>
234+
);
235+
236+
export const ExampleNoActions = {
237+
render: TreeExampleStaticNoActions,
238+
args: {
239+
selectionMode: 'multiple'
240+
}
241+
};
242+
243+
188244
let rows = [
189245
{id: 'projects', name: 'Projects', icon: <Folder />, childItems: [
190246
{id: 'project-1', name: 'Project 1 Level 1', icon: <FileTxt />},

‎packages/@react-spectrum/tree/stories/TreeView.stories.tsx

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,77 @@ TreeExampleStatic.story = {
174174
}
175175
};
176176

177+
export const TreeExampleStaticNoActions = (args: SpectrumTreeViewProps<unknown>) => (
178+
<div style={{width: '300px', resize: 'both', height: '90vh', overflow: 'auto'}}>
179+
<TreeView {...args} disabledKeys={['projects-1']} aria-label="test static tree" onExpandedChange={action('onExpandedChange')} onSelectionChange={action('onSelectionChange')}>
180+
<TreeViewItem id="Photos" textValue="Photos">
181+
<TreeViewItemContent>
182+
<Text>Photos</Text>
183+
<Folder />
184+
</TreeViewItemContent>
185+
</TreeViewItem>
186+
<TreeViewItem id="projects" textValue="Projects">
187+
<TreeViewItemContent>
188+
<Text>Projects</Text>
189+
<Folder />
190+
</TreeViewItemContent>
191+
<TreeViewItem id="projects-1" textValue="Projects-1">
192+
<TreeViewItemContent>
193+
<Text>Projects-1</Text>
194+
<Folder />
195+
</TreeViewItemContent>
196+
<TreeViewItem id="projects-1A" textValue="Projects-1A">
197+
<TreeViewItemContent>
198+
<Text>Projects-1A</Text>
199+
<FileTxt />
200+
</TreeViewItemContent>
201+
</TreeViewItem>
202+
</TreeViewItem>
203+
<TreeViewItem id="projects-2" textValue="Projects-2">
204+
<TreeViewItemContent>
205+
<Text>Projects-2</Text>
206+
<FileTxt />
207+
</TreeViewItemContent>
208+
</TreeViewItem>
209+
<TreeViewItem id="projects-3" textValue="Projects-3">
210+
<TreeViewItemContent>
211+
<Text>Projects-3</Text>
212+
<FileTxt />
213+
</TreeViewItemContent>
214+
</TreeViewItem>
215+
</TreeViewItem>
216+
</TreeView>
217+
</div>
218+
);
219+
220+
export const ExampleNoActions = {
221+
render: TreeExampleStaticNoActions,
222+
args: {
223+
selectionMode: 'none',
224+
selectionStyle: 'checkbox',
225+
disabledBehavior: 'selection'
226+
},
227+
argTypes: {
228+
selectionMode: {
229+
control: 'radio',
230+
options: ['none', 'single', 'multiple']
231+
},
232+
selectionStyle: {
233+
control: 'radio',
234+
options: ['checkbox', 'highlight']
235+
},
236+
disabledBehavior: {
237+
control: 'radio',
238+
options: ['selection', 'all']
239+
},
240+
disallowEmptySelection: {
241+
control: {
242+
type: 'boolean'
243+
}
244+
}
245+
}
246+
};
247+
177248
let rows = [
178249
{id: 'projects', name: 'Projects', icon: <Folder />, childItems: [
179250
{id: 'project-1', name: 'Project 1', icon: <FileTxt />},

‎packages/react-aria-components/stories/Tree.stories.tsx

Lines changed: 120 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,44 @@ const StaticTreeItem = (props: StaticTreeItemProps) => {
100100
);
101101
};
102102

103+
const StaticTreeItemNoActions = (props: StaticTreeItemProps) => {
104+
return (
105+
<TreeItem
106+
{...props}
107+
className={({isFocused, isSelected, isHovered, isFocusVisible}) => classNames(styles, 'tree-item', {
108+
focused: isFocused,
109+
'focus-visible': isFocusVisible,
110+
selected: isSelected,
111+
hovered: isHovered
112+
})}>
113+
<TreeItemContent>
114+
{({isExpanded, hasChildItems, level, selectionMode, selectionBehavior}) => (
115+
<>
116+
{selectionMode !== 'none' && selectionBehavior === 'toggle' && (
117+
<MyCheckbox slot="selection" />
118+
)}
119+
<div
120+
className={classNames(styles, 'content-wrapper')}
121+
style={{marginInlineStart: `${(!hasChildItems ? 20 : 0) + (level - 1) * 15}px`}}>
122+
{hasChildItems && (
123+
<Button className={styles.chevron} slot="chevron">
124+
<div style={{transform: `rotate(${isExpanded ? 90 : 0}deg)`, width: '16px', height: '16px'}}>
125+
<svg viewBox="0 0 24 24" style={{width: '16px', height: '16px'}}>
126+
<path d="m8.25 4.5 7.5 7.5-7.5 7.5" />
127+
</svg>
128+
</div>
129+
</Button>
130+
)}
131+
<Text className={styles.title}>{props.title || props.children}</Text>
132+
</div>
133+
</>
134+
)}
135+
</TreeItemContent>
136+
{props.title && props.children}
137+
</TreeItem>
138+
);
139+
};
140+
103141
const TreeExampleStaticRender = (args) => (
104142
<Tree className={styles.tree} {...args} disabledKeys={['projects']} aria-label="test static tree" onExpandedChange={action('onExpandedChange')} onSelectionChange={action('onSelectionChange')}>
105143
<StaticTreeItem id="Photos" textValue="Photos">Photos</StaticTreeItem>
@@ -147,6 +185,53 @@ const TreeExampleStaticRender = (args) => (
147185
</Tree>
148186
);
149187

188+
const TreeExampleStaticNoActionsRender = (args) => (
189+
<Tree className={styles.tree} {...args} disabledKeys={['projects']} aria-label="test static tree" onExpandedChange={action('onExpandedChange')} onSelectionChange={action('onSelectionChange')}>
190+
<StaticTreeItemNoActions id="Photos" textValue="Photos">Photos</StaticTreeItemNoActions>
191+
<StaticTreeItemNoActions id="projects" textValue="Projects" title="Projects">
192+
<StaticTreeItemNoActions id="projects-1" textValue="Projects-1" title="Projects-1">
193+
<StaticTreeItemNoActions id="projects-1A" textValue="Projects-1A">
194+
Projects-1A
195+
</StaticTreeItemNoActions>
196+
</StaticTreeItemNoActions>
197+
<StaticTreeItemNoActions id="projects-2" textValue="Projects-2">
198+
Projects-2
199+
</StaticTreeItemNoActions>
200+
<StaticTreeItemNoActions id="projects-3" textValue="Projects-3">
201+
Projects-3
202+
</StaticTreeItemNoActions>
203+
</StaticTreeItemNoActions>
204+
<StaticTreeItemNoActions
205+
id="reports"
206+
textValue="Reports"
207+
className={({isFocused, isSelected, isHovered, isFocusVisible}) => classNames(styles, 'tree-item', {
208+
focused: isFocused,
209+
'focus-visible': isFocusVisible,
210+
selected: isSelected,
211+
hovered: isHovered
212+
})}>
213+
<TreeItemContent>
214+
Reports
215+
</TreeItemContent>
216+
</StaticTreeItemNoActions>
217+
<StaticTreeItemNoActions
218+
id="Tests"
219+
textValue="Tests"
220+
className={({isFocused, isSelected, isHovered, isFocusVisible}) => classNames(styles, 'tree-item', {
221+
focused: isFocused,
222+
'focus-visible': isFocusVisible,
223+
selected: isSelected,
224+
hovered: isHovered
225+
})}>
226+
<TreeItemContent>
227+
{({isFocused}) => (
228+
<Text>{`${isFocused} Tests`}</Text>
229+
)}
230+
</TreeItemContent>
231+
</StaticTreeItemNoActions>
232+
</Tree>
233+
);
234+
150235
export const TreeExampleStatic = {
151236
render: TreeExampleStaticRender,
152237
args: {
@@ -176,6 +261,35 @@ export const TreeExampleStatic = {
176261
}
177262
};
178263

264+
export const TreeExampleStaticNoActions = {
265+
render: TreeExampleStaticNoActionsRender,
266+
args: {
267+
selectionMode: 'none',
268+
selectionBehavior: 'toggle',
269+
disabledBehavior: 'selection',
270+
disallowClearAll: false
271+
},
272+
argTypes: {
273+
selectionMode: {
274+
control: 'radio',
275+
options: ['none', 'single', 'multiple']
276+
},
277+
selectionBehavior: {
278+
control: 'radio',
279+
options: ['toggle', 'replace']
280+
},
281+
disabledBehavior: {
282+
control: 'radio',
283+
options: ['selection', 'all']
284+
}
285+
},
286+
parameters: {
287+
description: {
288+
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.'
289+
}
290+
}
291+
};
292+
179293
let rows = [
180294
{id: 'projects', name: 'Projects', childItems: [
181295
{id: 'project-1', name: 'Project 1'},
@@ -392,7 +506,7 @@ function LoadingStoryDepOnCollection(args) {
392506
getKey: item => item.id,
393507
getChildren: item => item.childItems
394508
});
395-
509+
396510
return (
397511
<Tree {...args} defaultExpandedKeys={defaultExpandedKeys} disabledKeys={['reports-1AB']} className={styles.tree} aria-label="test dynamic tree" onExpandedChange={action('onExpandedChange')} onSelectionChange={action('onSelectionChange')}>
398512
<Collection items={treeData.items} dependencies={[args.isLoading]}>
@@ -659,11 +773,11 @@ function SecondTree(args) {
659773
});
660774

661775
return (
662-
<Tree
663-
dragAndDropHooks={dragAndDropHooks}
664-
{...args}
665-
className={styles.tree}
666-
aria-label="Tree with drag and drop"
776+
<Tree
777+
dragAndDropHooks={dragAndDropHooks}
778+
{...args}
779+
className={styles.tree}
780+
aria-label="Tree with drag and drop"
667781
items={treeData.items}
668782
renderEmptyState={() => 'Drop items here'}>
669783
{(item: any) => (

0 commit comments

Comments
 (0)