From 0e591b1812fc1c733c03f7afcf81dee7a3e4b107 Mon Sep 17 00:00:00 2001 From: ghiscoding Date: Thu, 6 May 2021 10:06:46 -0400 Subject: [PATCH] fix(editors): select editor inline blur save before destroy --- .../editors/__tests__/checkboxEditor.spec.ts | 9 +++++++++ .../src/editors/__tests__/selectEditor.spec.ts | 16 ++++++++++++++++ packages/common/src/editors/checkboxEditor.ts | 7 +++++++ packages/common/src/editors/selectEditor.ts | 17 ++++++++++++----- .../common/src/services/gridEvent.service.ts | 6 +++--- 5 files changed, 47 insertions(+), 8 deletions(-) diff --git a/packages/common/src/editors/__tests__/checkboxEditor.spec.ts b/packages/common/src/editors/__tests__/checkboxEditor.spec.ts index 2de679770..bd5fdb16c 100644 --- a/packages/common/src/editors/__tests__/checkboxEditor.spec.ts +++ b/packages/common/src/editors/__tests__/checkboxEditor.spec.ts @@ -150,6 +150,15 @@ describe('CheckboxEditor', () => { expect(editor.getValue()).toBe(false); }); + it('should call "preClick" and expect the "checked" attribute to be toggled', () => { + editor = new CheckboxEditor(editorArguments); + const previousValue = editor.getValue(); + editor.preClick(); + const newValue = editor.getValue(); + + expect(previousValue).not.toEqual(newValue); + }); + it('should define an item datacontext containing a string as cell value and expect this value to be loaded in the editor when calling "loadValue"', () => { editor = new CheckboxEditor(editorArguments); editor.loadValue(mockItemData); diff --git a/packages/common/src/editors/__tests__/selectEditor.spec.ts b/packages/common/src/editors/__tests__/selectEditor.spec.ts index ec247e3c1..fb3def09d 100644 --- a/packages/common/src/editors/__tests__/selectEditor.spec.ts +++ b/packages/common/src/editors/__tests__/selectEditor.spec.ts @@ -512,6 +512,7 @@ describe('SelectEditor', () => { describe('save method', () => { afterEach(() => { + editor.destroy(); jest.clearAllMocks(); }); @@ -527,6 +528,21 @@ describe('SelectEditor', () => { expect(spy).toHaveBeenCalled(); }); + it('should call "save" and "getEditorLock" method when "hasAutoCommitEdit" is enabled and we are destroying the editor without it being saved yet', () => { + mockItemData = { id: 1, gender: 'male', isActive: true }; + gridOptionMock.autoCommitEdit = true; + const lockSpy = jest.spyOn(gridStub.getEditorLock(), 'commitCurrentEdit'); + + editor = new SelectEditor(editorArguments, true); + const saveSpy = jest.spyOn(editor, 'save'); + + editor.loadValue(mockItemData); + editor.destroy(); + + expect(saveSpy).toHaveBeenCalledWith(true); + expect(lockSpy).toHaveBeenCalled(); + }); + it('should not call "commitCurrentEdit" when "hasAutoCommitEdit" is disabled', () => { mockItemData = { id: 1, gender: 'male', isActive: true }; gridOptionMock.autoCommitEdit = false; diff --git a/packages/common/src/editors/checkboxEditor.ts b/packages/common/src/editors/checkboxEditor.ts index e8d40a931..cd7ced51b 100644 --- a/packages/common/src/editors/checkboxEditor.ts +++ b/packages/common/src/editors/checkboxEditor.ts @@ -141,6 +141,13 @@ export class CheckboxEditor implements Editor { } } + /** pre-click, when enabled, will simply toggle the checkbox without requiring to double-click */ + preClick() { + if (this._input) { + this._input.checked = !this._input.checked; + } + } + show() { const isCompositeEditor = !!this.args?.compositeEditorOptions; if (isCompositeEditor) { diff --git a/packages/common/src/editors/selectEditor.ts b/packages/common/src/editors/selectEditor.ts index b1b0b6e9a..ee713ca6c 100644 --- a/packages/common/src/editors/selectEditor.ts +++ b/packages/common/src/editors/selectEditor.ts @@ -37,7 +37,7 @@ export class SelectEditor implements Editor { // flag to signal that the editor is destroying itself, helps prevent // commit changes from being called twice and erroring - protected _destroying = false; + protected _isDisposing = false; /** Collection Service */ protected _collectionService!: CollectionService; @@ -399,7 +399,14 @@ export class SelectEditor implements Editor { } destroy() { - this._destroying = true; + // when autoCommitEdit is enabled, we might end up leave the editor without it being saved, if so do call a save before destroying + // this mainly happens doing a blur or focusing on another cell in the grid (it won't come here if we click outside the grid, in the body) + if (this.$editorElm && this.hasAutoCommitEdit && this.isValueChanged() && !this._isDisposing) { + this._isDisposing = true; // change destroying flag to avoid infinite loop + this.save(true); + } + + this._isDisposing = true; if (this.$editorElm && typeof this.$editorElm.multipleSelect === 'function') { this.$editorElm.multipleSelect('destroy'); this.$editorElm.remove(); @@ -502,7 +509,7 @@ export class SelectEditor implements Editor { } isValueChanged(): boolean { - const valueSelection = this.$editorElm.multipleSelect('getSelects'); + const valueSelection = this.$editorElm?.multipleSelect('getSelects'); if (this.isMultipleSelect) { const isEqual = dequal(valueSelection, this.originalValue); return !isEqual; @@ -535,11 +542,11 @@ export class SelectEditor implements Editor { } } - save() { + save(forceCommitCurrentEdit = false) { const validation = this.validate(); const isValid = validation?.valid ?? false; - if (!this._destroying && this.hasAutoCommitEdit && isValid) { + if ((!this._isDisposing || forceCommitCurrentEdit) && this.hasAutoCommitEdit && isValid) { // do not use args.commitChanges() as this sets the focus to the next row. // also the select list will stay shown when clicking off the grid this.grid.getEditorLock().commitCurrentEdit(); diff --git a/packages/common/src/services/gridEvent.service.ts b/packages/common/src/services/gridEvent.service.ts index 41073de33..25472b226 100644 --- a/packages/common/src/services/gridEvent.service.ts +++ b/packages/common/src/services/gridEvent.service.ts @@ -99,10 +99,10 @@ export class GridEventService { const column: Column = grid && grid.getColumns && grid.getColumns()[args.cell]; const gridOptions: GridOption = grid && grid.getOptions && grid.getOptions() || {}; - // only when using autoCommitEdit, we will make the cell active (in focus) when clicked - // setting the cell as active as a side effect and if autoCommitEdit is set to false then the Editors won't save correctly + // only when the grid option "autoCommitEdit" is enabled, we will make the cell active (in focus) when clicked + // setting the cell as active as a side effect and if "autoCommitEdit" is set to false then the Editors won't save correctly if (gridOptions.enableCellNavigation && (!gridOptions.editable || (gridOptions.editable && gridOptions.autoCommitEdit))) { - grid.setActiveCell(args.row, args.cell); + grid.setActiveCell(args.row, args.cell, false, false, true); } // if the column definition has a onCellClick property (a callback function), then run it