fix(editors): all Editors should call commitChanges even when invalid, …

…fixes #571 (#574) * fix(editors): fix all Editors custom validators with invalid results * fix(editors): all Editors should call commitChanges even when invalid - for the Editor validators to work properly it needs to call the commitChanges() at all time even when potentially invalid or value is unchanged so that the validator can be executed at all time
ghiscoding · Sep 8, 2020 · fd052d1 · fd052d1
1 parent e2b138d
commit fd052d1
Show file tree

Hide file tree

Showing 14 changed files with 83 additions and 59 deletions.
diff --git a/src/app/examples/grid-editor.component.ts b/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() {

diff --git a/src/app/modules/angular-slickgrid/editors/__tests__/floatEditor.spec.ts b/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) => {

diff --git a/src/app/modules/angular-slickgrid/editors/__tests__/selectEditor.spec.ts b/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();
       });
     });
 

diff --git a/src/app/modules/angular-slickgrid/editors/__tests__/sliderEditor.spec.ts b/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) => {

diff --git a/src/app/modules/angular-slickgrid/editors/autoCompleteEditor.ts b/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();
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/editors/checkboxEditor.ts b/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();
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/editors/dateEditor.ts b/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();
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/editors/floatEditor.ts b/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();
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/editors/integerEditor.ts b/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();
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/editors/longTextEditor.ts b/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();

diff --git a/src/app/modules/angular-slickgrid/editors/selectEditor.ts b/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();
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/editors/sliderEditor.ts b/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();
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/editors/textEditor.ts b/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();
     }
   }
 

diff --git a/src/app/modules/angular-slickgrid/models/locale.interface.ts b/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 */