Skip to content

Commit

Permalink
feat(common): update title prop on change event for Slider Filter/Edi…
Browse files Browse the repository at this point in the history
…tor (#743)

- whenever the slider value changes, we should update its title property with the new value
  • Loading branch information
ghiscoding authored Aug 11, 2022
1 parent ed6b0cc commit 0ca6f3f
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 56 deletions.
12 changes: 7 additions & 5 deletions packages/common/src/editors/sliderEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,7 @@ export class SliderEditor implements Editor {

// if user chose to display the slider number on the right side, then update it every time it changes
// we need to use both "input" and "change" event to be all cross-browser
if (!this.editorParams.hideSliderNumber) {
this._bindEventService.bind(this._editorElm, ['input', 'change'], this.handleChangeSliderNumber.bind(this));
}
this._bindEventService.bind(this._editorElm, ['input', 'change'], this.handleChangeSliderNumber.bind(this));
}
}

Expand Down Expand Up @@ -229,6 +227,7 @@ export class SliderEditor implements Editor {
this.originalValue = +value;
if (this._inputElm) {
this._inputElm.value = `${value}`;
this._inputElm.title = `${value}`;
}
if (this.sliderNumberElm) {
this.sliderNumberElm.textContent = `${value}`;
Expand Down Expand Up @@ -357,8 +356,11 @@ export class SliderEditor implements Editor {

protected handleChangeSliderNumber(event: Event) {
const value = (<HTMLInputElement>event.target)?.value ?? '';
if (value !== '' && this.sliderNumberElm) {
this.sliderNumberElm.textContent = value;
if (value !== '') {
if (!this.editorParams.hideSliderNumber && this.sliderNumberElm) {
this.sliderNumberElm.textContent = value;
}
this._inputElm.title = value;
}
}

Expand Down
20 changes: 11 additions & 9 deletions packages/common/src/filters/__tests__/compoundSliderFilter.spec.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { FieldType, OperatorType } from '../../enums/index';
import { Column, FilterArguments, GridOption, SlickGrid } from '../../interfaces/index';
import { Column, FilterArguments, GridOption, SlickGrid, SlickNamespace } from '../../interfaces/index';
import { Filters } from '../index';
import { CompoundSliderFilter } from '../compoundSliderFilter';
import { TranslateServiceStub } from '../../../../../test/translateServiceStub';

const containerId = 'demo-container';
declare const Slick: SlickNamespace;

// define a <div> container to simulate the grid container
const template = `<div id="${containerId}"></div>`;
Expand All @@ -23,6 +24,7 @@ const gridStub = {
getColumns: jest.fn(),
getHeaderRowColumn: jest.fn(),
render: jest.fn(),
onHeaderMouseLeave: new Slick.Event(),
} as unknown as SlickGrid;

describe('CompoundSliderFilter', () => {
Expand Down Expand Up @@ -76,7 +78,7 @@ describe('CompoundSliderFilter', () => {

it('should call "setValues" with "operator" set in the filter arguments and expect that value to be in the callback when triggered', () => {
const spyCallback = jest.spyOn(filterArguments, 'callback');
const filterArgs = { ...filterArguments, operator: '>' } as FilterArguments;
const filterArgs = { ...filterArguments, operator: '>', grid: gridStub } as FilterArguments;

filter.init(filterArgs);
filter.setValues(['2']);
Expand All @@ -88,7 +90,7 @@ describe('CompoundSliderFilter', () => {

it('should call "setValues" with "operator" set in the filter arguments and expect that value, converted as a string, to be in the callback when triggered', () => {
const spyCallback = jest.spyOn(filterArguments, 'callback');
const filterArgs = { ...filterArguments, operator: '<=' } as FilterArguments;
const filterArgs = { ...filterArguments, operator: '<=', grid: gridStub } as FilterArguments;

filter.init(filterArgs);
filter.setValues(3);
Expand Down Expand Up @@ -138,7 +140,7 @@ describe('CompoundSliderFilter', () => {
});

it('should create the input filter with default search terms range when passed as a filter argument', () => {
const filterArgs = { ...filterArguments, operator: '<=', searchTerms: [3] } as FilterArguments;
const filterArgs = { ...filterArguments, operator: '<=', searchTerms: [3], grid: gridStub } as FilterArguments;

filter.init(filterArgs);
const filterNumberElm = divContainer.querySelector('.input-group-text') as HTMLInputElement;
Expand All @@ -150,7 +152,7 @@ describe('CompoundSliderFilter', () => {
});

it('should create the input filter with default search terms and a different step size when "valueStep" is provided', () => {
const filterArgs = { ...filterArguments, operator: '<=', searchTerms: [15] } as FilterArguments;
const filterArgs = { ...filterArguments, operator: '<=', searchTerms: [15], grid: gridStub } as FilterArguments;
mockColumn.filter!.valueStep = 5;

filter.init(filterArgs);
Expand Down Expand Up @@ -205,7 +207,7 @@ describe('CompoundSliderFilter', () => {
});

it('should trigger a callback with the clear filter set when calling the "clear" method', () => {
const filterArgs = { ...filterArguments, operator: '<=', searchTerms: [3] } as FilterArguments;
const filterArgs = { ...filterArguments, operator: '<=', searchTerms: [3], grid: gridStub } as FilterArguments;
const spyCallback = jest.spyOn(filterArguments, 'callback');

filter.init(filterArgs);
Expand All @@ -216,7 +218,7 @@ describe('CompoundSliderFilter', () => {
});

it('should trigger a callback with the clear filter but without querying when when calling the "clear" method with False as argument', () => {
const filterArgs = { ...filterArguments, operator: '<=', searchTerms: [3] } as FilterArguments;
const filterArgs = { ...filterArguments, operator: '<=', searchTerms: [3], grid: gridStub } as FilterArguments;
const spyCallback = jest.spyOn(filterArguments, 'callback');

filter.init(filterArgs);
Expand All @@ -227,7 +229,7 @@ describe('CompoundSliderFilter', () => {
});

it('should trigger a callback with the clear filter set when calling the "clear" method and expect min slider values being with values of "sliderStartValue" when defined through the filter params', () => {
const filterArgs = { ...filterArguments, operator: '<=', searchTerms: [3] } as FilterArguments;
const filterArgs = { ...filterArguments, operator: '<=', searchTerms: [3], grid: gridStub, } as FilterArguments;
const spyCallback = jest.spyOn(filterArguments, 'callback');
mockColumn.filter = {
params: {
Expand Down Expand Up @@ -262,7 +264,7 @@ describe('CompoundSliderFilter', () => {
it('should have custom compound operator list showing up in the operator select dropdown options list', () => {
mockColumn.outputType = null as any;
filterArguments.searchTerms = ['9'];
mockColumn.filter.compoundOperatorList = [
mockColumn.filter!.compoundOperatorList = [
{ operator: '', description: '' },
{ operator: '=', description: 'Equal to' },
{ operator: '<', description: 'Less than' },
Expand Down
54 changes: 28 additions & 26 deletions packages/common/src/filters/__tests__/sliderFilter.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { Filters } from '../filters.index';
import { Column, FilterArguments, GridOption, SlickGrid } from '../../interfaces/index';
import { Column, FilterArguments, GridOption, SlickGrid, SlickNamespace } from '../../interfaces/index';
import { SliderFilter } from '../sliderFilter';

const containerId = 'demo-container';
declare const Slick: SlickNamespace;

// define a <div> container to simulate the grid container
const template = `<div id="${containerId}"></div>`;
Expand All @@ -17,12 +18,13 @@ const gridStub = {
getColumns: jest.fn(),
getHeaderRowColumn: jest.fn(),
render: jest.fn(),
onHeaderMouseLeave: new Slick.Event(),
} as unknown as SlickGrid;

describe('SliderFilter', () => {
let divContainer: HTMLDivElement;
let filter: SliderFilter;
let filterArguments: FilterArguments;
let filterArgs: FilterArguments;
let spyGetHeaderRow;
let mockColumn: Column;

Expand All @@ -33,7 +35,7 @@ describe('SliderFilter', () => {
spyGetHeaderRow = jest.spyOn(gridStub, 'getHeaderRowColumn').mockReturnValue(divContainer);

mockColumn = { id: 'duration', field: 'duration', filterable: true, filter: { model: Filters.slider } };
filterArguments = {
filterArgs = {
grid: gridStub,
columnDef: mockColumn,
callback: jest.fn(),
Expand All @@ -52,24 +54,24 @@ describe('SliderFilter', () => {
});

it('should initialize the filter', () => {
filter.init(filterArguments);
filter.init(filterArgs);
const filterCount = divContainer.querySelectorAll('.search-filter.slider-container.filter-duration').length;

expect(spyGetHeaderRow).toHaveBeenCalled();
expect(filterCount).toBe(1);
});

it('should have an aria-label when creating the filter', () => {
filter.init(filterArguments);
filter.init(filterArgs);
const filterInputElm = divContainer.querySelector('.search-filter.slider-container.filter-duration input') as HTMLInputElement;

expect(filterInputElm.getAttribute('aria-label')).toBe('Duration Search Filter');
});

it('should call "setValues" and expect that value to be in the callback when triggered', () => {
const spyCallback = jest.spyOn(filterArguments, 'callback');
const spyCallback = jest.spyOn(filterArgs, 'callback');

filter.init(filterArguments);
filter.init(filterArgs);
filter.setValues(['2']);
const filterElm = divContainer.querySelector('.search-filter.slider-container.filter-duration input') as HTMLInputElement;
filterElm.dispatchEvent(new CustomEvent('change'));
Expand All @@ -78,9 +80,9 @@ describe('SliderFilter', () => {
});

it('should call "setValues" and expect that value, converted as a string, to be in the callback when triggered', () => {
const spyCallback = jest.spyOn(filterArguments, 'callback');
const spyCallback = jest.spyOn(filterArgs, 'callback');

filter.init(filterArguments);
filter.init(filterArgs);
filter.setValues(3);
const filterElm = divContainer.querySelector('.search-filter.slider-container.filter-duration input') as HTMLInputElement;
filterElm.dispatchEvent(new CustomEvent('change'));
Expand All @@ -94,7 +96,7 @@ describe('SliderFilter', () => {
});

it('should be able to call "setValues" and set empty values and the input to not have the "filled" css class', () => {
filter.init(filterArguments);
filter.init(filterArgs);
filter.setValues(9);
let filledInputElm = divContainer.querySelector('.search-filter.slider-container.filter-duration.filled') as HTMLInputElement;

Expand All @@ -106,9 +108,9 @@ describe('SliderFilter', () => {
});

it('should create the input filter with default search terms range when passed as a filter argument', () => {
filterArguments.searchTerms = [3];
filterArgs.searchTerms = [3];

filter.init(filterArguments);
filter.init(filterArgs);
const filterNumberElm = divContainer.querySelector('.input-group-text') as HTMLInputElement;
const filterFilledElms = divContainer.querySelectorAll('.search-filter.slider-container.filter-duration.filled');

Expand All @@ -118,10 +120,10 @@ describe('SliderFilter', () => {
});

it('should create the input filter with default search terms and a different step size when "valueStep" is provided', () => {
filterArguments.searchTerms = [15];
filterArgs.searchTerms = [15];
mockColumn.filter!.valueStep = 5;

filter.init(filterArguments);
filter.init(filterArgs);
const filterNumberElm = divContainer.querySelector('.input-group-text') as HTMLInputElement;
const filterInputElm = divContainer.querySelector('.search-filter.slider-container.filter-duration input') as HTMLInputElement;

Expand All @@ -136,7 +138,7 @@ describe('SliderFilter', () => {
maxValue: 69,
};

filter.init(filterArguments);
filter.init(filterArgs);

const filterNumberElm = divContainer.querySelector('.input-group-text') as HTMLInputElement;

Expand All @@ -152,7 +154,7 @@ describe('SliderFilter', () => {
}
};

filter.init(filterArguments);
filter.init(filterArgs);

const filterNumberElm = divContainer.querySelector('.input-group-text') as HTMLInputElement;

Expand All @@ -161,10 +163,10 @@ describe('SliderFilter', () => {
});

it('should create the input filter with default search terms range but without showing side numbers when "hideSliderNumber" is set in params', () => {
filterArguments.searchTerms = [3];
filterArgs.searchTerms = [3];
mockColumn.filter!.params = { hideSliderNumber: true };

filter.init(filterArguments);
filter.init(filterArgs);

const filterNumberElms = divContainer.querySelectorAll<HTMLInputElement>('.input-group-text');

Expand All @@ -173,37 +175,37 @@ describe('SliderFilter', () => {
});

it('should trigger a callback with the clear filter set when calling the "clear" method', () => {
filterArguments.searchTerms = [3];
const spyCallback = jest.spyOn(filterArguments, 'callback');
filterArgs.searchTerms = [3];
const spyCallback = jest.spyOn(filterArgs, 'callback');

filter.init(filterArguments);
filter.init(filterArgs);
filter.clear();

expect(filter.getValues()).toBe(0);
expect(spyCallback).toHaveBeenLastCalledWith(expect.anything(), { columnDef: mockColumn, clearFilterTriggered: true, searchTerms: [], shouldTriggerQuery: true });
});

it('should trigger a callback with the clear filter but without querying when when calling the "clear" method with False as argument', () => {
filterArguments.searchTerms = [3];
const spyCallback = jest.spyOn(filterArguments, 'callback');
filterArgs.searchTerms = [3];
const spyCallback = jest.spyOn(filterArgs, 'callback');

filter.init(filterArguments);
filter.init(filterArgs);
filter.clear(false);

expect(filter.getValues()).toBe(0);
expect(spyCallback).toHaveBeenLastCalledWith(expect.anything(), { columnDef: mockColumn, clearFilterTriggered: true, searchTerms: [], shouldTriggerQuery: false });
});

it('should trigger a callback with the clear filter set when calling the "clear" method and expect min slider values being with values of "sliderStartValue" when defined through the filter params', () => {
const spyCallback = jest.spyOn(filterArguments, 'callback');
const spyCallback = jest.spyOn(filterArgs, 'callback');
mockColumn.filter = {
params: {
sliderStartValue: 4,
sliderEndValue: 69,
}
};

filter.init(filterArguments);
filter.init(filterArgs);
filter.clear(false);

expect(filter.getValues()).toEqual(4);
Expand Down
19 changes: 11 additions & 8 deletions packages/common/src/filters/compoundSliderFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { hasData, toSentenceCase } from '@slickgrid-universal/utils';
import {
Column,
ColumnFilter,
DOMEvent,
Filter,
FilterArguments,
FilterCallback,
Expand Down Expand Up @@ -114,14 +115,12 @@ export class CompoundSliderFilter implements Filter {

// step 2, subscribe to the input change event and run the callback when that happens
// also add/remove "filled" class for styling purposes
this._bindEventService.bind(this.filterInputElm, 'change', this.onTriggerEvent.bind(this));
this._bindEventService.bind(this.selectOperatorElm, 'change', this.onTriggerEvent.bind(this));
this._bindEventService.bind(this.filterInputElm, ['change', 'mouseup', 'touchend'], this.onTriggerEvent.bind(this) as EventListener);
this._bindEventService.bind(this.selectOperatorElm, ['change', 'mouseup', 'touchend'], this.onTriggerEvent.bind(this) as EventListener);

// if user chose to display the slider number on the right side, then update it every time it changes
// we need to use both "input" and "change" event to be all cross-browser
if (!this.filterParams.hideSliderNumber) {
this._bindEventService.bind(this.filterInputElm, ['input', 'change'], this.handleInputChange.bind(this));
}
this._bindEventService.bind(this.filterInputElm, ['input', 'change'], this.handleInputChange.bind(this));
}

/**
Expand Down Expand Up @@ -265,7 +264,7 @@ export class CompoundSliderFilter implements Filter {
this.filterInputElm = createDomElement('input', {
type: 'range', name: this._elementRangeInputId,
className: `form-control slider-filter-input range compound-slider ${this._elementRangeInputId}`,
defaultValue, value: searchTermInput,
defaultValue, value: searchTermInput, title: searchTermInput,
min: `${minValue}`, max: `${maxValue}`, step: `${step}`,
});
this.filterInputElm.setAttribute('aria-label', this.columnFilter?.ariaLabel ?? `${toSentenceCase(columnId + '')} Search Filter`);
Expand Down Expand Up @@ -310,13 +309,14 @@ export class CompoundSliderFilter implements Filter {
protected handleInputChange(event: Event) {
const value = (event?.target as HTMLInputElement).value;
if (value !== undefined && value !== null) {
if (this.filterNumberElm?.textContent) {
if (!this.filterParams.hideSliderNumber && this.filterNumberElm?.textContent) {
this.filterNumberElm.textContent = value;
}
this.filterInputElm.title = value;
}
}

protected onTriggerEvent(e: Event | undefined) {
protected onTriggerEvent(e?: DOMEvent<HTMLInputElement>) {
const value = this.filterInputElm.value;
this._currentValue = +value;

Expand All @@ -332,5 +332,8 @@ export class CompoundSliderFilter implements Filter {
// reset both flags for next use
this._clearFilterTriggered = false;
this._shouldTriggerQuery = true;

// trigger leave event to avoid having previous value still being displayed with custom tooltip feat
this.grid?.onHeaderMouseLeave.notify({ column: this.columnDef, grid: this.grid });
}
}
Loading

0 comments on commit 0ca6f3f

Please sign in to comment.