fix: prevent recurring selection attempts during drag selection (#8317)

vaadin · Dec 10, 2024 · 892664d · 892664d
1 parent 40e49c5
commit 892664d
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 10 deletions.
diff --git a/packages/grid/src/vaadin-grid-selection-column-base-mixin.js b/packages/grid/src/vaadin-grid-selection-column-base-mixin.js
@@ -250,11 +250,7 @@ export const GridSelectionColumnBaseMixin = (superClass) =>
       } else if (event.detail.state === 'end') {
         // if drag start and end stays within the same item, then toggle its state
         if (this.__dragStartItem) {
-          if (this.__selectOnDrag) {
-            this._selectItem(this.__dragStartItem);
-          } else {
-            this._deselectItem(this.__dragStartItem);
-          }
+          this.__toggleItem(this.__dragStartItem, this.__selectOnDrag);
         }
         // clear drag state after timeout, which allows preventing the
         // subsequent click event if drag started and ended on the same item
@@ -320,6 +316,7 @@ export const GridSelectionColumnBaseMixin = (superClass) =>
       if (this.__dragStartIndex === undefined) {
         return;
       }
+
       // Get the row being hovered over
       const renderedRows = this._grid._getRenderedRows();
       const hoveredRow = renderedRows.find((row) => {
@@ -344,11 +341,7 @@ export const GridSelectionColumnBaseMixin = (superClass) =>
             (hoveredIndex > this.__dragStartIndex && row.index >= this.__dragStartIndex && row.index <= hoveredIndex) ||
             (hoveredIndex < this.__dragStartIndex && row.index <= this.__dragStartIndex && row.index >= hoveredIndex)
           ) {
-            if (this.__selectOnDrag) {
-              this._selectItem(row._item);
-            } else {
-              this._deselectItem(row._item);
-            }
+            this.__toggleItem(row._item, this.__selectOnDrag);
             this.__dragStartItem = undefined;
           }
         });
@@ -430,6 +423,14 @@ export const GridSelectionColumnBaseMixin = (superClass) =>
      * @private
      */
     __toggleItem(item, selected = !this._grid._isSelected(item)) {
+      if (selected === this._grid._isSelected(item)) {
+        // Skip selection if the item is already in the desired state.
+        // Note, _selectItem and _deselectItem may be overridden in custom
+        // selection column implementations, and calling them unnecessarily
+        // might affect performance (e.g. vaadin-grid-flow-selection-column).
+        return;
+      }
+
       if (selected) {
         this._selectItem(item);
       } else {

diff --git a/packages/grid/test/selection.common.js b/packages/grid/test/selection.common.js
@@ -689,6 +689,25 @@ describe('multi selection column', () => {
       expect(grid.selectedItems).to.eql(grid.items.slice(1, 4));
     });
 
+    it('should not attempt to select item on mouse drag if it is already selected', () => {
+      const selectItemSpy = sinon.spy(selectionColumn, '_selectItem');
+
+      const row0cell = getBodyCellContent(grid, 0, 0);
+      const row1cell = getBodyCellContent(grid, 1, 0);
+
+      grid.selectedItems = [rows[1]._item];
+
+      fireTrackEvent(row0cell, row0cell, 'start');
+      clock.tick(10);
+      fireTrackEvent(row1cell, row0cell, 'track');
+      clock.tick(10);
+      fireTrackEvent(row1cell, row0cell, 'end');
+      clock.tick(10);
+
+      expect(selectItemSpy).to.be.calledOnce;
+      expect(selectItemSpy.args[0][0]).to.not.equal('1');
+    });
+
     it('should not select any items on mouse drag when dragSelect is disabled', () => {
       selectionColumn.dragSelect = false;
 
@@ -775,6 +794,25 @@ describe('multi selection column', () => {
       expect(grid.selectedItems).to.empty;
     });
 
+    it('should not attempt to deselect item on mouse drag if it is already deselected', () => {
+      const deselectItemSpy = sinon.spy(selectionColumn, '_deselectItem');
+
+      const row0cell = getBodyCellContent(grid, 0, 0);
+      const row1cell = getBodyCellContent(grid, 1, 0);
+
+      grid.selectedItems = [rows[0]._item];
+
+      fireTrackEvent(row0cell, row0cell, 'start');
+      clock.tick(10);
+      fireTrackEvent(row1cell, row0cell, 'track');
+      clock.tick(10);
+      fireTrackEvent(row1cell, row0cell, 'end');
+      clock.tick(10);
+
+      expect(deselectItemSpy).to.be.calledOnce;
+      expect(deselectItemSpy.args[0][0]).to.not.equal('1');
+    });
+
     it('should prevent text selection on mouse dragging', () => {
       const spy = sinon.spy();
       const sourceCell = getBodyCellContent(grid, 0, 0);