fix: make conditional editing work with lazy columns (#11405) (#11410)

* fix: make conditional editing work with lazy columns

* fix tests

* handle cell editor navigation when using lazy column rendering

* make getEventContext work with lazily rendered cells

* stop editing when edited cell is scrolled out of view

Co-authored-by: Sascha Ißbrücker <sissbruecker@vaadin.com>

Author: vaadin-bot

packages/grid-pro/src/vaadin-grid-pro-inline-editing-mixin.js

@@ -325,6 +325,19 @@ export const InlineEditingMixin = (superClass) =>
       this._debouncerStopEdit = Debouncer.debounce(this._debouncerStopEdit, animationFrame, this._stopEdit.bind(this));
     }
 
+    /**
+     * Override method from ScrollMixin to stop editing if the edited cell
+     * is scrolled out of the view and removed from the DOM.
+     * @private
+     */
+    __updateColumnsBodyContentHidden() {
+      super.__updateColumnsBodyContentHidden();
+
+      if (this.__edited && !this.__edited.cell.isConnected) {
+        this._stopEdit(true, false);
+      }
+    }
+
     /** @private */
     __shouldIgnoreFocusOut(event) {
       const edited = this.__edited;
@@ -357,9 +370,11 @@ export const InlineEditingMixin = (superClass) =>
       // Cancel debouncer enqueued on focusout
       this._cancelStopEdit();
 
-      this._scrollHorizontallyToCell(cell);
+      // Scroll column into view synchronously, which also triggers lazy column
+      // rendering to ensure cells for that column are in the DOM.
+      this.scrollToColumn(column);
 
-      const model = this.__getRowModel(cell.parentElement);
+      const model = this.__getRowModel(cell.__parentRow);
       this.__edited = { cell, column, model };
       column._startCellEdit(cell, model);
 
@@ -505,7 +520,7 @@ export const InlineEditingMixin = (superClass) =>
           // Stop looking if the next cell is editable
           const nextRow = this._getRowByIndex(nextIndex);
           // eslint-disable-next-line @typescript-eslint/no-loop-func
-          nextCell = nextRow && Array.from(nextRow.children).find((cell) => cell._column === nextColumn);
+          nextCell = nextRow && Array.from(nextRow.__cells).find((cell) => cell._column === nextColumn);
           if (nextCell && this._isCellEditable(nextCell)) {
             break;
           }
@@ -537,7 +552,7 @@ export const InlineEditingMixin = (superClass) =>
         if (!this._isCellEditable(cell)) {
           // Cell is no longer editable, cancel edit
           this._stopEdit(true, true);
-        } else if (cell.parentNode === row && item && this.getItemId(model.item) !== this.getItemId(item)) {
+        } else if (cell.__parentRow === row && item && this.getItemId(model.item) !== this.getItemId(item)) {
           // Edited item identity has changed, stop edit
           this._stopEdit();
         }
@@ -573,7 +588,7 @@ export const InlineEditingMixin = (superClass) =>
         return true;
       }
       // Otherwise, check isCellEditable function
-      const model = this.__getRowModel(cell.parentElement);
+      const model = this.__getRowModel(cell.__parentRow);
       return column.isCellEditable(model);
     }
 

packages/grid-pro/test/edit-column.test.js

@@ -1,6 +1,16 @@
 import { expect } from '@vaadin/chai-plugins';
 import { sendKeys } from '@vaadin/test-runner-commands';
-import { enter, esc, fixtureSync, focusin, focusout, nextFrame } from '@vaadin/testing-helpers';
+import {
+  aTimeout,
+  enter,
+  esc,
+  fixtureSync,
+  focusin,
+  focusout,
+  isFirefox,
+  nextFrame,
+  oneEvent,
+} from '@vaadin/testing-helpers';
 import sinon from 'sinon';
 import './not-animated-styles.js';
 import '../src/vaadin-grid-pro.js';
@@ -10,6 +20,7 @@ import {
   dblclick,
   flatMap,
   flushGrid,
+  getCellContent,
   getCellEditor,
   getContainerCell,
   getRowCells,
@@ -570,6 +581,111 @@ describe('edit column', () => {
         });
       });
     });
