fix(editors): all Editors should call commitChanges even when invalid, fixes #571

src/app/examples/grid-editor.component.ts

@@ -244,7 +244,13 @@ export class GridEditorComponent implements OnInit {
         },
         params: {
           formatters: [Formatters.collectionEditor, Formatters.percentCompleteBar],
-        }
+        },
+        // validator: (value, args) => {
+        //   if (value < 50) {
+        //     return { valid: false, msg: 'Please use at least 50%' };
+        //   }
+        //   return { valid: true, msg: '' };
+        // }
       }, {
         id: 'start',
         name: 'Start',
@@ -260,7 +266,7 @@ export class GridEditorComponent implements OnInit {
         sortable: true,
         type: FieldType.date,
         editor: {
-          model: Editors.date
+          model: Editors.date,
         },
       }, {
         id: 'finish',
@@ -582,7 +588,9 @@ export class GridEditorComponent implements OnInit {
   }
 
   onValidationError(e, args) {
-    alert(args.validationResults.msg);
+    if (args.validationResults) {
+      alert(args.validationResults.msg);
+    }
   }
 
   changeAutoCommit() {

src/app/modules/angular-slickgrid/editors/__tests__/floatEditor.spec.ts

@@ -405,7 +405,7 @@ describe('FloatEditor', () => {
         expect(spy).toHaveBeenCalled();
       });
 
-      it('should not call anything when the input value is not a valid float number', () => {
+      it('should call "commitCurrentEdit" even when the input value is not a valid float number', () => {
         mockItemData = { id: 1, price: null, isActive: true };
         gridOptionMock.autoCommitEdit = true;
         const spy = jest.spyOn(gridStub.getEditorLock(), 'commitCurrentEdit');
@@ -415,7 +415,7 @@ describe('FloatEditor', () => {
         editor.setValue('-.');
         editor.save();
 
-        expect(spy).not.toHaveBeenCalled();
+        expect(spy).toHaveBeenCalled();
       });
 
       it('should call "getEditorLock" and "save" methods when "hasAutoCommitEdit" is enabled and the event "focusout" is triggered', (done) => {

src/app/modules/angular-slickgrid/editors/__tests__/selectEditor.spec.ts

@@ -428,15 +428,13 @@ describe('SelectEditor', () => {
         mockItemData = { id: 1, gender: '', isActive: true };
         mockColumn.internalColumnEditor.required = true;
         gridOptionMock.autoCommitEdit = true;
-        const commitEditSpy = jest.spyOn(gridStub.getEditorLock(), 'commitCurrentEdit');
-        const commitChangeSpy = jest.spyOn(editorArguments, 'commitChanges');
+        const spy = jest.spyOn(gridStub.getEditorLock(), 'commitCurrentEdit');
 
         editor = new SelectEditor(editorArguments, true);
         editor.loadValue(mockItemData);
         editor.save();
 
-        expect(commitEditSpy).not.toHaveBeenCalled();
-        expect(commitChangeSpy).not.toHaveBeenCalled();
+        expect(spy).not.toHaveBeenCalled();
       });
     });
 

src/app/modules/angular-slickgrid/editors/__tests__/sliderEditor.spec.ts

@@ -389,7 +389,7 @@ describe('SliderEditor', () => {
         expect(spy).toHaveBeenCalled();
       });
 
-      it('should not call anything when the input value is the same as the default value', () => {
+      it('should call "commitCurrentEdit" even when the input value is the same as the default value', () => {
         mockItemData = { id: 1, price: 0, isActive: true };
         gridOptionMock.autoCommitEdit = true;
         const spy = jest.spyOn(gridStub.getEditorLock(), 'commitCurrentEdit');
@@ -398,7 +398,7 @@ describe('SliderEditor', () => {
         editor.loadValue(mockItemData);
         editor.save();
 
-        expect(spy).not.toHaveBeenCalled();
+        expect(spy).toHaveBeenCalled();
       });
 
       it('should call "getEditorLock" and "save" methods when "hasAutoCommitEdit" is enabled and the event "focusout" is triggered', (done) => {

src/app/modules/angular-slickgrid/editors/autoCompleteEditor.ts

@@ -212,12 +212,14 @@ export class AutoCompleteEditor implements Editor {
 
   save() {
     const validation = this.validate();
-    if (validation && validation.valid && this.isValueChanged()) {
-      if (this.hasAutoCommitEdit) {
-        this.grid.getEditorLock().commitCurrentEdit();
-      } else {
-        this.args.commitChanges();
-      }
+    const isValid = (validation && validation.valid) || false;
+
+    if (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();
+    } else {
+      this.args.commitChanges();
     }
   }
 

src/app/modules/angular-slickgrid/editors/checkboxEditor.ts

@@ -118,8 +118,14 @@ export class CheckboxEditor implements Editor {
 
   save() {
     const validation = this.validate();
-    if (validation && validation.valid && this.isValueChanged() && this.hasAutoCommitEdit) {
+    const isValid = (validation && validation.valid) || false;
+
+    if (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();
+    } else {
+      this.args.commitChanges();
     }
   }
 

src/app/modules/angular-slickgrid/editors/dateEditor.ts

@@ -220,14 +220,15 @@ export class DateEditor implements Editor {
   }
 
   save() {
-    // autocommit will not focus the next editor
     const validation = this.validate();
-    if (validation && validation.valid && this.isValueChanged()) {
-      if (this.hasAutoCommitEdit) {
-        this.grid.getEditorLock().commitCurrentEdit();
-      } else {
-        this.args.commitChanges();
-      }
+    const isValid = (validation && validation.valid) || false;
+
+    if (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();
+    } else {
+      this.args.commitChanges();
     }
   }
 

src/app/modules/angular-slickgrid/editors/floatEditor.ts

@@ -160,12 +160,14 @@ export class FloatEditor implements Editor {
 
   save() {
     const validation = this.validate();
-    if (validation && validation.valid && this.isValueChanged()) {
-      if (this.hasAutoCommitEdit) {
-        this.grid.getEditorLock().commitCurrentEdit();
-      } else {
-        this.args.commitChanges();
-      }
+    const isValid = (validation && validation.valid) || false;
+
+    if (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();
+    } else {
+      this.args.commitChanges();
     }
   }
 

src/app/modules/angular-slickgrid/editors/integerEditor.ts

@@ -129,12 +129,14 @@ export class IntegerEditor implements Editor {
 
   save() {
     const validation = this.validate();
-    if (validation && validation.valid && this.isValueChanged()) {
-      if (this.hasAutoCommitEdit) {
-        this.grid.getEditorLock().commitCurrentEdit();
-      } else {
-        this.args.commitChanges();
-      }
+    const isValid = (validation && validation.valid) || false;
+
+    if (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();
+    } else {
+      this.args.commitChanges();
     }
   }
 

src/app/modules/angular-slickgrid/editors/longTextEditor.ts

@@ -210,6 +210,8 @@ export class LongTextEditor implements Editor {
     const isValid = (validation && validation.valid) || false;
 
     if (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();
     } else {
       this.args.commitChanges();

src/app/modules/angular-slickgrid/editors/selectEditor.ts

@@ -417,16 +417,15 @@ export class SelectEditor implements Editor {
   }
 
   save() {
-    // autocommit will not focus the next editor
     const validation = this.validate();
-    if (validation && validation.valid && this.isValueChanged()) {
-      if (!this._destroying && this.hasAutoCommitEdit) {
-        // 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();
-      } else {
-        this.args.commitChanges();
-      }
+    const isValid = (validation && validation.valid) || false;
+
+    if (!this._destroying && 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();
+    } else {
+      this.args.commitChanges();
     }
   }
 

src/app/modules/angular-slickgrid/editors/sliderEditor.ts

@@ -153,12 +153,14 @@ export class SliderEditor implements Editor {
 
   save() {
     const validation = this.validate();
-    if (validation && validation.valid && this.isValueChanged()) {
-      if (this.hasAutoCommitEdit) {
-        this.args.grid.getEditorLock().commitCurrentEdit();
-      } else {
-        this.args.commitChanges();
-      }
+    const isValid = (validation && validation.valid) || false;
+
+    if (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();
+    } else {
+      this.args.commitChanges();
     }
   }
 

src/app/modules/angular-slickgrid/editors/textEditor.ts

@@ -129,12 +129,14 @@ export class TextEditor implements Editor {
 
   save() {
     const validation = this.validate();
-    if (validation && validation.valid && this.isValueChanged()) {
-      if (this.hasAutoCommitEdit) {
-        this.grid.getEditorLock().commitCurrentEdit();
-      } else {
-        this.args.commitChanges();
-      }
+    const isValid = (validation && validation.valid) || false;
+
+    if (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();
+    } else {
+      this.args.commitChanges();
     }
   }
 

src/app/modules/angular-slickgrid/models/locale.interface.ts

@@ -92,7 +92,7 @@ export interface Locale {
   /** Text "Remove Sort" shown in Header Menu */
   TEXT_REMOVE_SORT: string;
 
-  /** Text "Cancel" shown in the Long Text Editor dialog */
+  /** Text "Save" shown in the Long Text Editor dialog */
   TEXT_SAVE: string;
 
   /** Text "Select All" displayed in the Multiple Select Editor/Filter */