From 0ca6f3f4d8894d4bb9459cabca9a3492e7cca0ad Mon Sep 17 00:00:00 2001 From: Ghislain B Date: Thu, 11 Aug 2022 16:41:58 -0400 Subject: [PATCH] feat(common): update title prop on change event for Slider Filter/Editor (#743) - whenever the slider value changes, we should update its title property with the new value --- packages/common/src/editors/sliderEditor.ts | 12 +++-- .../__tests__/compoundSliderFilter.spec.ts | 20 +++---- .../filters/__tests__/sliderFilter.spec.ts | 54 ++++++++++--------- .../src/filters/compoundSliderFilter.ts | 19 ++++--- packages/common/src/filters/sliderFilter.ts | 19 ++++--- 5 files changed, 68 insertions(+), 56 deletions(-) diff --git a/packages/common/src/editors/sliderEditor.ts b/packages/common/src/editors/sliderEditor.ts index 44bb85328..9ecbdcf7a 100644 --- a/packages/common/src/editors/sliderEditor.ts +++ b/packages/common/src/editors/sliderEditor.ts @@ -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)); } } @@ -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}`; @@ -357,8 +356,11 @@ export class SliderEditor implements Editor { protected handleChangeSliderNumber(event: Event) { const value = (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; } } diff --git a/packages/common/src/filters/__tests__/compoundSliderFilter.spec.ts b/packages/common/src/filters/__tests__/compoundSliderFilter.spec.ts index 4a01ac565..e7cbc6b7e 100644 --- a/packages/common/src/filters/__tests__/compoundSliderFilter.spec.ts +++ b/packages/common/src/filters/__tests__/compoundSliderFilter.spec.ts @@ -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
container to simulate the grid container const template = `
`; @@ -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', () => { @@ -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']); @@ -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); @@ -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; @@ -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); @@ -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); @@ -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); @@ -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: { @@ -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' }, diff --git a/packages/common/src/filters/__tests__/sliderFilter.spec.ts b/packages/common/src/filters/__tests__/sliderFilter.spec.ts index 38a64b9fe..504c02c6c 100644 --- a/packages/common/src/filters/__tests__/sliderFilter.spec.ts +++ b/packages/common/src/filters/__tests__/sliderFilter.spec.ts @@ -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
container to simulate the grid container const template = `
`; @@ -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; @@ -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(), @@ -52,7 +54,7 @@ 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(); @@ -60,16 +62,16 @@ describe('SliderFilter', () => { }); 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')); @@ -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')); @@ -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; @@ -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'); @@ -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; @@ -136,7 +138,7 @@ describe('SliderFilter', () => { maxValue: 69, }; - filter.init(filterArguments); + filter.init(filterArgs); const filterNumberElm = divContainer.querySelector('.input-group-text') as HTMLInputElement; @@ -152,7 +154,7 @@ describe('SliderFilter', () => { } }; - filter.init(filterArguments); + filter.init(filterArgs); const filterNumberElm = divContainer.querySelector('.input-group-text') as HTMLInputElement; @@ -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('.input-group-text'); @@ -173,10 +175,10 @@ 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); @@ -184,10 +186,10 @@ describe('SliderFilter', () => { }); 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); @@ -195,7 +197,7 @@ describe('SliderFilter', () => { }); 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, @@ -203,7 +205,7 @@ describe('SliderFilter', () => { } }; - filter.init(filterArguments); + filter.init(filterArgs); filter.clear(false); expect(filter.getValues()).toEqual(4); diff --git a/packages/common/src/filters/compoundSliderFilter.ts b/packages/common/src/filters/compoundSliderFilter.ts index 17dcf78e7..d47b0927d 100644 --- a/packages/common/src/filters/compoundSliderFilter.ts +++ b/packages/common/src/filters/compoundSliderFilter.ts @@ -3,6 +3,7 @@ import { hasData, toSentenceCase } from '@slickgrid-universal/utils'; import { Column, ColumnFilter, + DOMEvent, Filter, FilterArguments, FilterCallback, @@ -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)); } /** @@ -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`); @@ -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) { const value = this.filterInputElm.value; this._currentValue = +value; @@ -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 }); } } diff --git a/packages/common/src/filters/sliderFilter.ts b/packages/common/src/filters/sliderFilter.ts index 5f03a2f62..3506c1f46 100644 --- a/packages/common/src/filters/sliderFilter.ts +++ b/packages/common/src/filters/sliderFilter.ts @@ -4,6 +4,7 @@ import { OperatorType, OperatorString, SearchTerm, } from '../enums/index'; import { Column, ColumnFilter, + DOMEvent, Filter, FilterArguments, FilterCallback, @@ -94,13 +95,11 @@ export class SliderFilter implements Filter { // step 2, subscribe to the change event and run the callback when that happens // also add/remove "filled" class for styling purposes - this._bindEventService.bind(this.filterInputElm, 'change', this.handleOnChange.bind(this)); + this._bindEventService.bind(this.filterInputElm, ['change', 'mouseup', 'touchend'], this.handleOnChange.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)); } /** @@ -199,7 +198,7 @@ export class SliderFilter implements Filter { this.filterInputElm = createDomElement('input', { type: 'range', name: this._elementRangeInputId, className: `form-control slider-filter-input range ${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`); @@ -233,14 +232,15 @@ export class SliderFilter 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 handleOnChange(e: any) { - const value = e && e.target && e.target.value; + protected handleOnChange(e: DOMEvent) { + const value = e?.target?.value ?? ''; this._currentValue = +value; if (this._clearFilterTriggered) { @@ -253,5 +253,8 @@ export class SliderFilter 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 }); } }