Skip to content

Commit 8a05f36

Browse files
JszigetiJszigeti
committed
Fix shift-click deselection behavior in Datagrid and add related tests
1 parent 7dc6c23 commit 8a05f36

File tree

3 files changed

+98
-15
lines changed

3 files changed

+98
-15
lines changed

‎cypress/e2e/list.cy.js‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,64 @@ describe('List Page', () => {
224224
.click({ shiftKey: true });
225225
cy.contains('6 items selected');
226226
});
227+
228+
it('should allow to deselect a range with shift key', () => {
229+
cy.contains('1-10 of 13'); // wait for data
230+
cy.get(ListPagePosts.elements.selectItem).eq(0).click();
231+
cy.get(ListPagePosts.elements.selectItem)
232+
.eq(4)
233+
.click({ shiftKey: true });
234+
cy.contains('5 items selected');
235+
cy.get(ListPagePosts.elements.selectedItem).should(els =>
236+
expect(els).to.have.length(5)
237+
);
238+
cy.get(ListPagePosts.elements.selectItem)
239+
.eq(2)
240+
.click({ shiftKey: true });
241+
cy.contains('2 items selected');
242+
cy.get(ListPagePosts.elements.selectedItem).should(els =>
243+
expect(els).to.have.length(2)
244+
);
245+
});
246+
247+
it('should allow alternating shift-select and shift-deselect', () => {
248+
cy.contains('1-10 of 13'); // wait for data
249+
cy.get(ListPagePosts.elements.selectItem).eq(0).click();
250+
cy.get(ListPagePosts.elements.selectItem)
251+
.eq(3)
252+
.click({ shiftKey: true });
253+
cy.contains('4 items selected');
254+
cy.get(ListPagePosts.elements.selectItem)
255+
.eq(4)
256+
.click({ shiftKey: true });
257+
cy.contains('5 items selected');
258+
cy.get(ListPagePosts.elements.selectItem)
259+
.eq(2)
260+
.click({ shiftKey: true });
261+
cy.contains('2 items selected');
262+
cy.get(ListPagePosts.elements.selectItem)
263+
.eq(4)
264+
.click({ shiftKey: true });
265+
cy.contains('5 items selected');
266+
cy.get(ListPagePosts.elements.selectedItem).should(els =>
267+
expect(els).to.have.length(5)
268+
);
269+
});
270+
271+
it('should support shift-deselect after select all then manual deselect', () => {
272+
cy.contains('1-10 of 13'); // wait for data
273+
ListPagePosts.toggleSelectAll();
274+
cy.contains('10 items selected');
275+
cy.get(ListPagePosts.elements.selectItem).eq(1).click();
276+
cy.contains('9 items selected');
277+
cy.get(ListPagePosts.elements.selectItem)
278+
.eq(3)
279+
.click({ shiftKey: true });
280+
cy.contains('7 items selected');
281+
cy.get(ListPagePosts.elements.selectedItem).should(els =>
282+
expect(els).to.have.length(7)
283+
);
284+
});
227285
});
228286

