Skip to content

Commit 22e803a

Browse files
LFDanLuLFDanLu
authored
Maintain Table row interactive states if interactive and prevent visually disabled state if non-interactive and disabled (#6863)
* Maintain row active/hover styles if it is interactive and do not apply disabled styles if non-interactive and disabled
1 parent 2e689d9 commit 22e803a

File tree

5 files changed

+150
-76
lines changed

5 files changed

+150
-76
lines changed

‎packages/@react-spectrum/list/stories/ListView.stories.tsx

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -114,37 +114,27 @@ export default {
114114
},
115115
argTypes: {
116116
selectionMode: {
117-
control: {
118-
type: 'radio',
119-
options: ['none', 'single', 'multiple']
120-
}
117+
control: 'radio',
118+
options: ['none', 'single', 'multiple']
121119
},
122120
selectionStyle: {
123-
control: {
124-
type: 'radio',
125-
options: ['checkbox', 'highlight']
126-
}
121+
control: 'radio',
122+
options: ['checkbox', 'highlight']
127123
},
128124
isQuiet: {
129-
control: {type: 'boolean'}
125+
control: 'boolean'
130126
},
131127
density: {
132-
control: {
133-
type: 'select',
134-
options: ['compact', 'regular', 'spacious']
135-
}
128+
control: 'select',
129+
options: ['compact', 'regular', 'spacious']
136130
},
137131
overflowMode: {
138-
control: {
139-
type: 'radio',
140-
options: ['truncate', 'wrap']
141-
}
132+
control: 'radio',
133+
options: ['truncate', 'wrap']
142134
},
143135
disabledBehavior: {
144-
control: {
145-
type: 'radio',
146-
options: ['selection', 'all']
147-
}
136+
control: 'radio',
137+
options: ['selection', 'all']
148138
}
149139
}
150140
} as ComponentMeta<typeof ListView>;
@@ -153,10 +143,10 @@ export type ListViewStory = ComponentStoryObj<typeof ListView>;
153143

154144
export const Default: ListViewStory = {
155145
render: (args) => (
156-
<ListView width="250px" aria-label="default ListView" {...args}>
157-
<Item textValue="Adobe Photoshop">Adobe Photoshop</Item>
158-
<Item textValue="Adobe Illustrator">Adobe Illustrator</Item>
159-
<Item textValue="Adobe XD">Adobe XD</Item>
146+
<ListView disabledKeys={['3']} width="250px" aria-label="default ListView" {...args}>
147+
<Item key="1" textValue="Adobe Photoshop">Adobe Photoshop</Item>
148+
<Item key="2" textValue="Adobe Illustrator">Adobe Illustrator</Item>
149+
<Item key="3" textValue="Adobe XD">Adobe XD</Item>
160150
</ListView>
161151
),
162152
name: 'default'

‎packages/@react-spectrum/list/stories/ListViewDnD.stories.tsx

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,37 +20,27 @@ export default {
2020
},
2121
argTypes: {
2222
selectionMode: {
23-
control: {
24-
type: 'radio',
25-
options: ['none', 'single', 'multiple']
26-
}
23+
control: 'radio',
24+
options: ['none', 'single', 'multiple']
2725
},
2826
selectionStyle: {
29-
control: {
30-
type: 'radio',
31-
options: ['checkbox', 'highlight']
32-
}
27+
control: 'radio',
28+
options: ['checkbox', 'highlight']
3329
},
3430
isQuiet: {
35-
control: {type: 'boolean'}
31+
control: 'boolean'
3632
},
3733
density: {
38-
control: {
39-
type: 'select',
40-
options: ['compact', 'regular', 'spacious']
41-
}
34+
control: 'select',
35+
options: ['compact', 'regular', 'spacious']
4236
},
4337
overflowMode: {
44-
control: {
45-
type: 'radio',
46-
options: ['truncate', 'wrap']
47-
}
38+
control: 'radio',
39+
options: ['truncate', 'wrap']
4840
},
4941
disabledBehavior: {
50-
control: {
51-
type: 'radio',
52-
options: ['selection', 'all']
53-
}
42+
control: 'radio',
43+
options: ['selection', 'all']
5444
}
5545
}
5646
} as ComponentMeta<typeof ListView>;

