From 22e803a2bb79306336190e7447469dbef2dff48b Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Wed, 21 Aug 2024 16:06:33 -0700 Subject: [PATCH] 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 --- .../list/stories/ListView.stories.tsx | 40 ++++------ .../list/stories/ListViewDnD.stories.tsx | 32 +++----- .../table/src/TableViewBase.tsx | 18 +++-- .../@react-spectrum/table/test/Table.test.js | 58 +++++++++++--- .../table/test/TableDnd.test.js | 78 ++++++++++++++++--- 5 files changed, 150 insertions(+), 76 deletions(-) diff --git a/packages/@react-spectrum/list/stories/ListView.stories.tsx b/packages/@react-spectrum/list/stories/ListView.stories.tsx index 3c4cb6d822f..51fc5eea01d 100644 --- a/packages/@react-spectrum/list/stories/ListView.stories.tsx +++ b/packages/@react-spectrum/list/stories/ListView.stories.tsx @@ -114,37 +114,27 @@ export default { }, argTypes: { selectionMode: { - control: { - type: 'radio', - options: ['none', 'single', 'multiple'] - } + control: 'radio', + options: ['none', 'single', 'multiple'] }, selectionStyle: { - control: { - type: 'radio', - options: ['checkbox', 'highlight'] - } + control: 'radio', + options: ['checkbox', 'highlight'] }, isQuiet: { - control: {type: 'boolean'} + control: 'boolean' }, density: { - control: { - type: 'select', - options: ['compact', 'regular', 'spacious'] - } + control: 'select', + options: ['compact', 'regular', 'spacious'] }, overflowMode: { - control: { - type: 'radio', - options: ['truncate', 'wrap'] - } + control: 'radio', + options: ['truncate', 'wrap'] }, disabledBehavior: { - control: { - type: 'radio', - options: ['selection', 'all'] - } + control: 'radio', + options: ['selection', 'all'] } } } as ComponentMeta; @@ -153,10 +143,10 @@ export type ListViewStory = ComponentStoryObj; export const Default: ListViewStory = { render: (args) => ( - - Adobe Photoshop - Adobe Illustrator - Adobe XD + + Adobe Photoshop + Adobe Illustrator + Adobe XD ), name: 'default' diff --git a/packages/@react-spectrum/list/stories/ListViewDnD.stories.tsx b/packages/@react-spectrum/list/stories/ListViewDnD.stories.tsx index 632d02b0f78..3734e0698af 100644 --- a/packages/@react-spectrum/list/stories/ListViewDnD.stories.tsx +++ b/packages/@react-spectrum/list/stories/ListViewDnD.stories.tsx @@ -20,37 +20,27 @@ export default { }, argTypes: { selectionMode: { - control: { - type: 'radio', - options: ['none', 'single', 'multiple'] - } + control: 'radio', + options: ['none', 'single', 'multiple'] }, selectionStyle: { - control: { - type: 'radio', - options: ['checkbox', 'highlight'] - } + control: 'radio', + options: ['checkbox', 'highlight'] }, isQuiet: { - control: {type: 'boolean'} + control: 'boolean' }, density: { - control: { - type: 'select', - options: ['compact', 'regular', 'spacious'] - } + control: 'select', + options: ['compact', 'regular', 'spacious'] }, overflowMode: { - control: { - type: 'radio', - options: ['truncate', 'wrap'] - } + control: 'radio', + options: ['truncate', 'wrap'] }, disabledBehavior: { - control: { - type: 'radio', - options: ['selection', 'all'] - } + control: 'radio', + options: ['selection', 'all'] } } } as ComponentMeta; diff --git a/packages/@react-spectrum/table/src/TableViewBase.tsx b/packages/@react-spectrum/table/src/TableViewBase.tsx index 2cd79351b19..8662df8701d 100644 --- a/packages/@react-spectrum/table/src/TableViewBase.tsx +++ b/packages/@react-spectrum/table/src/TableViewBase.tsx @@ -1128,9 +1128,10 @@ function TableRow({item, children, layoutInfo, parent, ...otherProps}) { shouldSelectOnPressUp: isTableDraggable }, state, ref); - let isDisabled = !allowsSelection && !hasAction; + let isDisabled = state.selectionManager.isDisabled(item.key); + let isInteractive = !isDisabled && (hasAction || allowsSelection || isTableDraggable); let isDroppable = isTableDroppable && !isDisabled; - let {pressProps, isPressed} = usePress({isDisabled}); + let {pressProps, isPressed} = usePress({isDisabled: !isInteractive}); // The row should show the focus background style when any cell inside it is focused. // 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}) { focusProps: focusWithinProps } = useFocusRing({within: true}); let {isFocusVisible, focusProps} = useFocusRing(); - let {hoverProps, isHovered} = useHover({isDisabled}); + let {hoverProps, isHovered} = useHover({isDisabled: !isInteractive}); let isFirstRow = state.collection.rows.find(row => row.level === 1)?.key === item.key; let isLastRow = item.nextKey == null; // 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}) { function TableDragCell({cell}) { let ref = useRef(undefined); let {state, isTableDraggable} = useTableContext(); - let isDisabled = state.disabledKeys.has(cell.parentKey); + let isDisabled = state.selectionManager.isDisabled(cell.parentKey); let {gridCellProps} = useTableCell({ node: cell, isVirtualized: true @@ -1301,7 +1302,10 @@ function TableDragCell({cell}) { function TableCheckboxCell({cell}) { let ref = useRef(undefined); let {state} = useTableContext(); - let isDisabled = state.disabledKeys.has(cell.parentKey); + // The TableCheckbox should always render its disabled status if the row is disabled, regardless of disabledBehavior, + // but the cell itself should not render its disabled styles if disabledBehavior="selection" because the row might have actions on it. + let isSelectionDisabled = state.disabledKeys.has(cell.parentKey); + let isDisabled = state.selectionManager.isDisabled(cell.parentKey); let {gridCellProps} = useTableCell({ node: cell, isVirtualized: true @@ -1332,7 +1336,7 @@ function TableCheckboxCell({cell}) { } @@ -1346,7 +1350,7 @@ function TableCell({cell}) { let isExpandableTable = 'expandedKeys' in state; let ref = useRef(undefined); let columnProps = cell.column.props as SpectrumColumnProps; - let isDisabled = state.disabledKeys.has(cell.parentKey); + let isDisabled = state.selectionManager.isDisabled(cell.parentKey); let {gridCellProps} = useTableCell({ node: cell, isVirtualized: true diff --git a/packages/@react-spectrum/table/test/Table.test.js b/packages/@react-spectrum/table/test/Table.test.js index cbd8344680f..6d12c616229 100644 --- a/packages/@react-spectrum/table/test/Table.test.js +++ b/packages/@react-spectrum/table/test/Table.test.js @@ -3621,31 +3621,67 @@ export let tableTests = () => { ); - it('displays pressed/hover styles when row is pressed/hovered and selection mode is not "none"', function () { + it('displays pressed/hover styles when row is pressed/hovered and selection mode is not "none"', async function () { let tree = render(); let row = tree.getAllByRole('row')[1]; - fireEvent.mouseDown(row, {detail: 1}); - expect(row.className.includes('is-active')).toBeTruthy(); - fireEvent.mouseEnter(row); + await user.hover(row); expect(row.className.includes('is-hovered')).toBeTruthy(); + await user.pointer({target: row, keys: '[MouseLeft>]'}); + expect(row.className.includes('is-active')).toBeTruthy(); + await user.pointer({target: row, keys: '[/MouseLeft]'}); rerender(tree, ); row = tree.getAllByRole('row')[1]; - fireEvent.mouseDown(row, {detail: 1}); - expect(row.className.includes('is-active')).toBeTruthy(); - fireEvent.mouseEnter(row); + await user.hover(row); expect(row.className.includes('is-hovered')).toBeTruthy(); + await user.pointer({target: row, keys: '[MouseLeft>]'}); + expect(row.className.includes('is-active')).toBeTruthy(); + await user.pointer({target: row, keys: '[/MouseLeft]'}); }); - it('doesn\'t show pressed/hover styles when row is pressed/hovered and selection mode is "none"', function () { - let tree = render(); + it('doesn\'t show pressed/hover styles when row is pressed/hovered and selection mode is "none" and disabledBehavior="all"', async function () { + let tree = render(); let row = tree.getAllByRole('row')[1]; - fireEvent.mouseDown(row, {detail: 1}); + await user.hover(row); + expect(row.className.includes('is-hovered')).toBeFalsy(); + await user.pointer({target: row, keys: '[MouseLeft>]'}); expect(row.className.includes('is-active')).toBeFalsy(); - fireEvent.mouseEnter(row); + await user.pointer({target: row, keys: '[/MouseLeft]'}); + }); + + it('shows pressed/hover styles when row is pressed/hovered and selection mode is "none", disabledBehavior="selection" and has a action', async function () { + let tree = render(); + + let row = tree.getAllByRole('row')[1]; + await user.hover(row); + expect(row.className.includes('is-hovered')).toBeTruthy(); + await user.pointer({target: row, keys: '[MouseLeft>]'}); + expect(row.className.includes('is-active')).toBeTruthy(); + await user.pointer({target: row, keys: '[/MouseLeft]'}); + }); + + it('shows pressed/hover styles when row is pressed/hovered, disabledBehavior="selection", row is disabled and has a action', async function () { + let tree = render(); + + let row = tree.getAllByRole('row')[1]; + await user.hover(row); + expect(row.className.includes('is-hovered')).toBeTruthy(); + await user.pointer({target: row, keys: '[MouseLeft>]'}); + expect(row.className.includes('is-active')).toBeTruthy(); + await user.pointer({target: row, keys: '[/MouseLeft]'}); + }); + + it('doesn\'t show pressed/hover styles when row is pressed/hovered, has a action, but is disabled and disabledBehavior="all"', async function () { + let tree = render(); + + let row = tree.getAllByRole('row')[1]; + await user.hover(row); expect(row.className.includes('is-hovered')).toBeFalsy(); + await user.pointer({target: row, keys: '[MouseLeft>]'}); + expect(row.className.includes('is-active')).toBeFalsy(); + await user.pointer({target: row, keys: '[/MouseLeft]'}); }); }); diff --git a/packages/@react-spectrum/table/test/TableDnd.test.js b/packages/@react-spectrum/table/test/TableDnd.test.js index 08282279139..58b305f8a3b 100644 --- a/packages/@react-spectrum/table/test/TableDnd.test.js +++ b/packages/@react-spectrum/table/test/TableDnd.test.js @@ -232,7 +232,7 @@ describe('TableView', function () { let {getByRole, getAllByText} = render( ); - + let grid = getByRole('grid'); let rowgroups = within(grid).getAllByRole('rowgroup'); let rows = within(rowgroups[1]).getAllByRole('row'); @@ -240,9 +240,9 @@ describe('TableView', function () { let cell = within(row).getAllByRole('rowheader')[0]; let cellText = getAllByText(cell.textContent); expect(cellText).toHaveLength(1); - + let dataTransfer = new DataTransfer(); - + fireEvent.pointerDown(cell, {pointerType: 'mouse', button: 0, pointerId: 1, clientX: 5, clientY: 5}); fireEvent(cell, new DragEvent('dragstart', {dataTransfer, clientX: 5, clientY: 5})); @@ -414,10 +414,29 @@ describe('TableView', function () { }); }); - it('should not allow drag operations on a disabled row', function () { + it('should allow drag operations on a disabled row with disabledBehavior="selection"', function () { let {getByRole} = render( - - ); + + ); + + let grid = getByRole('grid'); + let rowgroups = within(grid).getAllByRole('rowgroup'); + let rows = within(rowgroups[1]).getAllByRole('row'); + let row = rows[0]; + let cell = within(row).getAllByRole('rowheader')[0]; + expect(cell).toHaveTextContent('Vin'); + expect(row).toHaveAttribute('draggable', 'true'); + + let dataTransfer = new DataTransfer(); + fireEvent.pointerDown(cell, {pointerType: 'mouse', button: 0, pointerId: 1, clientX: 0, clientY: 0}); + fireEvent(cell, new DragEvent('dragstart', {dataTransfer, clientX: 0, clientY: 0})); + expect(onDragStart).toHaveBeenCalledTimes(1); + }); + + it('should not allow drag operations on a disabled row with disabledBehavior="all"', function () { + let {getByRole} = render( + + ); let grid = getByRole('grid'); let rowgroups = within(grid).getAllByRole('rowgroup'); @@ -2830,11 +2849,11 @@ describe('TableView', function () { expect(dragHandle.style).toBeTruthy(); expect(dragHandle.style.position).toBe('absolute'); - fireEvent.pointerDown(rows[0], {pointerType: 'mouse', button: 0, pointerId: 1}); + await user.pointer({target: rows[0], keys: '[MouseLeft>]'}); dragHandle = within(rows[0]).getAllByRole('button')[0]; expect(dragHandle.style).toBeTruthy(); expect(dragHandle.style.position).toBe('absolute'); - fireEvent.pointerUp(rows[0], {button: 0, pointerId: 1}); + await user.pointer({target: rows[0], keys: '[/MouseLeft]'}); fireEvent.pointerEnter(rows[0], {pointerType: 'mouse'}); dragHandle = within(rows[0]).getAllByRole('button')[0]; @@ -2842,13 +2861,48 @@ describe('TableView', function () { expect(dragHandle.style.position).toBe('absolute'); // If dragHandle doesn't have a position applied, it isn't visually hidden - fireEvent.keyDown(rows[0], {key: 'Enter'}); - fireEvent.keyUp(rows[0], {key: 'Enter'}); + await user.keyboard('{Enter}'); dragHandle = within(rows[0]).getAllByRole('button')[0]; expect(dragHandle.style.position).toBe(''); }); - it('should not display the drag handle on hover, press, or keyboard focus for disabled/non dragggable items', async function () { + it('should display the drag handle on hover, press, or keyboard focus for disabled/non dragggable items with disabledBehavior="select"', async function () { + function hasDragHandle(el) { + let buttons = within(el).queryAllByRole('button'); + if (buttons.length === 0) { + return false; + } + return buttons[0].getAttribute('draggable'); + } + + let {getByRole} = render( + + ); + + let grid = getByRole('grid'); + let rowgroups = within(grid).getAllByRole('rowgroup'); + let rows = within(rowgroups[1]).getAllByRole('row'); + + await user.tab(); + expect(hasDragHandle(rows[0])).toBeTruthy(); + moveFocus('ArrowDown'); + expect(hasDragHandle(rows[1])).toBeTruthy(); + + await user.pointer({target: rows[0], keys: '[MouseLeft>]'}); + expect(hasDragHandle(rows[0])).toBeTruthy(); + await user.pointer({target: rows[0], keys: '[/MouseLeft]'}); + + await user.pointer({target: rows[1], keys: '[MouseLeft>]'}); + expect(hasDragHandle(rows[1])).toBeTruthy(); + await user.pointer({target: rows[1], keys: '[/MouseLeft]'}); + + fireEvent.pointerEnter(rows[0]); + expect(hasDragHandle(rows[0])).toBeTruthy(); + fireEvent.pointerEnter(rows[1]); + expect(hasDragHandle(rows[1])).toBeTruthy(); + }); + + it('should not display the drag handle on hover, press, or keyboard focus for disabled/non dragggable items with disabledBehavior="all"', async function () { function hasDragHandle(el) { let buttons = within(el).queryAllByRole('button'); if (buttons.length === 0) { @@ -2858,7 +2912,7 @@ describe('TableView', function () { } let {getByRole} = render( - + ); let grid = getByRole('grid');