229287
describe('rowClick', () => {

‎packages/ra-core/src/dataTable/DataTableBase.tsx‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,20 @@
1+
import difference from 'lodash/difference';
2+
import union from 'lodash/union';
13
import * as React from 'react';
24
import { useEffect, useMemo, useRef, type FC, type ReactNode } from 'react';
3-
import union from 'lodash/union';
4-
import difference from 'lodash/difference';
55

6-
import { OptionalResourceContextProvider, useResourceContext } from '../core';
7-
import { useEvent } from '../util';
86
import { useListContextWithProps } from '../controller/list/useListContextWithProps';
97
import { type ListControllerResult } from '../controller/list/useListController';
10-
import { type RowClickFunctionBase } from './types';
8+
import { OptionalResourceContextProvider, useResourceContext } from '../core';
119
import { type Identifier, type RaRecord, type SortPayload } from '../types';
12-
import { DataTableConfigContext } from './DataTableConfigContext';
10+
import { useEvent } from '../util';
1311
import { DataTableCallbacksContext } from './DataTableCallbacksContext';
12+
import { DataTableConfigContext } from './DataTableConfigContext';
1413
import { DataTableDataContext } from './DataTableDataContext';
1514
import { DataTableSelectedIdsContext } from './DataTableSelectedIdsContext';
1615
import { DataTableSortContext } from './DataTableSortContext';
1716
import { DataTableStoreContext } from './DataTableStoreContext';
17+
import { type RowClickFunctionBase } from './types';
1818

1919
export const DataTableBase = function DataTable<
2020
RecordType extends RaRecord = any,
@@ -76,8 +76,6 @@ export const DataTableBase = function DataTable<
7676
if (!data) return;
7777
const ids = data.map(record => record.id);
7878
const lastSelectedIndex = ids.indexOf(lastSelected.current);
79-
// @ts-ignore FIXME useEvent prevents using event.currentTarget
80-
lastSelected.current = event.target.checked ? id : null;
8179

8280
if (event.shiftKey && lastSelectedIndex !== -1) {
8381
const index = ids.indexOf(id);
@@ -86,10 +84,10 @@ export const DataTableBase = function DataTable<
8684
Math.max(lastSelectedIndex, index) + 1
8785
);
8886

89-
// @ts-ignore FIXME useEvent prevents using event.currentTarget
90-
const newSelectedIds = event.target.checked
91-
? union(selectedIds, idsBetweenSelections)
92-
: difference(selectedIds, idsBetweenSelections);
87+
const isClickedItemSelected = selectedIds?.includes(id);
88+
const newSelectedIds = isClickedItemSelected
89+
? difference(selectedIds, idsBetweenSelections)
90+
: union(selectedIds, idsBetweenSelections);
9391

9492
onSelect?.(
9593
isRowSelectable
@@ -103,6 +101,8 @@ export const DataTableBase = function DataTable<
103101
} else {
104102
onToggleItem?.(id);
105103
}
104+
105+
lastSelected.current = id;
106106
}
107107
);
108108

‎packages/ra-ui-materialui/src/list/datatable/DataTable.spec.tsx‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
1+
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
12
import * as React from 'react';
2-
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
33
import {
44
Basic,
55
Columns,
66
Empty,
7-
StandaloneStatic,
8-
StandaloneDynamic,
97
Expand,
108
ExpandSingle,
119
IsRowExpandable,
1210
IsRowSelectable,
1311
NonPrimitiveData,
12+
StandaloneDynamic,
13+
StandaloneStatic,
1414
} from './DataTable.stories';
1515

1616
describe('DataTable', () => {
@@ -245,6 +245,31 @@ describe('DataTable', () => {
245245
fireEvent.click(checkboxes[0]);
246246
await screen.findByText('2 items selected');
247247
});
248+
it('should support alternating shift-select and shift-deselect', async () => {
249+
render(<Basic />);
250+
const checkboxes = await screen.findAllByRole('checkbox');
251+
fireEvent.click(checkboxes[1]);
252+
fireEvent.click(checkboxes[4], { shiftKey: true });
253+
await screen.findByText('4 items selected');
254+
fireEvent.click(checkboxes[5], { shiftKey: true });
255+
await screen.findByText('5 items selected');
256+
fireEvent.click(checkboxes[3], { shiftKey: true });
257+
await screen.findByText('2 items selected');
258+
fireEvent.click(checkboxes[5], { shiftKey: true });
259+
await screen.findByText('5 items selected');
260+
});
261+
it('should support shift-deselect after select all then deselect one', async () => {
262+
render(<Basic />);
263+
const checkboxes = await screen.findAllByRole('checkbox');
264+
fireEvent.click(checkboxes[0]);
265+
const selectAllButton = await screen.findByText('Select all');
266+
selectAllButton.click();
267+
await screen.findByText('7 items selected');
268+
fireEvent.click(checkboxes[2]);
269+
await screen.findByText('6 items selected');
270+
fireEvent.click(checkboxes[4], { shiftKey: true });
271+
await screen.findByText('4 items selected');
272+
});
248273
});
249274
describe('isRowSelectable', () => {
250275
it('should allow to disable row selection', async () => {

0 commit comments

Comments
 (0)