‎packages/@react-spectrum/table/src/TableViewBase.tsx

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1128,9 +1128,10 @@ function TableRow({item, children, layoutInfo, parent, ...otherProps}) {
11281128
shouldSelectOnPressUp: isTableDraggable
11291129
}, state, ref);
11301130

1131-
let isDisabled = !allowsSelection && !hasAction;
1131+
let isDisabled = state.selectionManager.isDisabled(item.key);
1132+
let isInteractive = !isDisabled && (hasAction || allowsSelection || isTableDraggable);
11321133
let isDroppable = isTableDroppable && !isDisabled;
1133-
let {pressProps, isPressed} = usePress({isDisabled});
1134+
let {pressProps, isPressed} = usePress({isDisabled: !isInteractive});
11341135

11351136
// The row should show the focus background style when any cell inside it is focused.
11361137
// If the row itself is focused, then it should have a blue focus indicator on the left.
@@ -1139,7 +1140,7 @@ function TableRow({item, children, layoutInfo, parent, ...otherProps}) {
11391140
focusProps: focusWithinProps
11401141
} = useFocusRing({within: true});
11411142
let {isFocusVisible, focusProps} = useFocusRing();
1142-
let {hoverProps, isHovered} = useHover({isDisabled});
1143+
let {hoverProps, isHovered} = useHover({isDisabled: !isInteractive});
11431144
let isFirstRow = state.collection.rows.find(row => row.level === 1)?.key === item.key;
11441145
let isLastRow = item.nextKey == null;
11451146
// Figure out if the TableView content is equal or greater in height to the container. If so, we'll need to round the bottom
@@ -1266,7 +1267,7 @@ function TableHeaderRow({item, children, layoutInfo, parent, ...props}) {
12661267
function TableDragCell({cell}) {
12671268
let ref = useRef(undefined);
12681269
let {state, isTableDraggable} = useTableContext();
1269-
let isDisabled = state.disabledKeys.has(cell.parentKey);
1270+
let isDisabled = state.selectionManager.isDisabled(cell.parentKey);
12701271
let {gridCellProps} = useTableCell({
12711272
node: cell,
12721273
isVirtualized: true
@@ -1301,7 +1302,10 @@ function TableDragCell({cell}) {
13011302
function TableCheckboxCell({cell}) {
13021303
let ref = useRef(undefined);
13031304
let {state} = useTableContext();
1304-
let isDisabled = state.disabledKeys.has(cell.parentKey);
1305+
// The TableCheckbox should always render its disabled status if the row is disabled, regardless of disabledBehavior,
1306+
// but the cell itself should not render its disabled styles if disabledBehavior="selection" because the row might have actions on it.
1307+
let isSelectionDisabled = state.disabledKeys.has(cell.parentKey);
1308+
let isDisabled = state.selectionManager.isDisabled(cell.parentKey);
13051309
let {gridCellProps} = useTableCell({
13061310
node: cell,
13071311
isVirtualized: true
@@ -1332,7 +1336,7 @@ function TableCheckboxCell({cell}) {
13321336
<Checkbox
13331337
{...checkboxProps}
13341338
isEmphasized
1335-
isDisabled={isDisabled}
1339+
isDisabled={isSelectionDisabled}
13361340
UNSAFE_className={classNames(styles, 'spectrum-Table-checkbox')} />
13371341
}
13381342
</div>
@@ -1346,7 +1350,7 @@ function TableCell({cell}) {
13461350
let isExpandableTable = 'expandedKeys' in state;
13471351
let ref = useRef(undefined);
13481352
let columnProps = cell.column.props as SpectrumColumnProps<unknown>;
1349-
let isDisabled = state.disabledKeys.has(cell.parentKey);
1353+
let isDisabled = state.selectionManager.isDisabled(cell.parentKey);
13501354
let {gridCellProps} = useTableCell({
13511355
node: cell,
13521356
isVirtualized: true

‎packages/@react-spectrum/table/test/Table.test.js

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3621,31 +3621,67 @@ export let tableTests = () => {
36213621
</TableView>
36223622
);
36233623

3624-
it('displays pressed/hover styles when row is pressed/hovered and selection mode is not "none"', function () {
3624+
it('displays pressed/hover styles when row is pressed/hovered and selection mode is not "none"', async function () {
36253625
let tree = render(<TableWithBreadcrumbs selectionMode="multiple" />);
36263626

36273627
let row = tree.getAllByRole('row')[1];
3628-
fireEvent.mouseDown(row, {detail: 1});
3629-
expect(row.className.includes('is-active')).toBeTruthy();
3630-
fireEvent.mouseEnter(row);
3628+
await user.hover(row);
36313629
expect(row.className.includes('is-hovered')).toBeTruthy();
3630+
await user.pointer({target: row, keys: '[MouseLeft>]'});
3631+
expect(row.className.includes('is-active')).toBeTruthy();
3632+
await user.pointer({target: row, keys: '[/MouseLeft]'});
36323633

36333634
rerender(tree, <TableWithBreadcrumbs selectionMode="single" />);
36343635
row = tree.getAllByRole('row')[1];
3635-
fireEvent.mouseDown(row, {detail: 1});
3636-
expect(row.className.includes('is-active')).toBeTruthy();
3637-
fireEvent.mouseEnter(row);
3636+
await user.hover(row);
36383637
expect(row.className.includes('is-hovered')).toBeTruthy();
3638+
await user.pointer({target: row, keys: '[MouseLeft>]'});
3639+
expect(row.className.includes('is-active')).toBeTruthy();
3640+
await user.pointer({target: row, keys: '[/MouseLeft]'});
36393641
});
36403642

3641-
it('doesn\'t show pressed/hover styles when row is pressed/hovered and selection mode is "none"', function () {
3642-
let tree = render(<TableWithBreadcrumbs selectionMode="none" />);
3643+
it('doesn\'t show pressed/hover styles when row is pressed/hovered and selection mode is "none" and disabledBehavior="all"', async function () {
3644+
let tree = render(<TableWithBreadcrumbs disabledBehavior="all" selectionMode="none" />);
36433645

36443646
let row = tree.getAllByRole('row')[1];
3645-
fireEvent.mouseDown(row, {detail: 1});
3647+
await user.hover(row);
3648+
expect(row.className.includes('is-hovered')).toBeFalsy();
3649+
await user.pointer({target: row, keys: '[MouseLeft>]'});
36463650
expect(row.className.includes('is-active')).toBeFalsy();
3647-
fireEvent.mouseEnter(row);
3651+
await user.pointer({target: row, keys: '[/MouseLeft]'});
3652+
});
3653+
3654+
it('shows pressed/hover styles when row is pressed/hovered and selection mode is "none", disabledBehavior="selection" and has a action', async function () {
3655+
let tree = render(<TableWithBreadcrumbs onAction={jest.fn()} disabledBehavior="selection" selectionMode="none" />);
3656+
3657+
let row = tree.getAllByRole('row')[1];
3658+
await user.hover(row);
3659+
expect(row.className.includes('is-hovered')).toBeTruthy();
3660+
await user.pointer({target: row, keys: '[MouseLeft>]'});
3661+
expect(row.className.includes('is-active')).toBeTruthy();
3662+
await user.pointer({target: row, keys: '[/MouseLeft]'});
3663+
});
3664+
3665+
it('shows pressed/hover styles when row is pressed/hovered, disabledBehavior="selection", row is disabled and has a action', async function () {
3666+
let tree = render(<TableWithBreadcrumbs disabledKeys={['Foo 1']} onAction={jest.fn()} disabledBehavior="selection" selectionMode="none" />);
3667+
3668+
let row = tree.getAllByRole('row')[1];
3669+
await user.hover(row);
3670+
expect(row.className.includes('is-hovered')).toBeTruthy();
3671+
await user.pointer({target: row, keys: '[MouseLeft>]'});
3672+
expect(row.className.includes('is-active')).toBeTruthy();
3673+
await user.pointer({target: row, keys: '[/MouseLeft]'});
3674+
});
3675+
3676+
it('doesn\'t show pressed/hover styles when row is pressed/hovered, has a action, but is disabled and disabledBehavior="all"', async function () {
3677+
let tree = render(<TableWithBreadcrumbs disabledKeys={['Foo 1']} onAction={jest.fn()} disabledBehavior="all" selectionMode="multiple" />);
3678+
3679+
let row = tree.getAllByRole('row')[1];
3680+
await user.hover(row);
36483681
expect(row.className.includes('is-hovered')).toBeFalsy();
3682+
await user.pointer({target: row, keys: '[MouseLeft>]'});
3683+
expect(row.className.includes('is-active')).toBeFalsy();
3684+
await user.pointer({target: row, keys: '[/MouseLeft]'});
36493685
});
36503686
});
36513687

‎packages/@react-spectrum/table/test/TableDnd.test.js

Lines changed: 66 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -232,17 +232,17 @@ describe('TableView', function () {
232232
let {getByRole, getAllByText} = render(
233233
<DraggbleWithoutRowHeader tableViewProps={{selectedKeys: ['a'], selectionStyle: 'checkbox'}} />
234234
);
235-
235+
236236
let grid = getByRole('grid');
237237
let rowgroups = within(grid).getAllByRole('rowgroup');
238238
let rows = within(rowgroups[1]).getAllByRole('row');
239239
let row = rows[0];
240240
let cell = within(row).getAllByRole('rowheader')[0];
241241
let cellText = getAllByText(cell.textContent);
242242
expect(cellText).toHaveLength(1);
243-
243+
244244
let dataTransfer = new DataTransfer();
245-
245+
246246
fireEvent.pointerDown(cell, {pointerType: 'mouse', button: 0, pointerId: 1, clientX: 5, clientY: 5});
247247
fireEvent(cell, new DragEvent('dragstart', {dataTransfer, clientX: 5, clientY: 5}));
248248

@@ -414,10 +414,29 @@ describe('TableView', function () {
414414
});
415415
});
416416

417-
it('should not allow drag operations on a disabled row', function () {
417+
it('should allow drag operations on a disabled row with disabledBehavior="selection"', function () {
418418
let {getByRole} = render(
419-
<DraggableTableView tableViewProps={{disabledKeys: ['a']}} />
420-
);
419+
<DraggableTableView tableViewProps={{disabledBehavior: 'selection', disabledKeys: ['a']}} />
420+
);
421+
422+
let grid = getByRole('grid');
423+
let rowgroups = within(grid).getAllByRole('rowgroup');
424+
let rows = within(rowgroups[1]).getAllByRole('row');
425+
let row = rows[0];
426+
let cell = within(row).getAllByRole('rowheader')[0];
427+
expect(cell).toHaveTextContent('Vin');
428+
expect(row).toHaveAttribute('draggable', 'true');
429+
430+
let dataTransfer = new DataTransfer();
431+
fireEvent.pointerDown(cell, {pointerType: 'mouse', button: 0, pointerId: 1, clientX: 0, clientY: 0});
432+
fireEvent(cell, new DragEvent('dragstart', {dataTransfer, clientX: 0, clientY: 0}));
433+
expect(onDragStart).toHaveBeenCalledTimes(1);
434+
});
435+
436+
it('should not allow drag operations on a disabled row with disabledBehavior="all"', function () {
437+
let {getByRole} = render(
438+
<DraggableTableView tableViewProps={{disabledBehavior: 'all', disabledKeys: ['a']}} />
439+
);
421440

422441
let grid = getByRole('grid');
423442
let rowgroups = within(grid).getAllByRole('rowgroup');
@@ -2830,25 +2849,60 @@ describe('TableView', function () {
28302849
expect(dragHandle.style).toBeTruthy();
28312850
expect(dragHandle.style.position).toBe('absolute');
28322851

2833-
fireEvent.pointerDown(rows[0], {pointerType: 'mouse', button: 0, pointerId: 1});
2852+
await user.pointer({target: rows[0], keys: '[MouseLeft>]'});
28342853
dragHandle = within(rows[0]).getAllByRole('button')[0];
28352854
expect(dragHandle.style).toBeTruthy();
28362855
expect(dragHandle.style.position).toBe('absolute');
2837-
fireEvent.pointerUp(rows[0], {button: 0, pointerId: 1});
2856+
await user.pointer({target: rows[0], keys: '[/MouseLeft]'});
28382857

28392858
fireEvent.pointerEnter(rows[0], {pointerType: 'mouse'});
28402859
dragHandle = within(rows[0]).getAllByRole('button')[0];
28412860
expect(dragHandle.style).toBeTruthy();
28422861
expect(dragHandle.style.position).toBe('absolute');
28432862

28442863
// If dragHandle doesn't have a position applied, it isn't visually hidden
2845-
fireEvent.keyDown(rows[0], {key: 'Enter'});
2846-
fireEvent.keyUp(rows[0], {key: 'Enter'});
2864+
await user.keyboard('{Enter}');
28472865
dragHandle = within(rows[0]).getAllByRole('button')[0];
28482866
expect(dragHandle.style.position).toBe('');
28492867
});
28502868

2851-
it('should not display the drag handle on hover, press, or keyboard focus for disabled/non dragggable items', async function () {
2869+
it('should display the drag handle on hover, press, or keyboard focus for disabled/non dragggable items with disabledBehavior="select"', async function () {
2870+
function hasDragHandle(el) {
2871+
let buttons = within(el).queryAllByRole('button');
2872+
if (buttons.length === 0) {
2873+
return false;
2874+
}
2875+
return buttons[0].getAttribute('draggable');
2876+
}
2877+
2878+
let {getByRole} = render(
2879+
<DraggableTableView tableViewProps={{disabledBehavior: 'select', disabledKeys: ['a']}} />
2880+
);
2881+
2882+
let grid = getByRole('grid');
2883+
let rowgroups = within(grid).getAllByRole('rowgroup');
2884+
let rows = within(rowgroups[1]).getAllByRole('row');
2885+
2886+
await user.tab();
2887+
expect(hasDragHandle(rows[0])).toBeTruthy();
2888+
moveFocus('ArrowDown');
2889+
expect(hasDragHandle(rows[1])).toBeTruthy();
2890+
2891+
await user.pointer({target: rows[0], keys: '[MouseLeft>]'});
2892+
expect(hasDragHandle(rows[0])).toBeTruthy();
2893+
await user.pointer({target: rows[0], keys: '[/MouseLeft]'});
2894+
2895+
await user.pointer({target: rows[1], keys: '[MouseLeft>]'});
2896+
expect(hasDragHandle(rows[1])).toBeTruthy();
2897+
await user.pointer({target: rows[1], keys: '[/MouseLeft]'});
2898+
2899+
fireEvent.pointerEnter(rows[0]);
2900+
expect(hasDragHandle(rows[0])).toBeTruthy();
2901+
fireEvent.pointerEnter(rows[1]);
2902+
expect(hasDragHandle(rows[1])).toBeTruthy();
2903+
});
2904+
2905+
it('should not display the drag handle on hover, press, or keyboard focus for disabled/non dragggable items with disabledBehavior="all"', async function () {
28522906
function hasDragHandle(el) {
28532907
let buttons = within(el).queryAllByRole('button');
28542908
if (buttons.length === 0) {
@@ -2858,7 +2912,7 @@ describe('TableView', function () {
28582912
}
28592913

28602914
let {getByRole} = render(
2861-
<DraggableTableView tableViewProps={{disabledKeys: ['a']}} />
2915+
<DraggableTableView tableViewProps={{disabledBehavior: 'all', disabledKeys: ['a']}} />
28622916
);
28632917

28642918
let grid = getByRole('grid');

0 commit comments

Comments
 (0)