Skip to content

Commit

Permalink
feat(filters): add grid option `skipCompoundOperatorFilterWithNullInp…
Browse files Browse the repository at this point in the history 
…ut` (#794)

- 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).
  • Loading branch information
@ghiscoding
ghiscoding committed Nov 5, 2022
1 parent b895864 commit 617c88d
Show file tree
Hide file tree
Showing 9 changed files with 165 additions and 33 deletions.
34 changes: 33 additions & 1 deletion packages/common/src/filters/__tests__/compoundDateFilter.spec.ts
View file
@@ -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';
Expand Down Expand Up @@ -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 = '<=';
Expand Down Expand Up @@ -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' },
Expand Down
32 changes: 30 additions & 2 deletions packages/common/src/filters/__tests__/compoundInputFilter.spec.ts
View file
Expand Up @@ -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('>=');
Expand All @@ -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;
Expand Down
38 changes: 32 additions & 6 deletions packages/common/src/filters/__tests__/compoundSliderFilter.spec.ts
View file
Expand Up @@ -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', () => {
Expand All @@ -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

Expand All @@ -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);
Expand All @@ -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 });
});
Expand Down Expand Up @@ -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%)');
});
Expand Down
14 changes: 7 additions & 7 deletions packages/common/src/filters/__tests__/singleSliderFilter.spec.ts
View file
Expand Up @@ -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', () => {
Expand All @@ -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());
});

Expand All @@ -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', () => {
Expand Down Expand Up @@ -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%)');
});
Expand Down
7 changes: 6 additions & 1 deletion packages/common/src/filters/compoundDateFilter.ts
View file
Expand Up @@ -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
Expand Down
34 changes: 25 additions & 9 deletions packages/common/src/filters/compoundInputFilter.ts
View file
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -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');
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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);
Expand All @@ -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');
Expand All @@ -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);
}
}
}

Expand Down

0 comments on commit 617c88d

Please sign in to comment.