fix(presets): Filter & Sorting presets & Footer metrics issues (#285)

- use `onRowCountChanged` instead of `onRowsOrCountChanged` to avoid full grid refresh/re-render - input filter should use keyup to clearly identify when to use the filter keyboard debounce (that is relevent to InputFilter & CompoundInputFilter) - grid presets of Filter & Sorting should be called even their input are an empty array, this fixes an issue identified when dynamically switching a Grid View (what is shown on Example 11) that includes Filters/Sorting/Columns
ghiscoding · Mar 16, 2021 · 3174c86 · 3174c86
1 parent 20bda50
commit 3174c86
Show file tree

Hide file tree

Showing 10 changed files with 51 additions and 85 deletions.
diff --git a/packages/common/src/filters/__tests__/compoundInputFilter.spec.ts b/packages/common/src/filters/__tests__/compoundInputFilter.spec.ts
@@ -85,7 +85,7 @@ describe('CompoundInputFilter', () => {
     const filterInputElm = divContainer.querySelector('.search-filter.filter-duration input') as HTMLInputElement;
 
     filterInputElm.focus();
-    filterInputElm.dispatchEvent(new (window.window as any).Event('input', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
+    filterInputElm.dispatchEvent(new (window.window as any).Event('keyup', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
     const filterFilledElms = divContainer.querySelectorAll<HTMLInputElement>('.search-filter.filter-duration.filled');
 
     expect(filterFilledElms.length).toBe(1);
@@ -109,23 +109,6 @@ describe('CompoundInputFilter', () => {
     expect(spyCallback).toHaveBeenCalledWith(expect.anything(), { columnDef: mockColumn, operator: '', searchTerms: ['abc'], shouldTriggerQuery: true });
   });
 
-  it('should call "setValues" and expect that value NOT to be in the callback when triggered by a keyup event that is NOT the ENTER key', () => {
-    const spyCallback = jest.spyOn(filterArguments, 'callback');
-
-    filter.init(filterArguments);
-    filter.setValues(['abc']);
-    const filterInputElm = divContainer.querySelector('.search-filter.filter-duration input') as HTMLInputElement;
-
-    filterInputElm.focus();
-    const event = new (window.window as any).Event('keyup', { bubbles: true, cancelable: true });
-    event.key = 'a';
-    filterInputElm.dispatchEvent(event);
-    const filterFilledElms = divContainer.querySelectorAll<HTMLInputElement>('.search-filter.filter-duration.filled');
-
-    expect(filterFilledElms.length).toBe(0);
-    expect(spyCallback).not.toHaveBeenCalled();
-  });
-
   it('should call "setValues" with "operator" set in the filter arguments and expect that value to be in the callback when triggered', () => {
     mockColumn.type = FieldType.number;
     const filterArgs = { ...filterArguments, operator: '>' } as FilterArguments;
@@ -136,7 +119,7 @@ describe('CompoundInputFilter', () => {
     const filterInputElm = divContainer.querySelector('.search-filter.filter-duration input') as HTMLInputElement;
 
     filterInputElm.focus();
-    filterInputElm.dispatchEvent(new (window.window as any).Event('input', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
+    filterInputElm.dispatchEvent(new (window.window as any).Event('keyup', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
 
     expect(spyCallback).toHaveBeenCalledWith(expect.anything(), { columnDef: mockColumn, operator: '>', searchTerms: ['9'], shouldTriggerQuery: true });
   });
@@ -180,7 +163,7 @@ describe('CompoundInputFilter', () => {
     const filterInputElm = divContainer.querySelector('.search-filter.filter-duration input') as HTMLInputElement;
 
     filterInputElm.focus();
-    filterInputElm.dispatchEvent(new (window.window as any).Event('input', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
+    filterInputElm.dispatchEvent(new (window.window as any).Event('keyup', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
 
     expect(spyCallback).toHaveBeenCalledWith(expect.anything(), { columnDef: mockColumn, operator: '>', searchTerms: ['987'], shouldTriggerQuery: true });
   });
@@ -197,7 +180,7 @@ describe('CompoundInputFilter', () => {
     const filterInputElm = divContainer.querySelector('.search-filter.filter-duration input') as HTMLInputElement;
 
     filterInputElm.focus();
-    filterInputElm.dispatchEvent(new (window.window as any).Event('input', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
+    filterInputElm.dispatchEvent(new (window.window as any).Event('keyup', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
 
     expect(spyCallback).toHaveBeenCalledWith(expect.anything(), { columnDef: mockColumn, operator: '>', searchTerms: ['987'], shouldTriggerQuery: true });
   });
@@ -210,7 +193,7 @@ describe('CompoundInputFilter', () => {
 
     filterInputElm.focus();
     filterInputElm.value = 'a';
-    filterInputElm.dispatchEvent(new (window.window as any).Event('input', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
+    filterInputElm.dispatchEvent(new (window.window as any).Event('keyup', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
 
     expect(spyCallback).toHaveBeenCalledWith(expect.anything(), { columnDef: mockColumn, operator: '', searchTerms: ['a'], shouldTriggerQuery: true });
   });
@@ -224,7 +207,7 @@ describe('CompoundInputFilter', () => {
 
     filterInputElm.focus();
     filterInputElm.value = 'a';
-    filterInputElm.dispatchEvent(new (window.window as any).Event('input', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
+    filterInputElm.dispatchEvent(new (window.window as any).Event('keyup', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
 
     setTimeout(() => {
       expect(spyCallback).toHaveBeenCalledWith(expect.anything(), { columnDef: mockColumn, operator: '', searchTerms: ['a'], shouldTriggerQuery: true });
@@ -245,7 +228,7 @@ describe('CompoundInputFilter', () => {
 
     filterInputElm.focus();
     filterInputElm.value = 'a';
-    filterInputElm.dispatchEvent(new (window.window as any).Event('input', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
+    filterInputElm.dispatchEvent(new (window.window as any).Event('keyup', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
 
     setTimeout(() => {
       expect(spyCallback).toHaveBeenCalledWith(expect.anything(), { columnDef: mockColumn, operator: '', searchTerms: ['a'], shouldTriggerQuery: true });

diff --git a/packages/common/src/filters/__tests__/inputFilter.spec.ts b/packages/common/src/filters/__tests__/inputFilter.spec.ts
@@ -77,7 +77,7 @@ describe('InputFilter', () => {
     const filterElm = divContainer.querySelector('input.filter-duration') as HTMLInputElement;
 
     filterElm.focus();
-    filterElm.dispatchEvent(new (window.window as any).Event('input', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
+    filterElm.dispatchEvent(new (window.window as any).Event('keyup', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
     const filterFilledElms = divContainer.querySelectorAll<HTMLInputElement>('input.filter-duration.filled');
 
     expect(filterFilledElms.length).toBe(1);
@@ -101,23 +101,6 @@ describe('InputFilter', () => {
     expect(spyCallback).toHaveBeenCalledWith(expect.anything(), { columnDef: mockColumn, operator: '', searchTerms: ['abc'], shouldTriggerQuery: true });
   });
 
-  it('should call "setValues" and expect that value NOT to be in the callback when triggered by a keyup event that is NOT the ENTER key', () => {
-    const spyCallback = jest.spyOn(filterArguments, 'callback');
-
-    filter.init(filterArguments);
-    filter.setValues('abc');
-    const filterElm = divContainer.querySelector('input.filter-duration') as HTMLInputElement;
-
-    filterElm.focus();
-    const event = new (window.window as any).Event('keyup', { bubbles: true, cancelable: true });
-    event.key = 'a';
-    filterElm.dispatchEvent(event);
-    const filterFilledElms = divContainer.querySelectorAll<HTMLInputElement>('input.filter-duration.filled');
-
-    expect(filterFilledElms.length).toBe(0);
-    expect(spyCallback).not.toHaveBeenCalled();
-  });
-
   it('should call "setValues" with an operator and with extra spaces at the beginning of the searchTerms and trim value when "enableFilterTrimWhiteSpace" is enabled in grid options', () => {
     gridOptionMock.enableFilterTrimWhiteSpace = true;
     const spyCallback = jest.spyOn(filterArguments, 'callback');
@@ -127,7 +110,7 @@ describe('InputFilter', () => {
     const filterElm = divContainer.querySelector('input.filter-duration') as HTMLInputElement;
 
     filterElm.focus();
-    filterElm.dispatchEvent(new (window.window as any).Event('input', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
+    filterElm.dispatchEvent(new (window.window as any).Event('keyup', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
     const filterFilledElms = divContainer.querySelectorAll<HTMLInputElement>('input.filter-duration.filled');
 
     expect(filterFilledElms.length).toBe(1);
@@ -144,7 +127,7 @@ describe('InputFilter', () => {
     const filterElm = divContainer.querySelector('input.filter-duration') as HTMLInputElement;
 
     filterElm.focus();
-    filterElm.dispatchEvent(new (window.window as any).Event('input', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
+    filterElm.dispatchEvent(new (window.window as any).Event('keyup', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
     const filterFilledElms = divContainer.querySelectorAll<HTMLInputElement>('input.filter-duration.filled');
 
     expect(filterFilledElms.length).toBe(1);
@@ -159,7 +142,7 @@ describe('InputFilter', () => {
 
     filterElm.focus();
     filterElm.value = 'a';
-    filterElm.dispatchEvent(new (window.window as any).Event('input', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
+    filterElm.dispatchEvent(new (window.window as any).Event('keyup', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
 
     expect(spyCallback).toHaveBeenCalledWith(expect.anything(), { columnDef: mockColumn, operator: 'EQ', searchTerms: ['a'], shouldTriggerQuery: true });
   });
@@ -173,7 +156,7 @@ describe('InputFilter', () => {
 
     filterElm.focus();
     filterElm.value = 'a';
-    filterElm.dispatchEvent(new (window.window as any).Event('input', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
+    filterElm.dispatchEvent(new (window.window as any).Event('keyup', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
 
     setTimeout(() => {
       expect(spyCallback).toHaveBeenCalledWith(expect.anything(), { columnDef: mockColumn, operator: 'EQ', searchTerms: ['a'], shouldTriggerQuery: true });
@@ -193,7 +176,7 @@ describe('InputFilter', () => {
 
     filterElm.focus();
     filterElm.value = 'a';
-    filterElm.dispatchEvent(new (window.window as any).Event('input', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
+    filterElm.dispatchEvent(new (window.window as any).Event('keyup', { key: 'a', keyCode: 97, bubbles: true, cancelable: true }));
 
     setTimeout(() => {
       expect(spyCallback).toHaveBeenCalledWith(expect.anything(), { columnDef: mockColumn, operator: 'EQ', searchTerms: ['a'], shouldTriggerQuery: true });

diff --git a/packages/common/src/filters/compoundInputFilter.ts b/packages/common/src/filters/compoundInputFilter.ts
@@ -99,7 +99,8 @@ export class CompoundInputFilter implements Filter {
 
     // step 3, subscribe to the input change event and run the callback when that happens
     // also add/remove "filled" class for styling purposes
-    this.$filterInputElm.on('keyup input blur', this.onTriggerEvent.bind(this));
+    // we'll use all necessary events to cover the following (keyup, change, mousewheel & spinner)
+    this.$filterInputElm.on('keyup blur change wheel', this.onTriggerEvent.bind(this));
     this.$selectOperatorElm.on('change', this.onTriggerEvent.bind(this));
   }
 
@@ -122,7 +123,7 @@ export class CompoundInputFilter implements Filter {
    */
   destroy() {
     if (this.$filterElm && this.$selectOperatorElm) {
-      this.$filterElm.off('keyup input').remove();
+      this.$filterElm.off('keyup blur change wheel').remove();
       this.$selectOperatorElm.off('change');
     }
     this.$filterElm = null;
@@ -252,17 +253,16 @@ export class CompoundInputFilter implements Filter {
     return $filterContainerElm;
   }
 
-  /** Event trigger, could be called by the Operator dropdown or the input itself */
+  /**
+   * 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: KeyboardEvent | undefined) {
-    // we'll use the "input" event for everything (keyup, change, mousewheel & spinner)
-    // with 1 small exception, we need to use the keyup event to handle ENTER key, everything will be processed by the "input" event
-    if (event && event.type === 'keyup' && event.key !== 'Enter') {
-      return;
-    }
     if (this._clearFilterTriggered) {
       this.callback(event, { columnDef: this.columnDef, clearFilterTriggered: this._clearFilterTriggered, shouldTriggerQuery: this._shouldTriggerQuery });
       this.$filterElm.removeClass('filled');
     } else {
+      const eventType = event?.type ?? '';
       const selectedOperator = this.$selectOperatorElm.find('option:selected').val();
       let value = this.$filterInputElm.val() as string;
       const enableWhiteSpaceTrim = this.gridOptions.enableFilterTrimWhiteSpace || this.columnFilter.enableTrimWhiteSpace;
@@ -272,7 +272,7 @@ export class CompoundInputFilter implements Filter {
 
       (value !== null && value !== undefined && value !== '') ? this.$filterElm.addClass('filled') : this.$filterElm.removeClass('filled');
       const callbackArgs = { columnDef: this.columnDef, searchTerms: (value ? [value] : null), operator: selectedOperator || '', shouldTriggerQuery: this._shouldTriggerQuery };
-      const typingDelay = (event?.key === 'Enter' || event?.type === 'blur') ? 0 : this._debounceTypingDelay;
+      const typingDelay = (eventType === 'keyup' && event?.key !== 'Enter') ? this._debounceTypingDelay : 0;
 
       if (typingDelay > 0) {
         clearTimeout(this.timer as NodeJS.Timeout);

diff --git a/packages/common/src/filters/inputFilter.ts b/packages/common/src/filters/inputFilter.ts
@@ -88,7 +88,8 @@ export class InputFilter implements Filter {
 
     // step 3, subscribe to the input event and run the callback when that happens
     // also add/remove "filled" class for styling purposes
-    this.$filterElm.on('keyup input blur', this.handleInputChange.bind(this));
+    // we'll use all necessary events to cover the following (keyup, change, mousewheel & spinner)
+    this.$filterElm.on('keyup blur change wheel', this.handleInputChange.bind(this));
   }
 
   /**
@@ -100,7 +101,7 @@ export class InputFilter implements Filter {
       this._shouldTriggerQuery = shouldTriggerQuery;
       this.searchTerms = [];
       this.$filterElm.val('');
-      this.$filterElm.trigger('input');
+      this.$filterElm.trigger('change');
     }
   }
 
@@ -109,7 +110,7 @@ export class InputFilter implements Filter {
    */
   destroy() {
     if (this.$filterElm) {
-      this.$filterElm.off('keyup input').remove();
+      this.$filterElm.off('keyup blur change wheel').remove();
     }
     this.$filterElm = null;
   }
@@ -168,25 +169,24 @@ export class InputFilter implements Filter {
     return $filterElm;
   }
 
+  /**
+   * Event handler to cover the following (keyup, change, mousewheel & spinner)
+   * We will trigger the Filter Service callback from this handler
+   */
   protected handleInputChange(event: KeyboardEvent & { target: any; }) {
-    // we'll use the "input" event for everything (keyup, change, mousewheel & spinner)
-    // with 1 small exception, we need to use the keyup event to handle ENTER key, everything will be processed by the "input" event
-    if (event && event.type === 'keyup' && event.key !== 'Enter') {
-      return;
-    }
-
     if (this._clearFilterTriggered) {
       this.callback(event, { columnDef: this.columnDef, clearFilterTriggered: this._clearFilterTriggered, shouldTriggerQuery: this._shouldTriggerQuery });
       this.$filterElm.removeClass('filled');
     } else {
+      const eventType = event?.type ?? '';
       let value = event?.target?.value ?? '';
       const enableWhiteSpaceTrim = this.gridOptions.enableFilterTrimWhiteSpace || this.columnFilter.enableTrimWhiteSpace;
       if (typeof value === 'string' && enableWhiteSpaceTrim) {
         value = value.trim();
       }
       value === '' ? this.$filterElm.removeClass('filled') : this.$filterElm.addClass('filled');
       const callbackArgs = { columnDef: this.columnDef, operator: this.operator, searchTerms: [value], shouldTriggerQuery: this._shouldTriggerQuery };
-      const typingDelay = (event?.key === 'Enter' || event?.type === 'blur') ? 0 : this._debounceTypingDelay;
+      const typingDelay = (eventType === 'keyup' && event?.key !== 'Enter') ? this._debounceTypingDelay : 0;
 
       if (typingDelay > 0) {
         clearTimeout(this._timer as NodeJS.Timeout);

diff --git a/packages/common/src/interfaces/gridOption.interface.ts b/packages/common/src/interfaces/gridOption.interface.ts
@@ -343,7 +343,9 @@ export interface GridOption {
   exportOptions?: TextExportOption;
 
   /**
-   * Default to 0ms, how long to wait between each characters that the user types before processing the filtering process (only applies for local/in-memory grid).
+   * Default to 0, how long to wait between each characters that the user types before processing the filtering process (only applies for local/in-memory grid).
+   * Especially useful when you have a big dataset and you want to limit the amount of search called (by default every keystroke will trigger a search on the dataset and that is sometime slow).
+   * This is only used by and relevant to 2 filters (InputFilter & CompoundInputFilter) which are the only ones triggering a search after each character typed.
    * NOTE: please note that the BackendServiceApi has its own `filterTypingDebounce` within the `BackendServiceApi` options which is set to 500ms.
    */
   filterTypingDebounce?: number;

diff --git a/packages/common/src/services/filter.service.ts b/packages/common/src/services/filter.service.ts
@@ -642,7 +642,7 @@ export class FilterService {
    * @param grid
    */
   populateColumnFilterSearchTermPresets(filters: CurrentFilter[]) {
-    if (Array.isArray(filters) && filters.length > 0) {
+    if (Array.isArray(filters)) {
       this._columnDefinitions.forEach((columnDef: Column) => {
         // clear any columnDef searchTerms before applying Presets
         if (columnDef.filter && columnDef.filter.searchTerms) {

diff --git a/packages/common/src/services/sort.service.ts b/packages/common/src/services/sort.service.ts
@@ -294,10 +294,8 @@ export class SortService {
         }
       });
 
-      if (sortCols.length > 0) {
-        this.onLocalSortChanged(this._grid, sortCols);
-        this._grid.setSortColumns(sortCols.map(s => ({ columnId: s.columnId, sortAsc: s.sortAsc }))); // use this to add sort icon(s) in UI
-      }
+      this.onLocalSortChanged(this._grid, sortCols);
+      this._grid.setSortColumns(sortCols.map(col => ({ columnId: col.columnId, sortAsc: col.sortAsc }))); // use this to add sort icon(s) in UI
 
     }
     return sortCols;

diff --git a/packages/vanilla-bundle/dist-grid-bundle-zip/slickgrid-vanilla-bundle.zip b/packages/vanilla-bundle/dist-grid-bundle-zip/slickgrid-vanilla-bundle.zip