+
+    describe('lazy column rendering', () => {
+      const UPDATE_CONTENT_VISIBILITY_DEBOUNCER_TIMEOUT = 100;
+
+      async function scrollHorizontally(delta) {
+        grid.$.table.scrollLeft += delta;
+        await oneEvent(grid.$.table, 'scroll');
+        await aTimeout(UPDATE_CONTENT_VISIBILITY_DEBOUNCER_TIMEOUT);
+      }
+
+      function getCellByColumnPath(columnPath) {
+        const row = getRows(grid.$.items)[0];
+        const cells = getRowCells(row);
+        const cell = cells.find((c) => c._column.path === columnPath);
+        expect(cell, `Could not find cell for column with path ${columnPath}`).to.be.ok;
+
+        return cell;
+      }
+
+      beforeEach(() => {
+        grid = fixtureSync(`<vaadin-grid-pro style="width: 400px;"></vaadin-grid-pro>`);
+
+        const columns = [];
+        for (let i = 0; i < 8; i++) {
+          const column = document.createElement('vaadin-grid-pro-edit-column');
+          column.path = `col${i}`;
+          column.header = `Col ${i}`;
+          column.width = '100px';
+          column.flexGrow = 0;
+          columns.push(column);
+          grid.appendChild(column);
+        }
+
+        grid.items = [
+          { col0: 'a0', col1: 'a1', col2: 'a2', col3: 'a3', col4: 'a4', col5: 'a5', col6: 'a6', col7: 'a7' },
+          { col0: 'b0', col1: 'b1', col2: 'b2', col3: 'b3', col4: 'b4', col5: 'b5', col6: 'b6', col7: 'b7' },
+        ];
+
+        // Make odd-indexed columns conditionally editable
+        columns.forEach((column, i) => {
+          column.isCellEditable = () => i % 2 === 1;
+        });
+
+        grid.columnRendering = 'lazy';
+        flushGrid(grid);
+      });
+
+      it('should mark initially visible cells as non-editable based on isCellEditable', () => {
+        expect(hasEditablePart(0, 0)).to.be.false;
+        expect(hasEditablePart(0, 1)).to.be.true;
+      });
+
+      it('should mark lazily rendered cells as non-editable based on isCellEditable after scrolling', async () => {
+        // Scroll far enough to reveal the last columns
+        await scrollHorizontally(400);
+
+        const cells = getRowCells(getRows(grid.$.items)[0]);
+
+        // Verify we are testing correct cells
+        expect(getCellContent(cells[cells.length - 2]).textContent).to.equal('a6');
+        expect(getCellContent(cells[cells.length - 1]).textContent).to.equal('a7');
+
+        expect(hasEditablePart(0, cells.length - 2)).to.be.false;
+        expect(hasEditablePart(0, cells.length - 1)).to.be.true;
+      });
+
+      // Focus button mode that is active on MacOS causes issues with Tab key navigation in Firefox when run with Playwright
+      (isFirefox && isMac ? it.skip : it)('should navigate through editable cells with Tab', async () => {
+        let cell = getCellByColumnPath('col1');
+        cell.focus();
+        await sendKeys({ press: 'Enter' });
+        expect(getCellEditor(cell)).to.be.ok;
+
+        await sendKeys({ press: 'Tab' });
+        cell = getCellByColumnPath('col3');
+        expect(getCellEditor(cell)).to.be.ok;
+
+        await sendKeys({ press: 'Tab' });
+        cell = getCellByColumnPath('col5');
+        expect(getCellEditor(cell)).to.be.ok;
+
+        await sendKeys({ press: 'Tab' });
+        cell = getCellByColumnPath('col7');
+        expect(getCellEditor(cell)).to.be.ok;
+      });
+
+      it('should edit cell again after it was temporarily removed due to scrolling', async () => {
+        const cell = getCellByColumnPath('col1');
+        cell.focus();
+        await sendKeys({ press: 'Enter' });
+        expect(getCellEditor(cell)).to.be.ok;
+
+        await scrollHorizontally(500);
+
+        expect(cell.isConnected).to.be.false;
+
+        await scrollHorizontally(-500);
+
+        expect(getCellEditor(cell)).to.not.be.ok;
+
+        cell.focus();
+        await sendKeys({ press: 'Enter' });
+        expect(getCellEditor(cell)).to.be.ok;
+      });
+    });
   });
 
   describe('vertical scrolling', () => {

packages/grid/src/vaadin-grid-event-context-mixin.js

@@ -49,7 +49,7 @@ export const EventContextMixin = (superClass) =>
       }
 
       if (context.section === 'body' || context.section === 'details') {
-        Object.assign(context, this.__getRowModel(cell.parentElement));
+        Object.assign(context, this.__getRowModel(cell.__parentRow));
       }
 
       return context;

packages/grid/src/vaadin-grid-mixin.js

@@ -536,6 +536,7 @@ export const GridMixin = (superClass) =>
                 contentsFragment.appendChild(detailsCell._content);
               }
               this._configureDetailsCell(detailsCell);
+              detailsCell.__parentRow = row;
               row.appendChild(detailsCell);
               // Cache the details cell reference
               row.__detailsCell = detailsCell;