feat(filters): add grid option skipCompoundOperatorFilterWithNullInput

- Should we skip filtering when the Operator is changed before the Compound Filter input. - For example, with a CompoundDate Filter it's probably better to wait until we have a date filled before filtering even if we start with the operator. - Defaults to True only for the Compound Date Filter (all other compound filters will still filter even when operator is first changed).
ghiscoding · Nov 5, 2022 · d906365 · d906365
1 parent b895864
commit d906365
Show file tree

Hide file tree

Showing 9 changed files with 165 additions and 33 deletions.
diff --git a/packages/common/src/filters/__tests__/compoundDateFilter.spec.ts b/packages/common/src/filters/__tests__/compoundDateFilter.spec.ts
@@ -1,3 +1,4 @@
+import 'jest-extended';
 import { Filters } from '../filters.index';
 import { FieldType, OperatorType } from '../../enums/index';
 import { Column, FilterArguments, GridOption, SlickGrid } from '../../interfaces/index';
@@ -173,6 +174,37 @@ describe('CompoundDateFilter', () => {
     expect(spyCallback).toHaveBeenCalledWith(undefined, { columnDef: mockColumn, operator: '>', searchTerms: ['2001-01-02'], shouldTriggerQuery: true });
   });
 
+  it('should change operator dropdown without a date entered and not expect the callback to be called', () => {
+    mockColumn.filter!.filterOptions = { allowInput: true }; // change to allow input value only for testing purposes
+    mockColumn.filter!.operator = '>';
+    const spyCallback = jest.spyOn(filterArguments, 'callback');
+
+    filter.init(filterArguments);
+    const filterInputElm = divContainer.querySelector('.search-filter.filter-finish .flatpickr input.input') as HTMLInputElement;
+    const filterSelectElm = divContainer.querySelector('.search-filter.filter-finish select') as HTMLInputElement;
+    filterInputElm.value = undefined as any;
+    filterSelectElm.value = '<=';
+    filterSelectElm.dispatchEvent(new Event('change'));
+
+    expect(spyCallback).not.toHaveBeenCalled();
+  });
+
+  it('should change operator dropdown without a date entered and expect the callback to be called when "skipCompoundOperatorFilterWithNullInput" is defined as False', () => {
+    mockColumn.filter!.filterOptions = { allowInput: true }; // change to allow input value only for testing purposes
+    mockColumn.filter!.operator = '>';
+    mockColumn.filter!.skipCompoundOperatorFilterWithNullInput = false;
+    const spyCallback = jest.spyOn(filterArguments, 'callback');
+
+    filter.init(filterArguments);
+    const filterInputElm = divContainer.querySelector('.search-filter.filter-finish .flatpickr input.input') as HTMLInputElement;
+    const filterSelectElm = divContainer.querySelector('.search-filter.filter-finish select') as HTMLInputElement;
+    filterInputElm.value = undefined as any;
+    filterSelectElm.value = '<=';
+    filterSelectElm.dispatchEvent(new Event('change'));
+
+    expect(spyCallback).toHaveBeenCalled();
+  });
+
   it('should create the input filter with a default search term when passed as a filter argument', () => {
     filterArguments.searchTerms = ['2000-01-01T05:00:00.000Z'];
     mockColumn.filter!.operator = '<=';
@@ -360,7 +392,7 @@ describe('CompoundDateFilter', () => {
   it('should have custom compound operator list showing up in the operator select dropdown options list', () => {
     mockColumn.outputType = null as any;
     filterArguments.searchTerms = ['2000-01-01T05:00:00.000Z'];
-    mockColumn.filter.compoundOperatorList = [
+    mockColumn.filter!.compoundOperatorList = [
       { operator: '', description: '' },
       { operator: '=', description: 'Equal to' },
       { operator: '<', description: 'Less than' },

diff --git a/packages/common/src/filters/__tests__/compoundInputFilter.spec.ts b/packages/common/src/filters/__tests__/compoundInputFilter.spec.ts
@@ -141,7 +141,7 @@ describe('CompoundInputFilter', () => {
     filter.setValues(['9'], OperatorType.greaterThanOrEqual);
 
     const filterSelectElm = divContainer.querySelector('.search-filter.filter-duration select') as HTMLInputElement;
-    filterSelectElm.dispatchEvent(new CustomEvent('change'));
+    filterSelectElm.dispatchEvent(new Event('change'));
 
     expect(spyCallback).toHaveBeenCalledWith(expect.anything(), { columnDef: mockColumn, operator: '>=', searchTerms: ['9'], shouldTriggerQuery: true });
     expect(filterSelectElm.value).toBe('>=');
@@ -168,11 +168,39 @@ describe('CompoundInputFilter', () => {
     const filterSelectElm = divContainer.querySelector('.search-filter.filter-duration select') as HTMLInputElement;
 
     filterSelectElm.value = '<=';
-    filterSelectElm.dispatchEvent(new CustomEvent('change'));
+    filterSelectElm.dispatchEvent(new Event('change'));
 
     expect(spyCallback).toHaveBeenCalledWith(expect.anything(), { columnDef: mockColumn, operator: '<=', searchTerms: ['9'], shouldTriggerQuery: true });
   });
 
+  it('should change operator dropdown without a value entered and not expect the callback to be called when "skipCompoundOperatorFilterWithNullInput" is defined as True', () => {
+    mockColumn.filter!.skipCompoundOperatorFilterWithNullInput = true;
+    mockColumn.type = FieldType.number;
+    const callbackSpy = jest.spyOn(filterArguments, 'callback');
+
+    filter.init(filterArguments);
+    const filterSelectElm = divContainer.querySelector('.search-filter.filter-duration select') as HTMLInputElement;
+
+    filterSelectElm.value = '<=';
+    filterSelectElm.dispatchEvent(new Event('change'));
+
+    expect(callbackSpy).not.toHaveBeenCalled();
+  });
+
+  it('should change operator dropdown without a value entered and not expect the callback to be called when "skipCompoundOperatorFilterWithNullInput" is defined as False', () => {
+    mockColumn.filter!.skipCompoundOperatorFilterWithNullInput = false;
+    mockColumn.type = FieldType.number;
+    const callbackSpy = jest.spyOn(filterArguments, 'callback');
+
+    filter.init(filterArguments);
+    const filterSelectElm = divContainer.querySelector('.search-filter.filter-duration select') as HTMLInputElement;
+
+    filterSelectElm.value = '<=';
+    filterSelectElm.dispatchEvent(new Event('change'));
+
+    expect(callbackSpy).toHaveBeenCalled();
+  });
+
   it('should call "setValues" with extra spaces at the beginning of the searchTerms and trim value when "enableFilterTrimWhiteSpace" is enabled in grid options', () => {
     gridOptionMock.enableFilterTrimWhiteSpace = true;
     mockColumn.type = FieldType.number;

diff --git a/packages/common/src/filters/__tests__/compoundSliderFilter.spec.ts b/packages/common/src/filters/__tests__/compoundSliderFilter.spec.ts
@@ -69,7 +69,7 @@ describe('CompoundSliderFilter', () => {
 
     expect(spyGetHeaderRow).toHaveBeenCalled();
     expect(filterCount).toBe(1);
-    expect(filter.currentValue).toBe(0);
+    expect(filter.currentValue).toBeUndefined();
   });
 
   it('should have an aria-label when creating the filter', () => {
@@ -87,7 +87,7 @@ describe('CompoundSliderFilter', () => {
     filter.init(filterArgs);
     filter.setValues(['2']);
     const filterElm = divContainer.querySelector('.input-group.search-filter.filter-duration input') as HTMLInputElement;
-    filterElm.dispatchEvent(new CustomEvent('change'));
+    filterElm.dispatchEvent(new Event('change'));
 
     jest.runAllTimers(); // fast-forward timer
 
@@ -102,7 +102,7 @@ describe('CompoundSliderFilter', () => {
     filter.init(filterArgs);
     filter.setValues(3);
     const filterElm = divContainer.querySelector('.input-group.search-filter.filter-duration input') as HTMLInputElement;
-    filterElm.dispatchEvent(new CustomEvent('change'));
+    filterElm.dispatchEvent(new Event('change'));
     const filterFilledElms = divContainer.querySelectorAll('.slider-container.search-filter.filter-duration.filled');
 
     expect(filterFilledElms.length).toBe(1);
@@ -117,19 +117,45 @@ describe('CompoundSliderFilter', () => {
     const filterSelectElm = divContainer.querySelector('.search-filter.filter-duration select') as HTMLInputElement;
 
     filterSelectElm.value = '<=';
-    filterSelectElm.dispatchEvent(new CustomEvent('change'));
+    filterSelectElm.dispatchEvent(new Event('change'));
 
     expect(callbackSpy).toHaveBeenCalledWith(expect.anything(), { columnDef: mockColumn, operator: '<=', searchTerms: [9], shouldTriggerQuery: true });
   });
 
+  it('should change operator dropdown without a value entered and not expect the callback to be called when "skipCompoundOperatorFilterWithNullInput" is defined as True', () => {
+    mockColumn.filter!.skipCompoundOperatorFilterWithNullInput = true;
+    const callbackSpy = jest.spyOn(filterArguments, 'callback');
+
+    filter.init(filterArguments);
+    const filterSelectElm = divContainer.querySelector('.search-filter.filter-duration select') as HTMLInputElement;
+
+    filterSelectElm.value = '<=';
+    filterSelectElm.dispatchEvent(new Event('change'));
+
+    expect(callbackSpy).not.toHaveBeenCalled();
+  });
+
+  it('should change operator dropdown without a value entered and expect the callback to be called when "skipCompoundOperatorFilterWithNullInput" is defined as False', () => {
+    mockColumn.filter!.skipCompoundOperatorFilterWithNullInput = false;
+    const callbackSpy = jest.spyOn(filterArguments, 'callback');
+
+    filter.init(filterArguments);
+    const filterSelectElm = divContainer.querySelector('.search-filter.filter-duration select') as HTMLInputElement;
+
+    filterSelectElm.value = '<=';
+    filterSelectElm.dispatchEvent(new Event('change'));
+
+    expect(callbackSpy).toHaveBeenCalled();
+  });
+
   it('should be able to call "setValues" with a value, converted as a number, and an extra operator and expect it to be set as new operator', () => {
     const callbackSpy = jest.spyOn(filterArguments, 'callback');
 
     filter.init(filterArguments);
     filter.setValues(['9'], OperatorType.greaterThanOrEqual);
 
     const filterSelectElm = divContainer.querySelector('.search-filter.filter-duration select') as HTMLInputElement;
-    filterSelectElm.dispatchEvent(new CustomEvent('change'));
+    filterSelectElm.dispatchEvent(new Event('change'));
 
     expect(callbackSpy).toHaveBeenCalledWith(expect.anything(), { columnDef: mockColumn, operator: '>=', searchTerms: [9], shouldTriggerQuery: true });
   });
@@ -257,7 +283,7 @@ describe('CompoundSliderFilter', () => {
     filter.init(filterArguments);
     filter.setValues(['80']);
     const filterElms = divContainer.querySelectorAll<HTMLInputElement>('.search-filter.slider-container.filter-duration input');
-    filterElms[0].dispatchEvent(new CustomEvent('change'));
+    filterElms[0].dispatchEvent(new Event('change'));
 
     expect(filter.sliderOptions?.sliderTrackBackground).toBe('linear-gradient(to right, #eee 0%, var(--slick-slider-filter-thumb-color, #86bff8) 0%, var(--slick-slider-filter-thumb-color, #86bff8) 80%, #eee 80%)');
   });

diff --git a/packages/common/src/filters/__tests__/singleSliderFilter.spec.ts b/packages/common/src/filters/__tests__/singleSliderFilter.spec.ts
@@ -64,7 +64,7 @@ describe('SingleSliderFilter', () => {
 
     expect(spyGetHeaderRow).toHaveBeenCalled();
     expect(filterCount).toBe(1);
-    expect(filter.currentValue).toBe(0);
+    expect(filter.currentValue).toBeUndefined();
   });
 
   it('should have an aria-label when creating the filter', () => {
@@ -81,11 +81,11 @@ describe('SingleSliderFilter', () => {
     filter.init(filterArgs);
     filter.setValues(['2']);
     const filterElm = divContainer.querySelector('.search-filter.slider-container.filter-duration input') as HTMLInputElement;
-    filterElm.dispatchEvent(new CustomEvent('change'));
+    filterElm.dispatchEvent(new Event('change'));
 
     jest.runAllTimers(); // fast-forward timer
 
-    expect(callbackSpy).toHaveBeenLastCalledWith(new CustomEvent('change'), { columnDef: mockColumn, operator: 'GE', searchTerms: [2], shouldTriggerQuery: true });
+    expect(callbackSpy).toHaveBeenLastCalledWith(new Event('change'), { columnDef: mockColumn, operator: 'GE', searchTerms: [2], shouldTriggerQuery: true });
     expect(rowMouseEnterSpy).toHaveBeenCalledWith({ column: mockColumn, grid: gridStub }, expect.anything());
   });
 
@@ -95,14 +95,14 @@ describe('SingleSliderFilter', () => {
     filter.init(filterArgs);
     filter.setValues(3);
     const filterElm = divContainer.querySelector('.search-filter.slider-container.filter-duration input') as HTMLInputElement;
-    filterElm.dispatchEvent(new CustomEvent('change'));
-    const mockEvent = new CustomEvent(`change`);
+    filterElm.dispatchEvent(new Event('change'));
+    const mockEvent = new Event('change');
     Object.defineProperty(mockEvent, 'target', { writable: true, configurable: true, value: { value: '13' } });
     filterElm.dispatchEvent(mockEvent);
     const filterFilledElms = divContainer.querySelectorAll('.search-filter.slider-container.filter-duration.filled');
 
     expect(filterFilledElms.length).toBe(1);
-    expect(callbackSpy).toHaveBeenLastCalledWith(new CustomEvent('change'), { columnDef: mockColumn, operator: 'GE', searchTerms: [3], shouldTriggerQuery: true });
+    expect(callbackSpy).toHaveBeenLastCalledWith(new Event('change'), { columnDef: mockColumn, operator: 'GE', searchTerms: [3], shouldTriggerQuery: true });
   });
 
   it('should be able to call "setValues" and set empty values and the input to not have the "filled" css class', () => {
@@ -227,7 +227,7 @@ describe('SingleSliderFilter', () => {
     filter.init(filterArgs);
     filter.setValues(['80']);
     const filterElms = divContainer.querySelectorAll<HTMLInputElement>('.search-filter.slider-container.filter-duration input');
-    filterElms[0].dispatchEvent(new CustomEvent('change'));
+    filterElms[0].dispatchEvent(new Event('change'));
 
     expect(filter.sliderOptions?.sliderTrackBackground).toBe('linear-gradient(to right, #eee 0%, var(--slick-slider-filter-thumb-color, #86bff8) 0%, var(--slick-slider-filter-thumb-color, #86bff8) 80%, #eee 80%)');
   });

diff --git a/packages/common/src/filters/compoundDateFilter.ts b/packages/common/src/filters/compoundDateFilter.ts
@@ -326,7 +326,12 @@ export class CompoundDateFilter implements Filter {
     } else {
       const selectedOperator = this._selectOperatorElm.value as OperatorString;
       (this._currentValue) ? this._filterElm.classList.add('filled') : this._filterElm.classList.remove('filled');
-      this.callback(e, { columnDef: this.columnDef, searchTerms: (this._currentValue ? [this._currentValue] : null), operator: selectedOperator || '', shouldTriggerQuery: this._shouldTriggerQuery });
+
+      // when changing compound operator, we don't want to trigger the filter callback unless the date input is also provided
+      const skipCompoundOperatorFilterWithNullInput = this.columnFilter.skipCompoundOperatorFilterWithNullInput ?? this.gridOptions.skipCompoundOperatorFilterWithNullInput ?? this.gridOptions.skipCompoundOperatorFilterWithNullInput === undefined;
+      if (!skipCompoundOperatorFilterWithNullInput || this._currentDate !== undefined) {
+        this.callback(e, { columnDef: this.columnDef, searchTerms: (this._currentValue ? [this._currentValue] : null), operator: selectedOperator || '', shouldTriggerQuery: this._shouldTriggerQuery });
+      }
     }
 
     // reset both flags for next use

diff --git a/packages/common/src/filters/compoundInputFilter.ts b/packages/common/src/filters/compoundInputFilter.ts
@@ -20,6 +20,7 @@ import { TranslaterService } from '../services/translater.service';
 export class CompoundInputFilter implements Filter {
   protected _bindEventService: BindingEventService;
   protected _clearFilterTriggered = false;
+  protected _currentValue?: number | string;
   protected _debounceTypingDelay = 0;
   protected _shouldTriggerQuery = true;
   protected _inputType = 'text';
@@ -103,8 +104,8 @@ export class CompoundInputFilter implements Filter {
     // step 3, subscribe to the keyup event and run the callback when that happens
     // also add/remove "filled" class for styling purposes
     // we'll use all necessary events to cover the following (keyup, change, mousewheel & spinner)
-    this._bindEventService.bind(this._filterInputElm, ['keyup', 'blur', 'change', 'wheel'], this.onTriggerEvent.bind(this));
-    this._bindEventService.bind(this._selectOperatorElm, 'change', this.onTriggerEvent.bind(this));
+    this._bindEventService.bind(this._filterInputElm, ['keyup', 'blur', 'change', 'wheel'], this.onTriggerEvent.bind(this) as EventListener);
+    this._bindEventService.bind(this._selectOperatorElm, 'change', this.onTriggerEvent.bind(this) as EventListener);
   }
 
   /**
@@ -117,6 +118,7 @@ export class CompoundInputFilter implements Filter {
       this.searchTerms = [];
       this._selectOperatorElm.selectedIndex = 0;
       this._filterInputElm.value = '';
+      this._currentValue = undefined;
       this.onTriggerEvent(undefined);
       this._filterElm.classList.remove('filled');
       this._filterInputElm.classList.remove('filled');
@@ -144,6 +146,7 @@ export class CompoundInputFilter implements Filter {
       newInputValue = `${newValue ?? ''}`;
     }
     this._filterInputElm.value = newInputValue;
+    this._currentValue = newInputValue;
 
     if (this.getValues() !== '') {
       this._filterElm.classList.add('filled');
@@ -240,8 +243,12 @@ export class CompoundInputFilter implements Filter {
     // create the DOM element & add an ID and filter class
     filterContainerElm.appendChild(containerInputGroupElm);
 
-    this._filterInputElm.value = `${searchTerm ?? ''}`;
     this._filterInputElm.dataset.columnid = `${columnId}`;
+    const searchVal = `${searchTerm ?? ''}`;
+    this._filterInputElm.value = searchVal;
+    if (searchTerm !== undefined) {
+      this._currentValue = searchVal;
+    }
 
     if (this.operator) {
       const operatorShorthand = mapOperatorToShorthandDesignation(this.operator);
@@ -265,7 +272,7 @@ export class CompoundInputFilter implements Filter {
    * Event trigger, could be called by the Operator dropdown or the input itself and we will cover the following (keyup, change, mousewheel & spinner)
    * We will trigger the Filter Service callback from this handler
    */
-  protected onTriggerEvent(event: Event | undefined) {
+  protected onTriggerEvent(event: MouseEvent | KeyboardEvent | undefined) {
     if (this._clearFilterTriggered) {
       this.callback(event, { columnDef: this.columnDef, clearFilterTriggered: this._clearFilterTriggered, shouldTriggerQuery: this._shouldTriggerQuery });
       this._filterElm.classList.remove('filled');
@@ -278,15 +285,24 @@ export class CompoundInputFilter implements Filter {
         value = value.trim();
       }
 
+      // only update ref when the value from the input
+      if ((event?.target as HTMLElement)?.tagName.toLowerCase() !== 'select') {
+        this._currentValue = value;
+      }
+
       (value !== null && value !== undefined && value !== '') ? this._filterElm.classList.add('filled') : this._filterElm.classList.remove('filled');
       const callbackArgs = { columnDef: this.columnDef, searchTerms: (value ? [value] : null), operator: selectedOperator, shouldTriggerQuery: this._shouldTriggerQuery };
       const typingDelay = (eventType === 'keyup' && (event as KeyboardEvent)?.key !== 'Enter') ? this._debounceTypingDelay : 0;
 
-      if (typingDelay > 0) {
-        clearTimeout(this.timer as NodeJS.Timeout);
-        this.timer = setTimeout(() => this.callback(event, callbackArgs), typingDelay);
-      } else {
-        this.callback(event, callbackArgs);
+      // when changing compound operator, we don't want to trigger the filter callback unless the filter input is also provided
+      const skipCompoundOperatorFilterWithNullInput = this.columnFilter.skipCompoundOperatorFilterWithNullInput ?? this.gridOptions.skipCompoundOperatorFilterWithNullInput;
+      if (!skipCompoundOperatorFilterWithNullInput || this._currentValue !== undefined) {
+        if (typingDelay > 0) {
+          clearTimeout(this.timer as NodeJS.Timeout);
+          this.timer = setTimeout(() => this.callback(event, callbackArgs), typingDelay);
+        } else {
+          this.callback(event, callbackArgs);
+        }
       }
     }