Fix: Shift+Click deselection range not working properly in Datagrid

cypress/e2e/list.cy.js

@@ -224,6 +224,64 @@ describe('List Page', () => {
                 .click({ shiftKey: true });
             cy.contains('6 items selected');
         });
+
+        it('should allow to deselect a range with shift key', () => {
+            cy.contains('1-10 of 13'); // wait for data
+            cy.get(ListPagePosts.elements.selectItem).eq(0).click();
+            cy.get(ListPagePosts.elements.selectItem)
+                .eq(4)
+                .click({ shiftKey: true });
+            cy.contains('5 items selected');
+            cy.get(ListPagePosts.elements.selectedItem).should(els =>
+                expect(els).to.have.length(5)
+            );
+            cy.get(ListPagePosts.elements.selectItem)
+                .eq(2)
+                .click({ shiftKey: true });
+            cy.contains('2 items selected');
+            cy.get(ListPagePosts.elements.selectedItem).should(els =>
+                expect(els).to.have.length(2)
+            );
+        });
+
+        it('should allow alternating shift-select and shift-deselect', () => {
+            cy.contains('1-10 of 13'); // wait for data
+            cy.get(ListPagePosts.elements.selectItem).eq(0).click();
+            cy.get(ListPagePosts.elements.selectItem)
+                .eq(3)
+                .click({ shiftKey: true });
+            cy.contains('4 items selected');
+            cy.get(ListPagePosts.elements.selectItem)
+                .eq(4)
+                .click({ shiftKey: true });
+            cy.contains('5 items selected');
+            cy.get(ListPagePosts.elements.selectItem)
+                .eq(2)
+                .click({ shiftKey: true });
+            cy.contains('2 items selected');
+            cy.get(ListPagePosts.elements.selectItem)
+                .eq(4)
+                .click({ shiftKey: true });
+            cy.contains('5 items selected');
+            cy.get(ListPagePosts.elements.selectedItem).should(els =>
+                expect(els).to.have.length(5)
+            );
+        });
+
+        it('should support shift-deselect after select all then manual deselect', () => {
+            cy.contains('1-10 of 13'); // wait for data
+            ListPagePosts.toggleSelectAll();
+            cy.contains('10 items selected');
+            cy.get(ListPagePosts.elements.selectItem).eq(1).click();
+            cy.contains('9 items selected');
+            cy.get(ListPagePosts.elements.selectItem)
+                .eq(3)
+                .click({ shiftKey: true });
+            cy.contains('7 items selected');
+            cy.get(ListPagePosts.elements.selectedItem).should(els =>
+                expect(els).to.have.length(7)
+            );
+        });
     });
 
     describe('rowClick', () => {

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

@@ -1,20 +1,20 @@
+import difference from 'lodash/difference';
+import union from 'lodash/union';
 import * as React from 'react';
 import { useEffect, useMemo, useRef, type FC, type ReactNode } from 'react';
-import union from 'lodash/union';
-import difference from 'lodash/difference';
 
-import { OptionalResourceContextProvider, useResourceContext } from '../core';
-import { useEvent } from '../util';
 import { useListContextWithProps } from '../controller/list/useListContextWithProps';
 import { type ListControllerResult } from '../controller/list/useListController';
-import { type RowClickFunctionBase } from './types';
+import { OptionalResourceContextProvider, useResourceContext } from '../core';
 import { type Identifier, type RaRecord, type SortPayload } from '../types';
-import { DataTableConfigContext } from './DataTableConfigContext';
+import { useEvent } from '../util';
 import { DataTableCallbacksContext } from './DataTableCallbacksContext';
+import { DataTableConfigContext } from './DataTableConfigContext';
 import { DataTableDataContext } from './DataTableDataContext';
 import { DataTableSelectedIdsContext } from './DataTableSelectedIdsContext';
 import { DataTableSortContext } from './DataTableSortContext';
 import { DataTableStoreContext } from './DataTableStoreContext';
+import { type RowClickFunctionBase } from './types';
 
 export const DataTableBase = function DataTable<
     RecordType extends RaRecord = any,
@@ -76,8 +76,6 @@ export const DataTableBase = function DataTable<
             if (!data) return;
             const ids = data.map(record => record.id);
             const lastSelectedIndex = ids.indexOf(lastSelected.current);
-            // @ts-ignore FIXME useEvent prevents using event.currentTarget
-            lastSelected.current = event.target.checked ? id : null;
 
             if (event.shiftKey && lastSelectedIndex !== -1) {
                 const index = ids.indexOf(id);
@@ -86,10 +84,10 @@ export const DataTableBase = function DataTable<
                     Math.max(lastSelectedIndex, index) + 1
                 );
 
-                // @ts-ignore FIXME useEvent prevents using event.currentTarget
-                const newSelectedIds = event.target.checked
-                    ? union(selectedIds, idsBetweenSelections)
-                    : difference(selectedIds, idsBetweenSelections);
+                const isClickedItemSelected = selectedIds?.includes(id);
+                const newSelectedIds = isClickedItemSelected
+                    ? difference(selectedIds, idsBetweenSelections)
+                    : union(selectedIds, idsBetweenSelections);
 
                 onSelect?.(
                     isRowSelectable
@@ -103,6 +101,8 @@ export const DataTableBase = function DataTable<
             } else {
                 onToggleItem?.(id);
             }
+
+            lastSelected.current = id;
         }
     );
 

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

@@ -1,16 +1,16 @@
+import { fireEvent, render, screen, waitFor } from '@testing-library/react';
 import * as React from 'react';
-import { render, screen, fireEvent, waitFor } from '@testing-library/react';
 import {
     Basic,
     Columns,
     Empty,
-    StandaloneStatic,
-    StandaloneDynamic,
     Expand,
     ExpandSingle,
     IsRowExpandable,
     IsRowSelectable,
     NonPrimitiveData,
+    StandaloneDynamic,
+    StandaloneStatic,
 } from './DataTable.stories';
 
 describe('DataTable', () => {
@@ -245,6 +245,31 @@ describe('DataTable', () => {
             fireEvent.click(checkboxes[0]);
             await screen.findByText('2 items selected');
         });
+        it('should support alternating shift-select and shift-deselect', async () => {
+            render(<Basic />);
+            const checkboxes = await screen.findAllByRole('checkbox');
+            fireEvent.click(checkboxes[1]);
+            fireEvent.click(checkboxes[4], { shiftKey: true });
+            await screen.findByText('4 items selected');
+            fireEvent.click(checkboxes[5], { shiftKey: true });
+            await screen.findByText('5 items selected');
+            fireEvent.click(checkboxes[3], { shiftKey: true });
+            await screen.findByText('2 items selected');
+            fireEvent.click(checkboxes[5], { shiftKey: true });
+            await screen.findByText('5 items selected');
+        });
+        it('should support shift-deselect after select all then deselect one', async () => {
+            render(<Basic />);
+            const checkboxes = await screen.findAllByRole('checkbox');
+            fireEvent.click(checkboxes[0]);
+            const selectAllButton = await screen.findByText('Select all');
+            selectAllButton.click();
+            await screen.findByText('7 items selected');
+            fireEvent.click(checkboxes[2]);
+            await screen.findByText('6 items selected');
+            fireEvent.click(checkboxes[4], { shiftKey: true });
+            await screen.findByText('4 items selected');
+        });
     });
     describe('isRowSelectable', () => {
         it('should allow to disable row selection', async () => {