Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.2] [Controls] Clear range/time slider selections when field changes (#129824) #130827

Merged
merged 1 commit into from
Apr 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,11 @@

import { BoolQuery } from '@kbn/es-query';
import { FieldSpec } from '../../../../data_views/common';
import { ControlInput } from '../../types';
import { DataControlInput } from '../../types';

export const OPTIONS_LIST_CONTROL = 'optionsListControl';

export interface OptionsListEmbeddableInput extends ControlInput {
fieldName: string;
dataViewId: string;

export interface OptionsListEmbeddableInput extends DataControlInput {
selectedOptions?: string[];
singleSelect?: boolean;
loading?: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,12 @@
* Side Public License, v 1.
*/

import { ControlInput } from '../../types';
import { DataControlInput } from '../../types';

export const RANGE_SLIDER_CONTROL = 'rangeSliderControl';

export type RangeValue = [string, string];

export interface RangeSliderEmbeddableInput extends ControlInput {
fieldName: string;
dataViewId: string;
export interface RangeSliderEmbeddableInput extends DataControlInput {
value: RangeValue;
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@
* Side Public License, v 1.
*/

import { ControlInput } from '../../types';
import { DataControlInput } from '../../types';

export const TIME_SLIDER_CONTROL = 'timeSlider';

export interface TimeSliderControlEmbeddableInput extends ControlInput {
fieldName: string;
dataViewId: string;
export interface TimeSliderControlEmbeddableInput extends DataControlInput {
value?: [number | null, number | null];
}
5 changes: 5 additions & 0 deletions src/plugins/controls/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,8 @@ export type ControlInput = EmbeddableInput & {
controlStyle?: ControlStyle;
ignoreParentSettings?: ParentIgnoreSettings;
};

export type DataControlInput = ControlInput & {
fieldName: string;
dataViewId: string;
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import React, { FC, useCallback, useState } from 'react';
import React, { FC, useCallback } from 'react';
import { BehaviorSubject } from 'rxjs';

import { DataViewField } from '../../../../data_views/public';
Expand Down Expand Up @@ -45,16 +45,13 @@ export const RangeSliderComponent: FC<Props> = ({ componentStateSubject }) => {
componentStateSubject.getValue()
);

const { value = ['', ''], id, title } = useEmbeddableSelector((state) => state);

const [selectedValue, setSelectedValue] = useState<RangeValue>(value || ['', '']);
const { value, id, title } = useEmbeddableSelector((state) => state);

const onChangeComplete = useCallback(
(range: RangeValue) => {
dispatch(selectRange(range));
setSelectedValue(range);
},
[selectRange, setSelectedValue, dispatch]
[selectRange, dispatch]
);

return (
Expand All @@ -64,7 +61,7 @@ export const RangeSliderComponent: FC<Props> = ({ componentStateSubject }) => {
min={min}
max={max}
title={title}
value={selectedValue}
value={value ?? ['', '']}
onChange={onChangeComplete}
fieldFormatter={fieldFormatter}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ export class RangeSliderEmbeddable extends Embeddable<RangeSliderEmbeddableInput
this.updateOutput({ filters: [rangeFilter], dataViews: [dataView], loading: false });
};

reload = () => {
public reload = () => {
this.fetchMinMax();
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ export class RangeSliderEmbeddableFactory
) => {
if (
embeddable &&
(!deepEqual(newInput.fieldName, embeddable.getInput().fieldName) ||
!deepEqual(newInput.dataViewId, embeddable.getInput().dataViewId))
((newInput.fieldName && !deepEqual(newInput.fieldName, embeddable.getInput().fieldName)) ||
(newInput.dataViewId && !deepEqual(newInput.dataViewId, embeddable.getInput().dataViewId)))
) {
// if the field name or data view id has changed in this editing session, selected values are invalid, so reset them.
newInput.value = ['', ''];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import React, { FC, useCallback, useState, useMemo } from 'react';
import React, { FC, useCallback, useMemo } from 'react';
import { BehaviorSubject } from 'rxjs';
import { debounce } from 'lodash';
import { useStateObservable } from '../../hooks/use_state_observable';
Expand Down Expand Up @@ -59,10 +59,6 @@ export const TimeSlider: FC<TimeSliderProps> = ({

const { value } = useEmbeddableSelector((state) => state);

const [selectedValue, setSelectedValue] = useState<[number | null, number | null]>(
value || [null, null]
);

const dispatchChange = useCallback(
(range: [number | null, number | null]) => {
dispatch(selectRange(range));
Expand All @@ -75,15 +71,14 @@ export const TimeSlider: FC<TimeSliderProps> = ({
const onChangeComplete = useCallback(
(range: [number | null, number | null]) => {
debouncedDispatchChange(range);
setSelectedValue(range);
},
[setSelectedValue, debouncedDispatchChange]
[debouncedDispatchChange]
);

return (
<Component
onChange={onChangeComplete}
value={selectedValue}
value={value ?? [null, null]}
range={[min, max]}
dateFormat={dateFormat}
timezone={timezone}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ export class TimesliderEmbeddableFactory
) => {
if (
embeddable &&
(!deepEqual(newInput.fieldName, embeddable.getInput().fieldName) ||
!deepEqual(newInput.dataViewId, embeddable.getInput().dataViewId))
((newInput.fieldName && !deepEqual(newInput.fieldName, embeddable.getInput().fieldName)) ||
(newInput.dataViewId && !deepEqual(newInput.dataViewId, embeddable.getInput().dataViewId)))
) {
// if the field name or data view id has changed in this editing session, selected options are invalid, so reset them.
newInput.value = undefined;
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/controls/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,4 @@ export interface ControlsPluginStartDeps {
}

// re-export from common
export type { ControlWidth, ControlInput, ControlStyle } from '../common/types';
export type { ControlWidth, ControlInput, DataControlInput, ControlStyle } from '../common/types';
30 changes: 30 additions & 0 deletions test/functional/apps/dashboard_elements/controls/options_list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,36 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
});
});

it('editing field clears selections', async () => {
const secondId = (await dashboardControls.getAllControlIds())[1];
await dashboardControls.optionsListOpenPopover(secondId);
await dashboardControls.optionsListPopoverSelectOption('hiss');
await dashboardControls.optionsListEnsurePopoverIsClosed(secondId);

await dashboardControls.editExistingControl(secondId);
await dashboardControls.controlsEditorSetfield('animal.keyword');
await dashboardControls.controlEditorSave();

const selectionString = await dashboardControls.optionsListGetSelectionsString(secondId);
expect(selectionString).to.be('Select...');
});

it('editing other control settings keeps selections', async () => {
const secondId = (await dashboardControls.getAllControlIds())[1];
await dashboardControls.optionsListOpenPopover(secondId);
await dashboardControls.optionsListPopoverSelectOption('dog');
await dashboardControls.optionsListPopoverSelectOption('cat');
await dashboardControls.optionsListEnsurePopoverIsClosed(secondId);

await dashboardControls.editExistingControl(secondId);
await dashboardControls.controlEditorSetTitle('Animal');
await dashboardControls.controlEditorSetWidth('large');
await dashboardControls.controlEditorSave();

const selectionString = await dashboardControls.optionsListGetSelectionsString(secondId);
expect(selectionString).to.be('dog, cat');
});

it('deletes an existing control', async () => {
const firstId = (await dashboardControls.getAllControlIds())[0];

Expand Down
76 changes: 44 additions & 32 deletions test/functional/apps/dashboard_elements/controls/range_slider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,20 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
'header',
]);

const validateRange = async (
compare: 'value' | 'placeholder', // if 'value', compare actual selections; otherwise, compare the default range
controlId: string,
expectedLowerBound: string,
expectedUpperBound: string
) => {
expect(await dashboardControls.rangeSliderGetLowerBoundAttribute(controlId, compare)).to.be(
expectedLowerBound
);
expect(await dashboardControls.rangeSliderGetUpperBoundAttribute(controlId, compare)).to.be(
expectedUpperBound
);
};

describe('Range Slider Control', async () => {
before(async () => {
await security.testUser.setRoles([
Expand Down Expand Up @@ -82,12 +96,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
});
expect(await dashboardControls.getControlsCount()).to.be(2);
const secondId = (await dashboardControls.getAllControlIds())[1];
expect(
await dashboardControls.rangeSliderGetLowerBoundAttribute(secondId, 'placeholder')
).to.be('100');
expect(
await dashboardControls.rangeSliderGetUpperBoundAttribute(secondId, 'placeholder')
).to.be('1200');
validateRange('placeholder', secondId, '100', '1200');
// data views should be properly propagated from the control group to the dashboard
expect(await filterBar.getIndexPatterns()).to.be('logstash-*,kibana_sample_data_flights');
});
Expand All @@ -112,12 +121,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await dashboardControls.controlsEditorSetfield('dayOfWeek');
await dashboardControls.controlEditorSave();
await dashboardControls.rangeSliderWaitForLoading();
expect(
await dashboardControls.rangeSliderGetLowerBoundAttribute(firstId, 'placeholder')
).to.be('0');
expect(
await dashboardControls.rangeSliderGetUpperBoundAttribute(firstId, 'placeholder')
).to.be('6');
validateRange('placeholder', firstId, '0', '6');

// when creating a new filter, the ability to select a data view should be removed, because the dashboard now only has one data view
await retry.try(async () => {
await testSubjects.click('addFilter');
Expand Down Expand Up @@ -150,31 +155,38 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
it('applies filter from the first control on the second control', async () => {
await dashboardControls.rangeSliderWaitForLoading();
const secondId = (await dashboardControls.getAllControlIds())[1];
const availableMin = await dashboardControls.rangeSliderGetLowerBoundAttribute(
secondId,
'placeholder'
);
expect(availableMin).to.be('100');
const availabeMax = await dashboardControls.rangeSliderGetUpperBoundAttribute(
secondId,
'placeholder'
);
expect(availabeMax).to.be('1000');
validateRange('placeholder', secondId, '100', '1000');
});

it('editing field clears selections', async () => {
const secondId = (await dashboardControls.getAllControlIds())[1];
await dashboardControls.editExistingControl(secondId);
await dashboardControls.controlsEditorSetfield('FlightDelayMin');
await dashboardControls.controlEditorSave();

await dashboardControls.rangeSliderWaitForLoading();
validateRange('value', secondId, '', '');
});

it('editing other control settings keeps selections', async () => {
const secondId = (await dashboardControls.getAllControlIds())[1];
await dashboardControls.rangeSliderSetLowerBound(secondId, '50');
await dashboardControls.rangeSliderSetUpperBound(secondId, '100');
await dashboardControls.rangeSliderWaitForLoading();

await dashboardControls.editExistingControl(secondId);
await dashboardControls.controlEditorSetTitle('Minimum Flight Delay');
await dashboardControls.controlEditorSetWidth('large');
await dashboardControls.controlEditorSave();

await dashboardControls.rangeSliderWaitForLoading();
validateRange('value', secondId, '50', '100');
});

it('can clear out selections by clicking the reset button', async () => {
const firstId = (await dashboardControls.getAllControlIds())[0];
await dashboardControls.rangeSliderClearSelection(firstId);
const lowerBoundSelection = await dashboardControls.rangeSliderGetLowerBoundAttribute(
firstId,
'value'
);
expect(lowerBoundSelection.length).to.be(0);
const upperBoundSelection = await dashboardControls.rangeSliderGetUpperBoundAttribute(
firstId,
'value'
);
expect(upperBoundSelection.length).to.be(0);
validateRange('value', firstId, '', '');
});

it('deletes an existing control', async () => {
Expand Down