Skip to content

Commit

Permalink
fix(core): Autocomplete Editor shouldn't navigate down twice on enter
Browse files Browse the repository at this point in the history
- when grid options are set to `{ autoEdit: true, autoCommitEdit: false }` the Autocomplete Editor was executing navigate down twice because when we choose an item from the autocomplete with ENTER key, it also executes a navigate down
  • Loading branch information
ghiscoding committed Sep 10, 2024
1 parent 606bcda commit 4f9eb36
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 10 deletions.
30 changes: 29 additions & 1 deletion packages/common/src/core/__tests__/slickGrid.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, test, vi } from 'vitest';
import { type BasePubSubService } from '@slickgrid-universal/event-pub-sub';
import { createDomElement } from '@slickgrid-universal/utils';

import { CheckboxEditor, InputEditor, LongTextEditor } from '../../editors';
import { AutocompleterEditor, CheckboxEditor, InputEditor, LongTextEditor } from '../../editors';
import { SlickCellSelectionModel, SlickRowSelectionModel } from '../../extensions';
import type { Column, Editor, FormatterResultWithHtml, FormatterResultWithText, GridOption, EditCommand } from '../../interfaces';
import { SlickEventData, SlickGlobalEditorLock } from '../slickCore';
Expand Down Expand Up @@ -4643,6 +4643,34 @@ describe('SlickGrid core file', () => {

expect(editorSpy).toHaveBeenCalledWith({ id: 0, name: 'Avery', age: 44 });
});

it('should call navigateDown() when calling save() on default editor', () => {
const columns = [{ id: 'name', field: 'name', name: 'Name' }, { id: 'age', field: 'age', name: 'Age', editorClass: InputEditor }] as Column[];
const items = [{ id: 0, name: 'Avery', age: 44 }, { id: 1, name: 'Bob', age: 20 }, { id: 2, name: 'Rachel', age: 46 },];

const navigateDownSpy = vi.spyOn(grid, 'navigateDown');
grid = new SlickGrid<any, Column>(container, items, columns, { ...defaultOptions, enableCellNavigation: true, editable: true });
grid.setActiveCell(0, 1);
grid.editActiveCell(InputEditor as any, true);
const currentEditor = grid.getCellEditor() as Editor;
currentEditor.save!();

expect(navigateDownSpy).not.toHaveBeenCalled();
});

it('should NOT call navigateDown() when calling save() on an AutoCompleterEditor that disabled navigate down', () => {
const columns = [{ id: 'name', field: 'name', name: 'Name' }, { id: 'age', field: 'age', name: 'Age', editorClass: AutocompleterEditor }] as Column[];
const items = [{ id: 0, name: 'Avery', age: 44 }, { id: 1, name: 'Bob', age: 20 }, { id: 2, name: 'Rachel', age: 46 },];

const navigateDownSpy = vi.spyOn(grid, 'navigateDown');
grid = new SlickGrid<any, Column>(container, items, columns, { ...defaultOptions, enableCellNavigation: true, editable: true });
grid.setActiveCell(0, 1);
grid.editActiveCell(AutocompleterEditor as any, true);
const currentEditor = grid.getCellEditor() as Editor;
currentEditor.save!();

expect(navigateDownSpy).not.toHaveBeenCalled();
});
});

describe('updateRow() method', () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/core/slickGrid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5523,12 +5523,12 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
}
}

protected commitEditAndSetFocus(): void {
protected commitEditAndSetFocus(navigateCellDown = true): void {
// if the commit fails, it would do so due to a validation error
// if so, do not steal the focus from the editor
if (this.getEditorLock()?.commitCurrentEdit()) {
this.setFocus();
if (this._options.autoEdit && !this._options.autoCommitEdit) {
if (this._options.autoEdit && !this._options.autoCommitEdit && navigateCellDown) {
this.navigateDown();
}
}
Expand Down
25 changes: 23 additions & 2 deletions packages/common/src/editors/__tests__/autocompleterEditor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -455,15 +455,36 @@ describe('AutocompleterEditor', () => {
expect(spy).toHaveBeenCalled();
});

it('should call "commitChanges" when "hasAutoCommitEdit" is disabled after calling "save()" method', () => {
it('should call "commitChanges" with false when calling "save()" method and the last event is the ENTER key', () => {
gridOptionMock.autoCommitEdit = false;
const spy = vi.spyOn(editorArguments, 'commitChanges');

editor = new AutocompleterEditor(editorArguments);
editor.setValue('a');
const event = new (window.window as any).KeyboardEvent('keydown', { key: 'Enter', bubbles: true, cancelable: true });
const editorElm = divContainer.querySelector('input.editor-gender') as HTMLInputElement;
editorElm.focus();
editorElm.dispatchEvent(event);

editor.save();

expect(spy).toHaveBeenCalled();
expect(spy).toHaveBeenCalledWith(false);
});

it('should call "commitChanges" with true when calling "save()" method and the last event is NOT the ENTER key', () => {
gridOptionMock.autoCommitEdit = false;
const spy = vi.spyOn(editorArguments, 'commitChanges');

editor = new AutocompleterEditor(editorArguments);
editor.setValue('a');
const event = new (window.window as any).KeyboardEvent('keydown', { key: 'a', bubbles: true, cancelable: true });
const editorElm = divContainer.querySelector('input.editor-gender') as HTMLInputElement;
editorElm.focus();
editorElm.dispatchEvent(event);

editor.save();

expect(spy).toHaveBeenCalledWith(true);
});
});

Expand Down
6 changes: 3 additions & 3 deletions packages/common/src/editors/autocompleterEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,8 @@ export class AutocompleterEditor<T extends AutocompleteItem = any> implements Ed
// also the select list will stay shown when clicking off the grid
this.grid.getEditorLock().commitCurrentEdit();
} else {
this.args.commitChanges();
const navigateDown = this._lastInputKeyEvent?.key !== 'Enter';
this.args.commitChanges(navigateDown);
}
}

Expand Down Expand Up @@ -461,7 +462,6 @@ export class AutocompleterEditor<T extends AutocompleteItem = any> implements Ed
// a better solution would be to get the autocomplete DOM element to work with selection but I couldn't find how to do that in Vitest
handleSelect(item: AutocompleteSearchItem): boolean {
if (item !== undefined) {
const event = null; // TODO do we need the event?
const selectedItem = item;
this._currentValue = selectedItem;
this._isValueTouched = true;
Expand All @@ -475,7 +475,7 @@ export class AutocompleterEditor<T extends AutocompleteItem = any> implements Ed
this.setValue(itemLabel);

if (compositeEditorOptions) {
this.handleChangeOnCompositeEditor(event, compositeEditorOptions);
this.handleChangeOnCompositeEditor(null, compositeEditorOptions);
} else {
this.save();
}
Expand Down
7 changes: 5 additions & 2 deletions packages/common/src/interfaces/editorArguments.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ export interface EditorArguments {
/** Cancel changes callback method that will execute after user cancels an edit */
cancelChanges: () => void;

/** Commit changes callback method that will execute after user commits the changes */
commitChanges: () => void;
/**
* Commit changes callback method that will execute after user commits the changes
* @param {Boolean} [navigateCellDown] - by default the `autoCommit` will navigate to next cell down (unless `autoCommitEdit` is enabled if so do nothing)
*/
commitChanges: (navigateCellDown?: boolean) => void;
}

0 comments on commit 4f9eb36

Please sign in to comment.