From 6a6eab978e84934a9a3239ba3badf3a1b89f3e7a Mon Sep 17 00:00:00 2001 From: Salvatore Previti Date: Wed, 21 Jan 2026 17:54:59 +0000 Subject: [PATCH] AG-16610 full row editing leaves leaves empty editor bug (#12938) * AG-16610-full-row-editing-leaves-leaves-empty-editor-bug * AG-16610-full-row-editing-leaves-leaves-empty-editor-bug * AG-16610-full-row-editing-leaves-leaves-empty-editor-bug * AG-16610-full-row-editing-leaves-leaves-empty-editor-bug --- .../src/edit/strategy/fullRowEditStrategy.ts | 36 +++++- .../src/edit/utils/editors.ts | 17 +-- .../cell-editing-regression.test.ts | 114 ++++++++++++++++++ 3 files changed, 159 insertions(+), 8 deletions(-) diff --git a/packages/ag-grid-community/src/edit/strategy/fullRowEditStrategy.ts b/packages/ag-grid-community/src/edit/strategy/fullRowEditStrategy.ts index 182cd6b1668..826308d0656 100644 --- a/packages/ag-grid-community/src/edit/strategy/fullRowEditStrategy.ts +++ b/packages/ag-grid-community/src/edit/strategy/fullRowEditStrategy.ts @@ -6,7 +6,12 @@ import type { EditPosition, EditRowPosition, StartEditWithPositionParams } from import type { IRowNode } from '../../interfaces/iRowNode'; import type { CellCtrl } from '../../rendering/cell/cellCtrl'; import { _getCellCtrl, _getRowCtrl } from '../utils/controllers'; -import { _populateModelValidationErrors, _setupEditor, _sourceAndPendingDiffer } from '../utils/editors'; +import { + _destroyEditor, + _populateModelValidationErrors, + _setupEditor, + _sourceAndPendingDiffer, +} from '../utils/editors'; import type { EditValidationAction, EditValidationResult } from './baseEditStrategy'; import { BaseEditStrategy } from './baseEditStrategy'; @@ -188,12 +193,41 @@ export class FullRowEditStrategy extends BaseEditStrategy { public override cleanupEditors(position: EditRowPosition = {}, includeEditing?: boolean): void { super.cleanupEditors(position, includeEditing); + for (const rowNode of this.startedRows) { this.dispatchRowEvent({ rowNode }, 'rowEditingStopped'); + this.destroyEditorsForRow(rowNode); } this.startedRows.length = 0; } + /** + * Destroys all editors for a row that started full row editing, including editors + * that are not represented in the edit model (e.g. empty/unedited editors). + */ + private destroyEditorsForRow(rowNode: IRowNode): void { + const rowCtrl = _getRowCtrl(this.beans, { rowNode }); + if (!rowCtrl) { + return; // Row not rendered, no editors to destroy. + } + + const destroyedColumns = new Set(); + for (const cellCtrl of rowCtrl.getAllCellCtrls()) { + const column = cellCtrl.column; + if (destroyedColumns.has(column)) { + continue; // Column editor already processed. + } + destroyedColumns.add(column); + + if (!cellCtrl.comp?.getCellEditor()) { + continue; // No editor to destroy. + } + + // Destroy every editor created for this row, including those without edit model entries. + _destroyEditor(this.beans, cellCtrl, undefined, cellCtrl); + } + } + // returns null if no navigation should be performed public override moveToNextEditingCell( prevCell: CellCtrl, diff --git a/packages/ag-grid-community/src/edit/utils/editors.ts b/packages/ag-grid-community/src/edit/utils/editors.ts index 825e039d077..5ad94a97ddc 100644 --- a/packages/ag-grid-community/src/edit/utils/editors.ts +++ b/packages/ag-grid-community/src/edit/utils/editors.ts @@ -432,11 +432,11 @@ type DestroyEditorParams = { event?: Event | null; silent?: boolean; cancel?: bo export function _destroyEditor( beans: BeanCollection, position: Required, - params?: DestroyEditorParams + params?: DestroyEditorParams, + cellCtrl = _getCellCtrl(beans, position) ): void { const enableGroupEditing = beans.gos.get('enableGroupEdit'); - const { editModelSvc } = beans; - const cellCtrl = _getCellCtrl(beans, position); + const editModelSvc = beans.editModelSvc; const edit = editModelSvc?.getEdit(position, true); @@ -448,10 +448,11 @@ export function _destroyEditor( return; } - const { comp } = cellCtrl; + const comp = cellCtrl.comp; + const cellEditor = comp?.getCellEditor(); // editor already cleaned up, refresh cell (React usually) - if (comp && !comp.getCellEditor()) { + if (comp && !cellEditor) { cellCtrl?.refreshCell(); if (edit) { @@ -470,7 +471,7 @@ export function _destroyEditor( } if (_hasValidationRules(beans)) { - const errorMessages = comp?.getCellEditor()?.getValidationErrors?.(); + const errorMessages = edit && cellEditor?.getValidationErrors?.(); const cellValidationModel = editModelSvc?.getCellValidationModel(); if (errorMessages?.length) { @@ -480,7 +481,9 @@ export function _destroyEditor( } } - editModelSvc?.setEdit(position, { state: 'changed' }); + if (edit) { + editModelSvc?.setEdit(position, { state: 'changed' }); + } comp?.setEditDetails(); // passing nothing stops editing comp?.refreshEditStyles(false, false); diff --git a/testing/behavioural/src/cell-editing/cell-editing-regression.test.ts b/testing/behavioural/src/cell-editing/cell-editing-regression.test.ts index 8287d41d848..19b072bf51d 100644 --- a/testing/behavioural/src/cell-editing/cell-editing-regression.test.ts +++ b/testing/behavioural/src/cell-editing/cell-editing-regression.test.ts @@ -18,6 +18,7 @@ import { asyncSetTimeout, fakeElementAttribute, getAllRows, + getRowHtmlElement, waitForInput, waitForPopup, } from '../test-utils'; @@ -106,6 +107,119 @@ describe('Cell Editing Regression', () => { expect(modelCellRow1).toHaveTextContent('Updated'); }); + test('full-row editing fires rowEditingStopped on stopEditing', async () => { + const onRowEditingStopped = vi.fn(); + + const api = await gridMgr.createGridAndWait('myGrid', { + columnDefs: [{ field: 'make' }, { field: 'model' }], + defaultColDef: { + editable: true, + }, + editType: 'fullRow', + rowData: [ + { make: 'Toyota', model: 'Celica' }, + { make: 'Ford', model: 'Mondeo' }, + ], + onRowEditingStopped, + }); + + const gridDiv = getGridElement(api)! as HTMLElement; + await asyncSetTimeout(1); + + const makeCellRow0 = getByTestId(gridDiv, agTestIdFor.cell('0', 'make')); + await userEvent.dblClick(makeCellRow0); + await waitForInput(gridDiv, makeCellRow0, { popup: false }); + + api.stopEditing(); + await asyncSetTimeout(1); + + expect(onRowEditingStopped).toHaveBeenCalledTimes(1); + expect(onRowEditingStopped.mock.calls[0][0].rowIndex).toBe(0); + }); + + test('full-row editing closes empty editors when tabbing to next row', async () => { + const api = await gridMgr.createGridAndWait('myGrid', { + columnDefs: [{ field: 'make' }, { field: 'model' }, { field: 'model3' }], + defaultColDef: { + editable: true, + }, + editType: 'fullRow', + rowData: [ + { make: 'Toyota', model: 'Celica', model3: undefined }, + { make: 'Ford', model: 'Mondeo', model3: undefined }, + ], + }); + + const gridDiv = getGridElement(api)! as HTMLElement; + await asyncSetTimeout(1); + + const makeCellRow0 = getByTestId(gridDiv, agTestIdFor.cell('0', 'make')); + await userEvent.dblClick(makeCellRow0); + await waitForInput(gridDiv, makeCellRow0, { popup: false }); + expect(getRowHtmlElement(api, '0')?.classList.contains('ag-row-editing')).toBe(true); + expect(getRowHtmlElement(api, '1')?.classList.contains('ag-row-editing')).toBe(false); + expect(api.isEditing({ rowIndex: 0, rowPinned: undefined, column: api.getColumn('model3')! })).toBe(true); + + await userEvent.keyboard('{Tab}{Tab}{Tab}'); + const makeCellRow1 = getByTestId(gridDiv, agTestIdFor.cell('1', 'make')); + await waitForInput(gridDiv, makeCellRow1, { popup: false }); + + await waitFor(() => { + const editingCells = api.getEditingCells(); + expect(editingCells.length).toBeGreaterThan(0); + expect(editingCells.every((cell) => cell.rowIndex === 1)).toBe(true); + }); + expect(getRowHtmlElement(api, '0')?.classList.contains('ag-row-editing')).toBe(false); + expect(getRowHtmlElement(api, '1')?.classList.contains('ag-row-editing')).toBe(true); + expect(api.isEditing({ rowIndex: 0, rowPinned: undefined, column: api.getColumn('model3')! })).toBe(false); + expect(api.isEditing({ rowIndex: 1, rowPinned: undefined, column: api.getColumn('model3')! })).toBe(true); + + const emptyCellRow0 = getByTestId(gridDiv, agTestIdFor.cell('0', 'model3')); + expect(emptyCellRow0.querySelector('input')).toBeNull(); + }); + + test('full-row editing closes empty editors when shift-tabbing to previous row', async () => { + const api = await gridMgr.createGridAndWait('myGrid', { + columnDefs: [{ field: 'make' }, { field: 'model' }, { field: 'model3' }], + defaultColDef: { + editable: true, + }, + editType: 'fullRow', + rowData: [ + { make: 'Toyota', model: 'Celica', model3: undefined }, + { make: 'Ford', model: 'Mondeo', model3: undefined }, + ], + }); + + const gridDiv = getGridElement(api)! as HTMLElement; + await asyncSetTimeout(1); + + const makeCellRow1 = getByTestId(gridDiv, agTestIdFor.cell('1', 'make')); + await userEvent.dblClick(makeCellRow1); + await waitForInput(gridDiv, makeCellRow1, { popup: false }); + expect(getRowHtmlElement(api, '1')?.classList.contains('ag-row-editing')).toBe(true); + expect(getRowHtmlElement(api, '0')?.classList.contains('ag-row-editing')).toBe(false); + expect(api.isEditing({ rowIndex: 0, rowPinned: undefined, column: api.getColumn('model3')! })).toBe(false); + expect(api.isEditing({ rowIndex: 1, rowPinned: undefined, column: api.getColumn('model3')! })).toBe(true); + + await userEvent.keyboard('{Shift>}{Tab}{/Shift}'); + const model3CellRow0 = getByTestId(gridDiv, agTestIdFor.cell('0', 'model3')); + await waitForInput(gridDiv, model3CellRow0, { popup: false }); + + await waitFor(() => { + const editingCells = api.getEditingCells(); + expect(editingCells.length).toBeGreaterThan(0); + expect(editingCells.every((cell) => cell.rowIndex === 0)).toBe(true); + }); + expect(getRowHtmlElement(api, '0')?.classList.contains('ag-row-editing')).toBe(true); + expect(getRowHtmlElement(api, '1')?.classList.contains('ag-row-editing')).toBe(false); + expect(api.isEditing({ rowIndex: 0, rowPinned: undefined, column: api.getColumn('model3')! })).toBe(true); + expect(api.isEditing({ rowIndex: 1, rowPinned: undefined, column: api.getColumn('model3')! })).toBe(false); + + const emptyCellRow1 = getByTestId(gridDiv, agTestIdFor.cell('1', 'model3')); + expect(emptyCellRow1.querySelector('input')).toBeNull(); + }); + // AG-15698 - row doesn't rerender after value is selected in rich select editor test('cell not refreshed after richSelectEditor select', async () => { // virtualList doesn't add option elements if the offsetHeight is 0, so we need to fake it