fix: add and remove drag-source when scrolling (#7663)

* fix: add/remove drag-source when scrolling * address feedback * add an expect to a test * fix: update tests and component prop default value * fix: reorder and update tests * reorder test * remove redundant rowsDraggable
vaadin · Aug 27, 2024 · 067c827 · 067c827
1 parent f50d8c2
commit 067c827
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 18 deletions.
diff --git a/packages/grid/src/vaadin-grid-drag-and-drop-mixin.js b/packages/grid/src/vaadin-grid-drag-and-drop-mixin.js
@@ -103,6 +103,11 @@ export const DragAndDropMixin = (superClass) =>
         __dndAutoScrollThreshold: {
           value: 50,
         },
+
+        /** @private  */
+        __draggedItems: {
+          value: () => [],
+        },
       };
     }
 
@@ -170,6 +175,8 @@ export const DragAndDropMixin = (superClass) =>
             .filter((row) => !this.dragFilter || this.dragFilter(this.__getRowModel(row)));
         }
 
+        this.__draggedItems = rows.map((row) => row._item);
+
         // Set the default transfer data
         e.dataTransfer.setData('text', this.__formatDefaultTransferData(rows));
 
@@ -181,14 +188,12 @@ export const DragAndDropMixin = (superClass) =>
           updateBooleanRowStates(row, { dragstart: false });
           this.style.setProperty('--_grid-drag-start-x', '');
           this.style.setProperty('--_grid-drag-start-y', '');
-          rows.forEach((row) => {
-            updateBooleanRowStates(row, { 'drag-source': true });
-          });
+          this.requestContentUpdate();
         });
 
         const event = new CustomEvent('grid-dragstart', {
           detail: {
-            draggedItems: rows.map((row) => row._item),
+            draggedItems: [...this.__draggedItems],
             setDragData: (type, data) => e.dataTransfer.setData(type, data),
             setDraggedItemsCount: (count) => row.setAttribute('dragstart', count),
           },
@@ -206,9 +211,8 @@ export const DragAndDropMixin = (superClass) =>
       event.originalEvent = e;
       this.dispatchEvent(event);
 
-      iterateChildren(this.$.items, (row) => {
-        updateBooleanRowStates(row, { 'drag-source': false });
-      });
+      this.__draggedItems = [];
+      this.requestContentUpdate();
     }
 
     /** @private */
@@ -338,6 +342,11 @@ export const DragAndDropMixin = (superClass) =>
       });
     }
 
+    /** @private */
+    __updateDragSourceParts(row, model) {
+      updateBooleanRowStates(row, { 'drag-source': this.__draggedItems.includes(model.item) });
+    }
+
     /** @private */
     _onDrop(e) {
       if (this.dropMode) {

diff --git a/packages/grid/src/vaadin-grid-mixin.js b/packages/grid/src/vaadin-grid-mixin.js
@@ -968,6 +968,7 @@ export const GridMixin = (superClass) =>
       this._generateCellClassNames(row, model);
       this._generateCellPartNames(row, model);
       this._filterDragAndDrop(row, model);
+      this.__updateDragSourceParts(row, model);
 
       iterateChildren(row, (cell) => {
         if (cell._column && !cell._column.isConnected) {

diff --git a/packages/grid/test/drag-and-drop.common.js b/packages/grid/test/drag-and-drop.common.js
@@ -266,26 +266,21 @@ describe('drag and drop', () => {
         }
       });
 
-      it('should add drag-source- part only to dragged rows', async () => {
+      it('should add drag-source- part to dragged rows', async () => {
         fireDragStart();
-        let cells = getRowBodyCells(getRows(grid.$.items)[0]);
         await nextFrame();
-        for (const cell of cells) {
+        for (const cell of getRowBodyCells(getRows(grid.$.items)[0])) {
           expect(cell.getAttribute('part')).to.contain('drag-source-row-cell');
         }
-        cells = getRowBodyCells(getRows(grid.$.items)[1]);
-        for (const cell of cells) {
-          expect(cell.getAttribute('part')).to.not.contain('drag-source-row-cell');
-        }
       });
 
-      it('should remove drag-source- part from row when drag ends', async () => {
+      it('should remove drag-source- part from dragged rows', async () => {
         fireDragStart();
-        const row = getRows(grid.$.items)[0];
-        const cells = getRowBodyCells(row);
         await nextFrame();
+
         fireDragEnd();
-        for (const cell of cells) {
+        await nextFrame();
+        for (const cell of getRowBodyCells(getRows(grid.$.items)[0])) {
           expect(cell.getAttribute('part')).to.not.contain('drag-source-row-cell');
         }
       });
@@ -1073,5 +1068,19 @@ describe('drag and drop', () => {
       fireDragOver(grid.__getViewportRows()[0], 'above');
       expect(grid.$.table.scrollTop).to.be.within(scrollTop - 200, scrollTop - 100);
     });
+
+    it('should add/remove drag-source- part when scrolling', () => {
+      grid.rowsDraggable = true;
+      grid.selectItem(grid.items[0]);
+      fireDragStart();
+
+      // Scroll down so that the drag source cell leaves the viewport
+      grid.scrollToIndex(50);
+      // Expect no cells with drag-source-row-cell part in the DOM
+      expect(grid.$.items.querySelector('tr:not([hidden]) [part~="drag-source-row-cell"]')).to.be.null;
+
+      grid.scrollToIndex(0);
+      expect(getFirstCell(grid).getAttribute('part')).to.contain('drag-source-row-cell');
+    });
   });
 });