Skip to content

Commit

Permalink
Maintain Table row interactive states if interactive and prevent visu…
Browse files Browse the repository at this point in the history 
…ally 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
  • Loading branch information
@LFDanLu
LFDanLu authored Aug 21, 2024
1 parent 2e689d9 commit 22e803a
Show file tree
Hide file tree
Showing 5 changed files with 150 additions and 76 deletions.
40 changes: 15 additions & 25 deletions packages/@react-spectrum/list/stories/ListView.stories.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof ListView>;
Expand All @@ -153,10 +143,10 @@ export type ListViewStory = ComponentStoryObj<typeof ListView>;

export const Default: ListViewStory = {
render: (args) => (
<ListView width="250px" aria-label="default ListView" {...args}>
<Item textValue="Adobe Photoshop">Adobe Photoshop</Item>
<Item textValue="Adobe Illustrator">Adobe Illustrator</Item>
<Item textValue="Adobe XD">Adobe XD</Item>
<ListView disabledKeys={['3']} width="250px" aria-label="default ListView" {...args}>
<Item key="1" textValue="Adobe Photoshop">Adobe Photoshop</Item>
<Item key="2" textValue="Adobe Illustrator">Adobe Illustrator</Item>
<Item key="3" textValue="Adobe XD">Adobe XD</Item>
</ListView>
),
name: 'default'
Expand Down
32 changes: 11 additions & 21 deletions packages/@react-spectrum/list/stories/ListViewDnD.stories.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof ListView>;
Expand Down
18 changes: 11 additions & 7 deletions packages/@react-spectrum/table/src/TableViewBase.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1332,7 +1336,7 @@ function TableCheckboxCell({cell}) {
<Checkbox
{...checkboxProps}
isEmphasized
isDisabled={isDisabled}
isDisabled={isSelectionDisabled}
UNSAFE_className={classNames(styles, 'spectrum-Table-checkbox')} />
}
</div>
Expand All @@ -1346,7 +1350,7 @@ function TableCell({cell}) {
let isExpandableTable = 'expandedKeys' in state;
let ref = useRef(undefined);
let columnProps = cell.column.props as SpectrumColumnProps<unknown>;
let isDisabled = state.disabledKeys.has(cell.parentKey);
let isDisabled = state.selectionManager.isDisabled(cell.parentKey);
let {gridCellProps} = useTableCell({
node: cell,
isVirtualized: true
Expand Down
58 changes: 47 additions & 11 deletions packages/@react-spectrum/table/test/Table.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3621,31 +3621,67 @@ export let tableTests = () => {
</TableView>
);

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(<TableWithBreadcrumbs selectionMode="multiple" />);

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, <TableWithBreadcrumbs selectionMode="single" />);
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(<TableWithBreadcrumbs selectionMode="none" />);
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(<TableWithBreadcrumbs disabledBehavior="all" selectionMode="none" />);

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(<TableWithBreadcrumbs onAction={jest.fn()} disabledBehavior="selection" selectionMode="none" />);

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(<TableWithBreadcrumbs disabledKeys={['Foo 1']} onAction={jest.fn()} disabledBehavior="selection" selectionMode="none" />);

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(<TableWithBreadcrumbs disabledKeys={['Foo 1']} onAction={jest.fn()} disabledBehavior="all" selectionMode="multiple" />);

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]'});
});
});

Expand Down
78 changes: 66 additions & 12 deletions packages/@react-spectrum/table/test/TableDnd.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,17 +232,17 @@ describe('TableView', function () {
let {getByRole, getAllByText} = render(
<DraggbleWithoutRowHeader tableViewProps={{selectedKeys: ['a'], selectionStyle: 'checkbox'}} />
);

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];
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}));

Expand Down Expand Up @@ -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(
<DraggableTableView tableViewProps={{disabledKeys: ['a']}} />
);
<DraggableTableView tableViewProps={{disabledBehavior: 'selection', disabledKeys: ['a']}} />
);

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(
<DraggableTableView tableViewProps={{disabledBehavior: 'all', disabledKeys: ['a']}} />
);

let grid = getByRole('grid');
let rowgroups = within(grid).getAllByRole('rowgroup');
Expand Down Expand Up @@ -2830,25 +2849,60 @@ 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];
expect(dragHandle.style).toBeTruthy();
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(
<DraggableTableView tableViewProps={{disabledBehavior: 'select', disabledKeys: ['a']}} />
);

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) {
Expand All @@ -2858,7 +2912,7 @@ describe('TableView', function () {
}

let {getByRole} = render(
<DraggableTableView tableViewProps={{disabledKeys: ['a']}} />
<DraggableTableView tableViewProps={{disabledBehavior: 'all', disabledKeys: ['a']}} />
);

let grid = getByRole('grid');
Expand Down

1 comment on commit 22e803a

@rspbot
Copy link

@rspbot rspbot commented on 22e803a Aug 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdaccio builds:
CRA Test App
NextJS Test App
RAC Tailwind Example
RAC Spectrum + Tailwind Example
CRA Test App Size
NextJS App Size
Publish stats
Size diff since last release
Docs
Storybook
S2 Storybook

Please sign in to comment.