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

* Reset selections on save of existing control

* Allow reset to force render for range/time sliders

* Reset selections only when field name or data view changes

* Make generic DataControlInput interface

* Fix infinite useEffect + imports + types

* Simpler solution without resetSelections()

* Add functional tests

* Apply Devon's changes
This commit is contained in:
Hannah Mudge 2022-04-21 13:39:09 -06:00 committed by GitHub
parent 4d78a770a1
commit 5a86421003
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 98 additions and 66 deletions

View file

@ -8,14 +8,11 @@
import { BoolQuery } from '@kbn/es-query';
import { FieldSpec } from '@kbn/data-views-plugin/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;

View file

@ -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;
}

View file

@ -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];
}

View file

@ -27,3 +27,8 @@ export type ControlInput = EmbeddableInput & {
controlStyle?: ControlStyle;
ignoreParentSettings?: ParentIgnoreSettings;
};
export type DataControlInput = ControlInput & {
fieldName: string;
dataViewId: string;
};

View file

@ -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 '@kbn/data-views-plugin/public';
@ -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 (
@ -64,7 +61,7 @@ export const RangeSliderComponent: FC<Props> = ({ componentStateSubject }) => {
min={min}
max={max}
title={title}
value={selectedValue}
value={value ?? ['', '']}
onChange={onChangeComplete}
fieldFormatter={fieldFormatter}
/>

View file

@ -311,7 +311,7 @@ export class RangeSliderEmbeddable extends Embeddable<RangeSliderEmbeddableInput
this.updateOutput({ filters: [rangeFilter], dataViews: [dataView], loading: false });
};
reload = () => {
public reload = () => {
this.fetchMinMax();
};

View file

@ -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 = ['', ''];

View file

@ -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 { useReduxEmbeddableContext } from '@kbn/presentation-util-plugin/public';
@ -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));
@ -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}

View file

@ -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;

View file

@ -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';

View file

@ -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];

View file

@ -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([
@ -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');
});
@ -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');
@ -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 () => {