fix: remove custom part names from loading grid rows (#7469)

vaadin · Jun 4, 2024 · 07d0f0f · 07d0f0f
1 parent e30e403
commit 07d0f0f
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 6 deletions.
diff --git a/packages/grid-pro/src/vaadin-grid-pro-inline-editing-mixin.js b/packages/grid-pro/src/vaadin-grid-pro-inline-editing-mixin.js
@@ -520,7 +520,7 @@ export const InlineEditingMixin = (superClass) =>
       super._generateCellPartNames(row, model);
 
       iterateRowCells(row, (cell) => {
-        const isEditable = this._isCellEditable(cell);
+        const isEditable = !row.hasAttribute('loading') && this._isCellEditable(cell);
         const target = cell._focusButton || cell;
         updatePart(target, isEditable, 'editable-cell');
       });

diff --git a/packages/grid-pro/test/edit-column.common.js b/packages/grid-pro/test/edit-column.common.js
@@ -372,6 +372,45 @@ describe('edit column', () => {
       expect(triggersNavigatingState(1, 2)).to.be.true;
     });
 
+    describe('async data provider', () => {
+      let dataProviderCallbacks;
+
+      function flushDataProvider() {
+        dataProviderCallbacks.forEach((cb) => cb()); // NOSONAR
+        dataProviderCallbacks = [];
+      }
+
+      beforeEach(() => {
+        grid.items = undefined;
+        dataProviderCallbacks = [];
+        /* prettier-ignore */
+        grid.dataProvider = ({ page, pageSize }, callback) => { // NOSONAR
+          const items = [...Array(pageSize).keys()].map((i) => {
+            return { 
+              id: page * pageSize + i,
+              status: 'draft',
+              amount: 100,
+              notes: 'foo'
+            };
+          });
+
+          dataProviderCallbacks.push(() => callback(items, pageSize * 2));
+        };
+        amountColumn.isCellEditable = () => true; // NOSONAR
+      });
+
+      it('should add editable-cell part to rows that are not in loading state ', () => {
+        flushDataProvider();
+        expect(hasEditablePart(1, 2)).to.be.true;
+      });
+
+      it('should remove editable-cell part for rows that enter loading state', () => {
+        flushDataProvider();
+        grid.clearCache();
+        expect(hasEditablePart(1, 2)).to.be.false;
+      });
+    });
+
     describe('editor navigation', () => {
       beforeEach(async () => {
         // Five rows, only second and forth are editable

diff --git a/packages/grid/src/vaadin-grid-data-provider-mixin.js b/packages/grid/src/vaadin-grid-data-provider-mixin.js
@@ -234,6 +234,12 @@ export const DataProviderMixin = (superClass) =>
 
       // Cells part attribute
       updateCellsPart(cells, 'loading-row-cell', loading);
+
+      if (loading) {
+        // Run style generators for the loading row to have custom names cleared
+        this._generateCellClassNames(row);
+        this._generateCellPartNames(row);
+      }
     }
 
     /**

diff --git a/packages/grid/src/vaadin-grid-styling-mixin.js b/packages/grid/src/vaadin-grid-styling-mixin.js
@@ -89,7 +89,7 @@ export const StylingMixin = (superClass) =>
      */
     generateCellClassNames() {
       iterateChildren(this.$.items, (row) => {
-        if (!row.hidden && !row.hasAttribute('loading')) {
+        if (!row.hidden) {
           this._generateCellClassNames(row, this.__getRowModel(row));
         }
       });
@@ -103,7 +103,7 @@ export const StylingMixin = (superClass) =>
      */
     generateCellPartNames() {
       iterateChildren(this.$.items, (row) => {
-        if (!row.hidden && !row.hasAttribute('loading')) {
+        if (!row.hidden) {
           this._generateCellPartNames(row, this.__getRowModel(row));
         }
       });
@@ -115,7 +115,7 @@ export const StylingMixin = (superClass) =>
         if (cell.__generatedClasses) {
           cell.__generatedClasses.forEach((className) => cell.classList.remove(className));
         }
-        if (this.cellClassNameGenerator) {
+        if (this.cellClassNameGenerator && !row.hasAttribute('loading')) {
           const result = this.cellClassNameGenerator(cell._column, model);
           cell.__generatedClasses = result && result.split(' ').filter((className) => className.length > 0);
           if (cell.__generatedClasses) {
@@ -134,7 +134,7 @@ export const StylingMixin = (superClass) =>
             updatePart(cell, null, partName);
           });
         }
-        if (this.cellPartNameGenerator) {
+        if (this.cellPartNameGenerator && !row.hasAttribute('loading')) {
           const result = this.cellPartNameGenerator(cell._column, model);
           cell.__generatedParts = result && result.split(' ').filter((partName) => partName.length > 0);
           if (cell.__generatedParts) {

diff --git a/packages/grid/test/styling.common.js b/packages/grid/test/styling.common.js
@@ -155,6 +155,20 @@ describe('styling', () => {
         clock.tick(10);
         grid[requestFn]();
         expect(spy.called).to.be.true;
+        expect(spy.getCalls().filter((call) => call.args[1].index === 0).length).to.be.lessThan(5); // NOSONAR
+      });
+
+      it(`should remove custom ${entries} for rows that enter loading state`, () => {
+        grid[generatorFn] = () => 'foo'; // NOSONAR
+        clock.tick(10);
+
+        expect(grid._getRenderedRows()[0].hasAttribute('loading')).to.be.false;
+        assertCallback(['foo']);
+
+        grid.clearCache();
+
+        expect(grid._getRenderedRows()[0].hasAttribute('loading')).to.be.true;
+        assertCallback([]);
       });
     });
   }
@@ -183,7 +197,9 @@ describe('styling', () => {
 
     const assertPartNames = (expectedParts, row = 0, col = 0) => {
       const cell = getContainerCell(grid.$.items, row, col);
-      const actualPart = cell.getAttribute('part');
+      let actualPart = cell.getAttribute('part');
+      // Remove "loading-row-cell" since initialCellPart doesn't include it and it's irrelevant for the test
+      actualPart = actualPart.replace('loading-row-cell', '').trim();
       const customParts = expectedParts.length ? ` ${expectedParts.join(' ')}` : '';
 
       if (row === 0 && col === 0) {