Skip to content

Commit

Permalink
fix(editors): select editor inline blur save before destroy
Browse files Browse the repository at this point in the history
  • Loading branch information
ghiscoding committed May 6, 2021
1 parent 19a777e commit 0e591b1
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 8 deletions.
9 changes: 9 additions & 0 deletions packages/common/src/editors/__tests__/checkboxEditor.spec.ts
Expand Up @@ -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);
Expand Down
16 changes: 16 additions & 0 deletions packages/common/src/editors/__tests__/selectEditor.spec.ts
Expand Up @@ -512,6 +512,7 @@ describe('SelectEditor', () => {

describe('save method', () => {
afterEach(() => {
editor.destroy();
jest.clearAllMocks();
});

Expand All @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions packages/common/src/editors/checkboxEditor.ts
Expand Up @@ -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) {
Expand Down
17 changes: 12 additions & 5 deletions packages/common/src/editors/selectEditor.ts
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions packages/common/src/services/gridEvent.service.ts
Expand Up @@ -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
Expand Down

0 comments on commit 0e591b1

Please sign in to comment.