Skip to content

Commit

Permalink
fix(common): changing Slider value(s) should update Tooltip instantly (
Browse files Browse the repository at this point in the history
…#800)

* fix(common): changing Slider value(s) should update Tooltip in sync
- when dragging slider to a new value, it should update the tooltip (when using SlickCustomTooltip plugin) on the fly while dragging, that is whenever the Slider value changes it should reflect the new value instantly on the tooltip as well

* fix(tooltip): should not read slick cell for tooltip when not exist
- if we're not a slick-cell div, we should not be able to execute `grid.getCellFromEvent(event)` in the SlickCustomTooltip plugin
  • Loading branch information
ghiscoding committed Nov 10, 2022
1 parent 83a86d0 commit 9c6be27
Show file tree
Hide file tree
Showing 12 changed files with 189 additions and 58 deletions.
Expand Up @@ -8,6 +8,7 @@ import {
GridOption,
OperatorType,
SliderOption,
SliderRangeOption,
} from '@slickgrid-universal/common';
import { SlickCustomTooltip } from '@slickgrid-universal/custom-tooltip-plugin';
import { ExcelExportService } from '@slickgrid-universal/excel-export';
Expand Down Expand Up @@ -154,8 +155,8 @@ export class Example16 {
exportWithFormatter: false,
formatter: Formatters.percentCompleteBar,
sortable: true, filterable: true,
filter: { model: Filters.sliderRange, operator: '>=' },
customTooltip: { useRegularTooltip: true, position: 'center' },
filter: { model: Filters.sliderRange, operator: '>=', filterOptions: { hideSliderNumbers: true } as SliderRangeOption },
customTooltip: { position: 'center', formatter: (row, cell, value) => `${value}%`, headerFormatter: null, headerRowFormatter: null },
},
{
id: 'start', name: 'Start', field: 'start', sortable: true,
Expand Down
3 changes: 3 additions & 0 deletions packages/common/src/editors/__tests__/sliderEditor.spec.ts
Expand Up @@ -30,6 +30,7 @@ const gridStub = {
getOptions: () => gridOptionMock,
render: jest.fn(),
onBeforeEditCell: new Slick.Event(),
onMouseEnter: new Slick.Event(),
onCompositeEditorChange: new Slick.Event(),
} as unknown as SlickGrid;

Expand Down Expand Up @@ -215,6 +216,7 @@ describe('SliderEditor', () => {
});

it('should update slider number every time a change event happens on the input slider', () => {
const cellMouseEnterSpy = jest.spyOn(gridStub.onMouseEnter, 'notify');
(mockColumn.internalColumnEditor as ColumnEditor).params = { hideSliderNumber: false };
mockItemData = { id: 1, price: 32, isActive: true };
editor = new SliderEditor(editorArguments);
Expand All @@ -229,6 +231,7 @@ describe('SliderEditor', () => {

expect(editor.isValueChanged()).toBe(true);
expect(editorNumberElm.textContent).toBe('13');
expect(cellMouseEnterSpy).toHaveBeenCalledWith({ grid: gridStub }, expect.anything());
});

describe('isValueChanged method', () => {
Expand Down
17 changes: 13 additions & 4 deletions packages/common/src/editors/sliderEditor.ts
Expand Up @@ -33,6 +33,7 @@ export class SliderEditor implements Editor {
protected _defaultValue = 0;
protected _isValueTouched = false;
protected _originalValue?: number | string;
protected _cellContainerElm!: HTMLDivElement;
protected _editorElm!: HTMLDivElement;
protected _inputElm!: HTMLInputElement;
protected _sliderOptions!: CurrentSliderOption;
Expand Down Expand Up @@ -98,9 +99,9 @@ export class SliderEditor implements Editor {
}

init(): void {
const container = this.args && this.args.container;
this._cellContainerElm = this.args?.container;

if (container && this.columnDef) {
if (this._cellContainerElm && this.columnDef) {
// define the input & slider number IDs
const compositeEditorOptions = this.args.compositeEditorOptions;

Expand All @@ -113,7 +114,7 @@ export class SliderEditor implements Editor {
}

// watch on change event
container.appendChild(this._editorElm);
this._cellContainerElm.appendChild(this._editorElm);
this._bindEventService.bind(this._sliderTrackElm, ['click', 'mouseup'], this.sliderTrackClicked.bind(this) as EventListener);
this._bindEventService.bind(this._editorElm, ['change', 'mouseup', 'touchend'], this.handleChangeEvent.bind(this) as EventListener);

Expand Down Expand Up @@ -313,7 +314,7 @@ export class SliderEditor implements Editor {
*/
protected buildDomElement(): HTMLDivElement {
const columnId = this.columnDef?.id ?? '';
const title = this.columnEditor && this.columnEditor.title || '';
const title = this.columnEditor?.title ?? '';
const minValue = +(this.columnEditor?.minValue ?? Constants.SLIDER_DEFAULT_MIN_VALUE);
const maxValue = +(this.columnEditor?.maxValue ?? Constants.SLIDER_DEFAULT_MAX_VALUE);
const step = +(this.columnEditor?.valueStep ?? Constants.SLIDER_DEFAULT_STEP);
Expand Down Expand Up @@ -377,6 +378,14 @@ export class SliderEditor implements Editor {
this._sliderNumberElm.textContent = value;
}
this._inputElm.title = value;

// trigger mouse enter event on the editor for optionally hooked SlickCustomTooltip
if (!this.args?.compositeEditorOptions) {
this.grid.onMouseEnter.notify(
{ grid: this.grid },
{ ...new Slick.EventData(), target: event?.target }
);
}
}
this.updateTrackFilledColorWhenEnabled();
}
Expand Down
16 changes: 14 additions & 2 deletions packages/common/src/filters/__tests__/compoundSliderFilter.spec.ts
Expand Up @@ -89,12 +89,24 @@ describe('CompoundSliderFilter', () => {
const filterElm = divContainer.querySelector('.input-group.search-filter.filter-duration input') as HTMLInputElement;
filterElm.dispatchEvent(new Event('change'));

jest.runAllTimers(); // fast-forward timer

expect(callbackSpy).toHaveBeenLastCalledWith(expect.anything(), { columnDef: mockColumn, operator: '>', searchTerms: [2], shouldTriggerQuery: true });
expect(rowMouseEnterSpy).toHaveBeenCalledWith({ column: mockColumn, grid: gridStub }, expect.anything());
});

it('should trigger an slider input change event and expect slider value to be updated and also "onHeaderRowMouseEnter" to be notified', () => {
const rowMouseEnterSpy = jest.spyOn(gridStub.onHeaderRowMouseEnter, 'notify');
const filterArgs = { ...filterArguments, operator: '>', grid: gridStub } as FilterArguments;

filter.init(filterArgs);
filter.setValues(['2']);
const filterNumberElm = divContainer.querySelector('.input-group-text') as HTMLInputElement;
const filterElm = divContainer.querySelector('.input-group.search-filter.filter-duration input') as HTMLInputElement;
filterElm.dispatchEvent(new Event('input'));

expect(filterNumberElm.textContent).toBe('2');
expect(rowMouseEnterSpy).toHaveBeenCalledWith({ column: mockColumn, grid: gridStub }, expect.anything());
});

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 callbackSpy = jest.spyOn(filterArguments, 'callback');
const filterArgs = { ...filterArguments, operator: '<=', grid: gridStub } as FilterArguments;
Expand Down
15 changes: 13 additions & 2 deletions packages/common/src/filters/__tests__/singleSliderFilter.spec.ts
Expand Up @@ -83,12 +83,23 @@ describe('SingleSliderFilter', () => {
const filterElm = divContainer.querySelector('.search-filter.slider-container.filter-duration input') as HTMLInputElement;
filterElm.dispatchEvent(new Event('change'));

jest.runAllTimers(); // fast-forward timer

expect(callbackSpy).toHaveBeenLastCalledWith(new Event('change'), { columnDef: mockColumn, operator: 'GE', searchTerms: [2], shouldTriggerQuery: true });
expect(rowMouseEnterSpy).toHaveBeenCalledWith({ column: mockColumn, grid: gridStub }, expect.anything());
});

it('should trigger an slider input change event and expect slider value to be updated and also "onHeaderRowMouseEnter" to be notified', () => {
const rowMouseEnterSpy = jest.spyOn(gridStub.onHeaderRowMouseEnter, 'notify');

filter.init(filterArgs);
filter.setValues(['2']);
const filterNumberElm = divContainer.querySelector('.input-group-text') as HTMLInputElement;
const filterElm = divContainer.querySelector('.input-group.search-filter.filter-duration input') as HTMLInputElement;
filterElm.dispatchEvent(new Event('input'));

expect(filterNumberElm.textContent).toBe('2');
expect(rowMouseEnterSpy).toHaveBeenCalledWith({ column: mockColumn, grid: gridStub }, expect.anything());
});

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

Expand Down
18 changes: 16 additions & 2 deletions packages/common/src/filters/__tests__/sliderRangeFilter.spec.ts
Expand Up @@ -102,12 +102,26 @@ describe('SliderRangeFilter', () => {
const filterElms = divContainer.querySelectorAll<HTMLInputElement>('.search-filter.slider-container.filter-duration input');
filterElms[0].dispatchEvent(new CustomEvent('change'));

jest.runAllTimers(); // fast-forward timer

expect(callbackSpy).toHaveBeenLastCalledWith(expect.anything(), { columnDef: mockColumn, operator: 'RangeInclusive', searchTerms: [2, 80], shouldTriggerQuery: true });
expect(rowMouseEnterSpy).toHaveBeenCalledWith({ column: mockColumn, grid: gridStub }, expect.anything());
});

it('should trigger an slider input change event and expect slider value to be updated and also "onHeaderRowMouseEnter" to be notified', () => {
const rowMouseEnterSpy = jest.spyOn(gridStub.onHeaderRowMouseEnter, 'notify');

filter.init(filterArguments);
filter.setValues([2, 80]);
const filterElms = divContainer.querySelectorAll<HTMLInputElement>('.search-filter.slider-container.filter-duration input');
filterElms[0].dispatchEvent(new CustomEvent('input'));

const filterLowestElm = divContainer.querySelector('.lowest-range-duration') as HTMLInputElement;
const filterHighestElm = divContainer.querySelector('.highest-range-duration') as HTMLInputElement;

expect(filterLowestElm.textContent).toBe('2');
expect(filterHighestElm.textContent).toBe('80');
expect(rowMouseEnterSpy).toHaveBeenCalledWith({ column: mockColumn, grid: gridStub }, expect.anything());
});

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

Expand Down
23 changes: 13 additions & 10 deletions packages/common/src/filters/sliderFilter.ts
Expand Up @@ -418,10 +418,10 @@ export class SliderFilter implements Filter {

// trigger mouse enter event on the filter for optionally hooked SlickCustomTooltip
// the minimum requirements for tooltip to work are the columnDef and targetElement
setTimeout(() => this.grid.onHeaderRowMouseEnter.notify(
this.grid.onHeaderRowMouseEnter.notify(
{ column: this.columnDef, grid: this.grid },
{ ...new Slick.EventData(), target: this._argFilterContainerElm }
));
);
}

protected changeBothSliderFocuses(isAddingFocus: boolean) {
Expand All @@ -438,8 +438,6 @@ export class SliderFilter implements Filter {
this._sliderLeftElm.value = String(sliderLeftVal - getFilterOptionByName<SliderRangeOption, 'stopGapBetweenSliderHandles'>(this.columnFilter, 'stopGapBetweenSliderHandles', GAP_BETWEEN_SLIDER_HANDLES)!);
}

this._sliderRangeContainElm.title = this.sliderType === 'double' ? `${sliderLeftVal} - ${sliderRightVal}` : `${sliderRightVal}`;

// change which handle has higher z-index to make them still usable,
// ie when left handle reaches the end, it has to have higher z-index or else it will be stuck below
// and we cannot move right because it cannot go below min value
Expand All @@ -453,12 +451,7 @@ export class SliderFilter implements Filter {
}
}

this.updateTrackFilledColorWhenEnabled();
this.changeBothSliderFocuses(true);
const hideSliderNumbers = getFilterOptionByName<SliderOption, 'hideSliderNumber'>(this.columnFilter, 'hideSliderNumber') ?? getFilterOptionByName<SliderRangeOption, 'hideSliderNumbers'>(this.columnFilter, 'hideSliderNumbers');
if (!hideSliderNumbers && this._leftSliderNumberElm?.textContent) {
this._leftSliderNumberElm.textContent = this._sliderLeftElm?.value ?? '';
}
this.sliderLeftOrRightChanged(sliderLeftVal, sliderRightVal);
}

protected slideRightInputChanged() {
Expand All @@ -469,6 +462,10 @@ export class SliderFilter implements Filter {
this._sliderRightElm.value = String(sliderLeftVal + getFilterOptionByName<SliderRangeOption, 'stopGapBetweenSliderHandles'>(this.columnFilter, 'stopGapBetweenSliderHandles', GAP_BETWEEN_SLIDER_HANDLES)!);
}

this.sliderLeftOrRightChanged(sliderLeftVal, sliderRightVal);
}

protected sliderLeftOrRightChanged(sliderLeftVal: number, sliderRightVal: number) {
this.updateTrackFilledColorWhenEnabled();
this.changeBothSliderFocuses(true);
this._sliderRangeContainElm.title = this.sliderType === 'double' ? `${sliderLeftVal} - ${sliderRightVal}` : `${sliderRightVal}`;
Expand All @@ -477,6 +474,12 @@ export class SliderFilter implements Filter {
if (!hideSliderNumbers && this._rightSliderNumberElm?.textContent) {
this._rightSliderNumberElm.textContent = this._sliderRightElm?.value ?? '';
}

// also trigger mouse enter event on the filter in case a SlickCustomTooltip is attached
this.grid.onHeaderRowMouseEnter.notify(
{ column: this.columnDef, grid: this.grid },
{ ...new Slick.EventData(), target: this._argFilterContainerElm }
);
}

protected sliderTrackClicked(e: MouseEvent) {
Expand Down
6 changes: 3 additions & 3 deletions packages/common/src/styles/_variables.scss
Expand Up @@ -104,7 +104,7 @@ $slick-header-filter-row-border-bottom: 0 none !default;
$slick-header-filter-row-border-left: 0 none !default;
$slick-header-font-size: $slick-font-size-base !default;
$slick-header-font-weight: bold !default;
$slick-header-input-height: 27px !default; // height of the filter form element (input, textarea, select)
$slick-header-input-height: 100% !default; // height of the filter form element (input, textarea, select)
$slick-header-input-padding: 0 6px !default; // padding of the filter form element (input, textarea, select)
$slick-header-padding-top: 4px !default;
$slick-header-padding-right: 4px !default;
Expand Down Expand Up @@ -455,7 +455,7 @@ $slick-text-editor-padding-top: 0 !default;
$slick-text-editor-focus-border-color: $slick-editor-focus-border-color !default;
$slick-text-editor-focus-box-shadow: $slick-editor-focus-box-shadow !default;
$slick-text-editor-readonly-color: #f0f0f0 !default;
$slick-slider-editor-height: $slick-editor-input-height !default;
$slick-slider-editor-height: 100% !default;
$slick-slider-editor-runnable-track-padding: 0 6px !default;
$slick-slider-editor-number-padding: 4px 6px !default;
$slick-slider-editor-focus-border-color: $slick-editor-focus-border-color !default;
Expand Down Expand Up @@ -564,7 +564,7 @@ $slick-editor-modal-large-editor-count-color: $slick-large-editor-
$slick-editor-modal-large-editor-count-font-size: 10px !default;
$slick-editor-modal-large-editor-count-margin: 0 !default;
$slick-editor-modal-multiselect-editor-height: $slick-editor-modal-input-editor-height !default;
$slick-editor-modal-slider-editor-value-height: $slick-editor-modal-input-editor-height !default;
$slick-editor-modal-slider-editor-value-height: 100% !default;
$slick-editor-modal-slider-editor-value-min-height: 100% !default;
$slick-editor-modal-title-font-color: #333333 !default;
$slick-editor-modal-title-font-size: 20px !default;
Expand Down
1 change: 1 addition & 0 deletions packages/common/src/styles/slick-editors.scss
Expand Up @@ -383,6 +383,7 @@
}
}
.input-group {
display: flex;
position: relative;
height: var(--slick-editor-modal-input-editor-height, $slick-editor-modal-input-editor-height);
input {
Expand Down
6 changes: 6 additions & 0 deletions packages/common/src/styles/slick-plugins.scss
Expand Up @@ -971,6 +971,7 @@ input.flatpickr.form-control {

.slider-container {
display: flex;
height: 100%;

input[type=range] {
/*removes default webkit styles*/
Expand Down Expand Up @@ -1131,6 +1132,10 @@ input.flatpickr.form-control {
border-bottom-left-radius: var(--slick-slider-filter-border-radius, $slick-slider-filter-border-radius);
border-top-left-radius: var(--slick-slider-filter-border-radius, $slick-slider-filter-border-radius);
}
.input-group-addon:last-child .input-group-text {
border-bottom-left-radius: 0;
border-top-left-radius: 0;
}
}

.slider-editor {
Expand All @@ -1146,6 +1151,7 @@ input.flatpickr.form-control {
}

.slider-input-container {
height: 100%;
position: relative;
flex: 1 1 auto;
width: 1%;
Expand Down

0 comments on commit 9c6be27

Please sign in to comment.