From 3b1cd481928178e73ada8659f82ba769484b8132 Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Mon, 5 Jun 2023 15:24:14 -0600 Subject: [PATCH 01/22] Replace with `EuiFormControlLayoutDelimited` --- .../options_list/components/options_list.scss | 2 +- .../range_slider/components/range_slider.scss | 30 ++--- .../components/range_slider_control.tsx | 118 +++++++----------- 3 files changed, 61 insertions(+), 89 deletions(-) diff --git a/src/plugins/controls/public/options_list/components/options_list.scss b/src/plugins/controls/public/options_list/components/options_list.scss index ad042916fff6e..26fc6321f5a55 100644 --- a/src/plugins/controls/public/options_list/components/options_list.scss +++ b/src/plugins/controls/public/options_list/components/options_list.scss @@ -33,7 +33,7 @@ color: $euiTextSubduedColor; text-decoration: line-through; margin-left: $euiSizeS; - font-weight: 300; + font-weight: $euiFontWeightRegular;; } .optionsList__existsFilter { diff --git a/src/plugins/controls/public/range_slider/components/range_slider.scss b/src/plugins/controls/public/range_slider/components/range_slider.scss index d1a360b465962..a06e3445e39f8 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider.scss +++ b/src/plugins/controls/public/range_slider/components/range_slider.scss @@ -17,42 +17,36 @@ } .rangeSliderAnchor__button { - display: flex; - align-items: center; width: 100%; height: 100%; - justify-content: space-between; background-color: $euiFormBackgroundColor; - @include euiFormControlSideBorderRadius($euiFormControlBorderRadius, $side: 'right', $internal: true); + padding: 0px; + .euiFormControlLayout__childrenWrapper { + border-radius: 0 $euiFormControlBorderRadius $euiFormControlBorderRadius 0 !important; + } + .euiToolTipAnchor { width: 100%; } - .rangeSliderAnchor__delimiter { - background-color: unset; - padding: $euiSizeS*1.5 0; - } .rangeSliderAnchor__fieldNumber { font-weight: $euiFontWeightBold; box-shadow: none; text-align: center; background-color: unset; + &:invalid { + color: $euiTextSubduedColor; + text-decoration: line-through; + font-weight: $euiFontWeightRegular;; + background-image: none; // hide the red bottom border + } + &::placeholder { font-weight: $euiFontWeightRegular; color: $euiColorMediumShade; text-decoration: none; } } - - .rangeSliderAnchor__fieldNumber--invalid { - text-decoration: line-through; - font-weight: $euiFontWeightRegular; - color: $euiColorMediumShade; - } - - .rangeSliderAnchor__spinner { - padding-right: $euiSizeS; - } } \ No newline at end of file diff --git a/src/plugins/controls/public/range_slider/components/range_slider_control.tsx b/src/plugins/controls/public/range_slider/components/range_slider_control.tsx index e483cf4bb16ae..74043328a4214 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_control.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_control.tsx @@ -8,14 +8,7 @@ import React, { FC, useState, useRef } from 'react'; -import { - EuiFieldNumber, - EuiText, - EuiInputPopover, - EuiLoadingSpinner, - EuiFlexGroup, - EuiFlexItem, -} from '@elastic/eui'; +import { EuiFieldNumber, EuiInputPopover, EuiFormControlLayoutDelimited } from '@elastic/eui'; import { useRangeSlider } from '../embeddable/range_slider_embeddable'; import { RangeSliderPopover, EuiDualRangeRef } from './range_slider_popover'; @@ -23,7 +16,7 @@ import { RangeSliderPopover, EuiDualRangeRef } from './range_slider_popover'; import './range_slider.scss'; import { ControlError } from '../../control_group/component/control_error_component'; -const INVALID_CLASS = 'rangeSliderAnchor__fieldNumber--invalid'; +const RangeSliderInput = () => {}; export const RangeSliderControl: FC = () => { const rangeRef = useRef(null); @@ -55,68 +48,53 @@ export const RangeSliderControl: FC = () => { const roundedMax = hasAvailableRange ? Math.ceil(maxValue) : maxValue; const button = ( - + onClick={(event) => { + // the popover should remain open if the click target is one of the number inputs + if (isPopoverOpen && event.target instanceof HTMLInputElement) { + return; + } + setIsPopoverOpen(!isPopoverOpen); + }} + startControl={ + { + rangeSlider.dispatch.setSelectedRange([ + event.target.value, + isNaN(upperBoundValue) ? '' : String(upperBoundValue), + ]); + }} + disabled={isLoading} + placeholder={`${hasAvailableRange ? roundedMin : ''}`} + isInvalid={isInvalid} + data-test-subj="rangeSlider__lowerBoundFieldNumber" + /> + } + endControl={ + { + rangeSlider.dispatch.setSelectedRange([ + isNaN(lowerBoundValue) ? '' : String(lowerBoundValue), + event.target.value, + ]); + }} + placeholder={`${hasAvailableRange ? roundedMax : ''}`} + isInvalid={isInvalid} + data-test-subj="rangeSlider__upperBoundFieldNumber" + /> + } + /> ); return error ? ( From 2c26893a051c64872b34ea1bc71bbe2cb01dd81b Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Wed, 7 Jun 2023 10:23:14 -0600 Subject: [PATCH 02/22] Add debounce through unpublished changes --- .../components/options_list_popover.tsx | 2 +- .../components/range_slider_button.tsx | 71 ++++++++++++ .../components/range_slider_control.tsx | 79 +++---------- .../components/range_slider_popover.tsx | 107 +++++++----------- .../embeddable/range_slider_embeddable.tsx | 92 +++++++++++---- .../range_slider/range_slider_reducers.ts | 4 +- .../controls/public/range_slider/types.ts | 4 +- 7 files changed, 202 insertions(+), 157 deletions(-) create mode 100644 src/plugins/controls/public/range_slider/components/range_slider_button.tsx diff --git a/src/plugins/controls/public/options_list/components/options_list_popover.tsx b/src/plugins/controls/public/options_list/components/options_list_popover.tsx index 79e20925ed369..a8a17011760bf 100644 --- a/src/plugins/controls/public/options_list/components/options_list_popover.tsx +++ b/src/plugins/controls/public/options_list/components/options_list_popover.tsx @@ -42,7 +42,7 @@ export const OptionsListPopover = ({ const hideActionBar = optionsList.select((state) => state.explicitInput.hideActionBar); const [showOnlySelected, setShowOnlySelected] = useState(false); - + console.log('render'); return ( <>
) => void; + onChange: (newRange: [string, string]) => void; + currentRange: [string, string]; +}) => { + const rangeSlider = useRangeSlider(); + + const min = rangeSlider.select((state) => state.componentState.min); + const max = rangeSlider.select((state) => state.componentState.max); + const isInvalid = rangeSlider.select((state) => state.componentState.isInvalid); + + const id = rangeSlider.select((state) => state.explicitInput.id); + const value = rangeSlider.select((state) => state.explicitInput.value); + const isLoading = rangeSlider.select((state) => state.output.loading); + + return ( + { + onChange([event.target.value, value[1]]); + }} + placeholder={String(min)} + isInvalid={isInvalid} + className={'rangeSliderAnchor__fieldNumber'} + /> + } + endControl={ + { + onChange([value[0], event.target.value]); + }} + placeholder={String(max)} + isInvalid={isInvalid} + className={'rangeSliderAnchor__fieldNumber'} + /> + } + /> + ); +}; diff --git a/src/plugins/controls/public/range_slider/components/range_slider_control.tsx b/src/plugins/controls/public/range_slider/components/range_slider_control.tsx index 74043328a4214..5e07e9d63eba9 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_control.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_control.tsx @@ -6,17 +6,16 @@ * Side Public License, v 1. */ -import React, { FC, useState, useRef } from 'react'; +import React, { FC, useState, useRef, useEffect } from 'react'; -import { EuiFieldNumber, EuiInputPopover, EuiFormControlLayoutDelimited } from '@elastic/eui'; +import { EuiInputPopover } from '@elastic/eui'; import { useRangeSlider } from '../embeddable/range_slider_embeddable'; import { RangeSliderPopover, EuiDualRangeRef } from './range_slider_popover'; import './range_slider.scss'; import { ControlError } from '../../control_group/component/control_error_component'; - -const RangeSliderInput = () => {}; +import { RangeSliderButton } from './range_slider_button'; export const RangeSliderControl: FC = () => { const rangeRef = useRef(null); @@ -24,34 +23,19 @@ export const RangeSliderControl: FC = () => { const rangeSlider = useRangeSlider(); - const min = rangeSlider.select((state) => state.componentState.min); - const max = rangeSlider.select((state) => state.componentState.max); const error = rangeSlider.select((state) => state.componentState.error); - const isInvalid = rangeSlider.select((state) => state.componentState.isInvalid); - - const id = rangeSlider.select((state) => state.explicitInput.id); - const value = rangeSlider.select((state) => state.explicitInput.value) ?? ['', '']; - const isLoading = rangeSlider.select((state) => state.output.loading); - - const hasAvailableRange = min !== '' && max !== ''; - - const hasLowerBoundSelection = value[0] !== ''; - const hasUpperBoundSelection = value[1] !== ''; + const value = rangeSlider.select((state) => state.explicitInput.value); - const lowerBoundValue = parseFloat(value[0]); - const upperBoundValue = parseFloat(value[1]); - const minValue = parseFloat(min); - const maxValue = parseFloat(max); + const [currentRange, setCurrentRange] = useState(value); - // EuiDualRange can only handle integers as min/max - const roundedMin = hasAvailableRange ? Math.floor(minValue) : minValue; - const roundedMax = hasAvailableRange ? Math.ceil(maxValue) : maxValue; + useEffect(() => { + rangeSlider.dispatch.setSelectedRange(currentRange); + }, [currentRange, rangeSlider.dispatch]); const button = ( - { // the popover should remain open if the click target is one of the number inputs if (isPopoverOpen && event.target instanceof HTMLInputElement) { @@ -59,41 +43,6 @@ export const RangeSliderControl: FC = () => { } setIsPopoverOpen(!isPopoverOpen); }} - startControl={ - { - rangeSlider.dispatch.setSelectedRange([ - event.target.value, - isNaN(upperBoundValue) ? '' : String(upperBoundValue), - ]); - }} - disabled={isLoading} - placeholder={`${hasAvailableRange ? roundedMin : ''}`} - isInvalid={isInvalid} - data-test-subj="rangeSlider__lowerBoundFieldNumber" - /> - } - endControl={ - { - rangeSlider.dispatch.setSelectedRange([ - isNaN(lowerBoundValue) ? '' : String(lowerBoundValue), - event.target.value, - ]); - }} - placeholder={`${hasAvailableRange ? roundedMax : ''}`} - isInvalid={isInvalid} - data-test-subj="rangeSlider__upperBoundFieldNumber" - /> - } /> ); @@ -116,7 +65,11 @@ export const RangeSliderControl: FC = () => { rangeRef.current?.onResize(width); }} > - + ); }; diff --git a/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx b/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx index c3b2ccbe676f4..84e169bd6a601 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import React, { FC, ComponentProps, Ref, useEffect, useState } from 'react'; +import React, { FC, ComponentProps, Ref, useEffect, useState, useMemo } from 'react'; import useMount from 'react-use/lib/useMount'; import { @@ -14,7 +14,6 @@ import { EuiFlexGroup, EuiFlexItem, EuiDualRange, - EuiText, EuiToolTip, EuiButtonIcon, } from '@elastic/eui'; @@ -22,13 +21,16 @@ import type { EuiDualRangeClass } from '@elastic/eui/src/components/form/range/d import { pluginServices } from '../../services'; import { RangeSliderStrings } from './range_slider_strings'; -import { RangeValue } from '../../../common/range_slider/types'; import { useRangeSlider } from '../embeddable/range_slider_embeddable'; // Unfortunately, wrapping EuiDualRange in `withEuiTheme` has created this annoying/verbose typing export type EuiDualRangeRef = EuiDualRangeClass & ComponentProps; -export const RangeSliderPopover: FC<{ rangeRef?: Ref }> = ({ rangeRef }) => { +export const RangeSliderPopover: FC<{ + onChange: (newValue: [string, string]) => void; + currentRange: [string, string]; + rangeRef?: Ref; +}> = ({ onChange, currentRange, rangeRef }) => { const [fieldFormatter, setFieldFormatter] = useState(() => (toFormat: string) => toFormat); // Controls Services Context @@ -41,77 +43,48 @@ export const RangeSliderPopover: FC<{ rangeRef?: Ref }> = ({ ra const dataViewId = rangeSlider.select((state) => state.output.dataViewId); const fieldSpec = rangeSlider.select((state) => state.componentState.field); const id = rangeSlider.select((state) => state.explicitInput.id); - const isInvalid = rangeSlider.select((state) => state.componentState.isInvalid); + // const isInvalid = rangeSlider.select((state) => state.componentState.isInvalid); const max = rangeSlider.select((state) => state.componentState.max); const min = rangeSlider.select((state) => state.componentState.min); const title = rangeSlider.select((state) => state.explicitInput.title); - const value = rangeSlider.select((state) => state.explicitInput.value) ?? ['', '']; - const hasAvailableRange = min !== '' && max !== ''; - const hasLowerBoundSelection = value[0] !== ''; - const hasUpperBoundSelection = value[1] !== ''; + const hasAvailableRange = min !== undefined && max !== undefined; - const lowerBoundSelection = parseFloat(value[0]); - const upperBoundSelection = parseFloat(value[1]); - const minValue = parseFloat(min); - const maxValue = parseFloat(max); + // const errorMessage = ''; + // let helpText = ''; - // EuiDualRange can only handle integers as min/max - const roundedMin = hasAvailableRange ? Math.floor(minValue) : minValue; - const roundedMax = hasAvailableRange ? Math.ceil(maxValue) : maxValue; + // if (!hasAvailableRange) { + // helpText = RangeSliderStrings.popover.getNoAvailableDataHelpText(); + // } else if (isInvalid) { + // helpText = RangeSliderStrings.popover.getNoDataHelpText(); + // } // Caches min and max displayed on popover open so the range slider doesn't resize as selections change - const [rangeSliderMin, setRangeSliderMin] = useState(roundedMin); - const [rangeSliderMax, setRangeSliderMax] = useState(roundedMax); + const [rangeSliderMin, setRangeSliderMin] = useState(min); + const [rangeSliderMax, setRangeSliderMax] = useState(max); useMount(() => { + const [lowerBoundSelection, upperBoundSelection] = [ + parseFloat(currentRange[0]), + parseFloat(currentRange[1]), + ]; + setRangeSliderMin( Math.min( - roundedMin, + min, isNaN(lowerBoundSelection) ? Infinity : lowerBoundSelection, isNaN(upperBoundSelection) ? Infinity : upperBoundSelection ) ); setRangeSliderMax( Math.max( - roundedMax, + max, isNaN(lowerBoundSelection) ? -Infinity : lowerBoundSelection, isNaN(upperBoundSelection) ? -Infinity : upperBoundSelection ) ); }); - const errorMessage = ''; - let helpText = ''; - - if (!hasAvailableRange) { - helpText = RangeSliderStrings.popover.getNoAvailableDataHelpText(); - } else if (isInvalid) { - helpText = RangeSliderStrings.popover.getNoDataHelpText(); - } - - const displayedValue = [ - hasLowerBoundSelection - ? String(lowerBoundSelection) - : hasAvailableRange - ? String(roundedMin) - : '', - hasUpperBoundSelection - ? String(upperBoundSelection) - : hasAvailableRange - ? String(roundedMax) - : '', - ] as RangeValue; - - const ticks = []; - const levels = []; - - if (hasAvailableRange) { - ticks.push({ value: rangeSliderMin, label: fieldFormatter(String(rangeSliderMin)) }); - ticks.push({ value: rangeSliderMax, label: fieldFormatter(String(rangeSliderMax)) }); - levels.push({ min: roundedMin, max: roundedMax, color: 'success' }); - } - // derive field formatter from fieldSpec and dataViewId useEffect(() => { (async () => { @@ -126,6 +99,17 @@ export const RangeSliderPopover: FC<{ rangeRef?: Ref }> = ({ ra })(); }, [fieldSpec, dataViewId, getDataViewById]); + const ticks = useMemo(() => { + return [ + { value: min, label: fieldFormatter(String(min)) }, + { value: max, label: fieldFormatter(String(max)) }, + ]; + }, [min, max, fieldFormatter]); + + const levels = useMemo(() => { + return [{ min, max, color: 'success' }]; + }, [min, max]); + return ( <> {title} @@ -138,17 +122,12 @@ export const RangeSliderPopover: FC<{ rangeRef?: Ref }> = ({ ra { - const updatedLowerBound = - typeof newLowerBound === 'number' ? String(newLowerBound) : value[0]; - const updatedUpperBound = - typeof newUpperBound === 'number' ? String(newUpperBound) : value[1]; - - rangeSlider.dispatch.setSelectedRange([updatedLowerBound, updatedUpperBound]); + min={rangeSliderMin} + max={rangeSliderMax} + onChange={([minSelection, maxSelection]) => { + onChange([String(minSelection), String(maxSelection)]); }} - value={displayedValue} + value={currentRange} ticks={hasAvailableRange ? ticks : undefined} levels={hasAvailableRange ? levels : undefined} showTicks={hasAvailableRange} @@ -157,13 +136,13 @@ export const RangeSliderPopover: FC<{ rangeRef?: Ref }> = ({ ra ref={rangeRef} data-test-subj="rangeSlider__slider" /> - {errorMessage || helpText} - + */} @@ -171,7 +150,7 @@ export const RangeSliderPopover: FC<{ rangeRef?: Ref }> = ({ ra iconType="eraser" color="danger" onClick={() => { - rangeSlider.dispatch.setSelectedRange(['', '']); + onChange(['', '']); }} aria-label={RangeSliderStrings.popover.getClearRangeButtonTitle()} data-test-subj="rangeSlider__clearRangeButton" diff --git a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx index e1bacc7327310..7e3c4c50e7fa4 100644 --- a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx +++ b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx @@ -12,7 +12,7 @@ import { isEmpty } from 'lodash'; import { batch } from 'react-redux'; import { get, isEqual } from 'lodash'; import deepEqual from 'fast-deep-equal'; -import { Subscription, lastValueFrom } from 'rxjs'; +import { Subscription, lastValueFrom, BehaviorSubject } from 'rxjs'; import { debounceTime, distinctUntilChanged, skip, map } from 'rxjs/operators'; import { @@ -41,6 +41,7 @@ import { ControlsDataService } from '../../services/data/types'; import { RangeSliderControl } from '../components/range_slider_control'; import { ControlsDataViewsService } from '../../services/data_views/types'; import { getDefaultComponentState, rangeSliderReducers } from '../range_slider_reducers'; +import { RangeValue } from '@kbn/controls-plugin/common/range_slider/types'; const diffDataFetchProps = ( current?: RangeSliderDataFetchProps, @@ -100,6 +101,10 @@ export class RangeSliderEmbeddable extends Embeddable = new BehaviorSubject({}); + private cleanupStateTools: () => void; constructor( @@ -126,13 +131,14 @@ export class RangeSliderEmbeddable extends Embeddable { const initialValue = this.getInput().value; if (!initialValue) { + console.log('no initial value'); this.setInitializationFinished(); } @@ -145,6 +151,7 @@ export class RangeSliderEmbeddable extends Embeddable { if (initialValue) { + await this.buildFilter(); this.setInitializationFinished(); } this.setupSubscriptions(); @@ -170,24 +177,61 @@ export class RangeSliderEmbeddable extends Embeddable - this.runRangeSliderQuery().catch((e) => { - this.dispatch.setErrorMessage(e.message); - }) + this.runRangeSliderQuery() + .catch((e) => { + this.dispatch.setErrorMessage(e.message); + }) + .then(async () => { + await this.buildFilter(); + }) ) ); - // build filters when value change + // write to unpublished changes when value changes this.subscriptions.add( this.getInput$() - .pipe( - debounceTime(400), - distinctUntilChanged((a, b) => isEqual(a.value, b.value)), - skip(1) // skip the first input update because initial filters will be built by initialize. - ) - .subscribe(this.buildFilter) + .pipe(distinctUntilChanged((a, b) => isEqual(a.value, b.value))) + .subscribe(({ value }) => { + // console.log('value changed, output to unpublished'); + this.unpublishedChanges.next({ range: value }); + }) ); + + // debounce publishing of unpublished changes for smoother selection experience + this.subscriptions.add( + this.unpublishedChanges.pipe(debounceTime(400)).subscribe(() => { + this.publishNewRange(); + }) + ); + + // // build filters when value change + // this.subscriptions.add( + // this.getInput$() + // .pipe( + // debounceTime(400), + // distinctUntilChanged((a, b) => isEqual(a.value, b.value)), + // skip(1) // skip the first input update because initial filters will be built by initialize. + // ) + // .subscribe(this.buildFilter) + // ); }; + public async publishNewRange() { + // console.log('publish changes'); + const { range } = this.unpublishedChanges.getValue(); + if (!range) return; + + const [newLowerBound, newUpperBound] = range; + + const updatedLowerBound = + typeof newLowerBound === 'number' ? String(newLowerBound) : newLowerBound; + const updatedUpperBound = + typeof newUpperBound === 'number' ? String(newUpperBound) : newUpperBound; + this.dispatch.setSelectedRange([updatedLowerBound, updatedUpperBound]); + await this.buildFilter(); + this.unpublishedChanges.next({}); + } + private getCurrentDataViewAndField = async (): Promise<{ dataView?: DataView; field?: DataViewField; @@ -285,15 +329,11 @@ export class RangeSliderEmbeddable extends Embeddable { throw e; }); - + console.log('set min max', min, max); this.dispatch.setMinMax({ min: `${min ?? ''}`, max: `${max ?? ''}`, }); - // build filter with new min/max - await this.buildFilter().catch((e) => { - throw e; - }); }; private fetchMinMax = async ({ @@ -306,7 +346,8 @@ export class RangeSliderEmbeddable extends Embeddable { + }): Promise<{ min?: number; max?: number }> => { + console.log('fetch min max'); const searchSource = await this.dataService.searchSource.create(); searchSource.setField('size', 0); searchSource.setField('index', dataView); @@ -343,8 +384,8 @@ export class RangeSliderEmbeddable extends Embeddable { throw e; }); - const min = get(resp, 'rawResponse.aggregations.minAgg.value', ''); - const max = get(resp, 'rawResponse.aggregations.maxAgg.value', ''); + const min = get(resp, 'rawResponse.aggregations.minAgg.value'); + const max = get(resp, 'rawResponse.aggregations.maxAgg.value'); return { min, max }; }; @@ -357,10 +398,10 @@ export class RangeSliderEmbeddable extends Embeddable { this.dispatch.setLoading(false); @@ -443,6 +483,8 @@ export class RangeSliderEmbeddable extends Embeddable { diff --git a/src/plugins/controls/public/range_slider/range_slider_reducers.ts b/src/plugins/controls/public/range_slider/range_slider_reducers.ts index e8fcf0a10b2c3..b2b356e9e453f 100644 --- a/src/plugins/controls/public/range_slider/range_slider_reducers.ts +++ b/src/plugins/controls/public/range_slider/range_slider_reducers.ts @@ -53,8 +53,8 @@ export const rangeSliderReducers = { state: WritableDraft, action: PayloadAction<{ min: string; max: string }> ) => { - state.componentState.min = action.payload.min; - state.componentState.max = action.payload.max; + state.componentState.min = Math.floor(parseFloat(action.payload.min)); + state.componentState.max = Math.ceil(parseFloat(action.payload.max)); }, publishFilters: ( state: WritableDraft, diff --git a/src/plugins/controls/public/range_slider/types.ts b/src/plugins/controls/public/range_slider/types.ts index ad321271631e0..8b283d300e6ae 100644 --- a/src/plugins/controls/public/range_slider/types.ts +++ b/src/plugins/controls/public/range_slider/types.ts @@ -15,8 +15,8 @@ import { ControlOutput } from '../types'; // Component state is only used by public components. export interface RangeSliderComponentState { field?: FieldSpec; - min: string; - max: string; + min: number; + max: number; error?: string; isInvalid?: boolean; } From b38dd7acc0ac96e149d184d7892caf34741f33df Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Wed, 7 Jun 2023 15:54:30 -0600 Subject: [PATCH 03/22] Add error message --- .../components/range_slider_popover.tsx | 39 +++++++------------ 1 file changed, 14 insertions(+), 25 deletions(-) diff --git a/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx b/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx index 84e169bd6a601..a1948a22fc681 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx @@ -16,6 +16,7 @@ import { EuiDualRange, EuiToolTip, EuiButtonIcon, + EuiText, } from '@elastic/eui'; import type { EuiDualRangeClass } from '@elastic/eui/src/components/form/range/dual_range'; @@ -41,23 +42,14 @@ export const RangeSliderPopover: FC<{ // Select current state from Redux using multiple selectors to avoid rerenders. const dataViewId = rangeSlider.select((state) => state.output.dataViewId); - const fieldSpec = rangeSlider.select((state) => state.componentState.field); + const id = rangeSlider.select((state) => state.explicitInput.id); - // const isInvalid = rangeSlider.select((state) => state.componentState.isInvalid); - const max = rangeSlider.select((state) => state.componentState.max); - const min = rangeSlider.select((state) => state.componentState.min); const title = rangeSlider.select((state) => state.explicitInput.title); - const hasAvailableRange = min !== undefined && max !== undefined; - - // const errorMessage = ''; - // let helpText = ''; - - // if (!hasAvailableRange) { - // helpText = RangeSliderStrings.popover.getNoAvailableDataHelpText(); - // } else if (isInvalid) { - // helpText = RangeSliderStrings.popover.getNoDataHelpText(); - // } + const min = rangeSlider.select((state) => state.componentState.min); + const max = rangeSlider.select((state) => state.componentState.max); + const fieldSpec = rangeSlider.select((state) => state.componentState.field); + const isInvalid = rangeSlider.select((state) => state.componentState.isInvalid); // Caches min and max displayed on popover open so the range slider doesn't resize as selections change const [rangeSliderMin, setRangeSliderMin] = useState(min); @@ -128,21 +120,18 @@ export const RangeSliderPopover: FC<{ onChange([String(minSelection), String(maxSelection)]); }} value={currentRange} - ticks={hasAvailableRange ? ticks : undefined} - levels={hasAvailableRange ? levels : undefined} - showTicks={hasAvailableRange} - disabled={!hasAvailableRange} + ticks={ticks} + levels={levels} + showTicks fullWidth ref={rangeRef} data-test-subj="rangeSlider__slider" /> - {/* - {errorMessage || helpText} - */} + {isInvalid && ( + + {RangeSliderStrings.popover.getNoDataHelpText()} + + )} From b62fa6c25403ca56ab2c2960fb90d61ad6972a5a Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Thu, 8 Jun 2023 12:19:24 -0600 Subject: [PATCH 04/22] Clean up --- .../options_list/components/options_list.scss | 2 +- .../components/options_list_popover.tsx | 2 +- .../components/range_slider_control.tsx | 5 +++- .../embeddable/range_slider_embeddable.tsx | 27 +++---------------- 4 files changed, 10 insertions(+), 26 deletions(-) diff --git a/src/plugins/controls/public/options_list/components/options_list.scss b/src/plugins/controls/public/options_list/components/options_list.scss index 26fc6321f5a55..0309437b8c9b3 100644 --- a/src/plugins/controls/public/options_list/components/options_list.scss +++ b/src/plugins/controls/public/options_list/components/options_list.scss @@ -33,7 +33,7 @@ color: $euiTextSubduedColor; text-decoration: line-through; margin-left: $euiSizeS; - font-weight: $euiFontWeightRegular;; + font-weight: $euiFontWeightRegular; } .optionsList__existsFilter { diff --git a/src/plugins/controls/public/options_list/components/options_list_popover.tsx b/src/plugins/controls/public/options_list/components/options_list_popover.tsx index a8a17011760bf..79e20925ed369 100644 --- a/src/plugins/controls/public/options_list/components/options_list_popover.tsx +++ b/src/plugins/controls/public/options_list/components/options_list_popover.tsx @@ -42,7 +42,7 @@ export const OptionsListPopover = ({ const hideActionBar = optionsList.select((state) => state.explicitInput.hideActionBar); const [showOnlySelected, setShowOnlySelected] = useState(false); - console.log('render'); + return ( <>
{ className="rangeSlider__popoverOverride" anchorClassName="rangeSlider__anchorOverride" panelClassName="rangeSlider__panelOverride" - closePopover={() => setIsPopoverOpen(false)} + closePopover={() => { + rangeSlider.publishNewRange(); + setIsPopoverOpen(false); + }} anchorPosition="downCenter" attachToAnchor={false} disableFocusTrap diff --git a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx index 7e3c4c50e7fa4..fa395dd1616e7 100644 --- a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx +++ b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx @@ -131,14 +131,13 @@ export class RangeSliderEmbeddable extends Embeddable { const initialValue = this.getInput().value; if (!initialValue) { - console.log('no initial value'); + this.unpublishedChanges.next({ range: ['', ''] }); this.setInitializationFinished(); } @@ -192,44 +191,30 @@ export class RangeSliderEmbeddable extends Embeddable isEqual(a.value, b.value))) .subscribe(({ value }) => { - // console.log('value changed, output to unpublished'); this.unpublishedChanges.next({ range: value }); }) ); // debounce publishing of unpublished changes for smoother selection experience this.subscriptions.add( - this.unpublishedChanges.pipe(debounceTime(400)).subscribe(() => { + this.unpublishedChanges.pipe(debounceTime(750)).subscribe(() => { this.publishNewRange(); }) ); - - // // build filters when value change - // this.subscriptions.add( - // this.getInput$() - // .pipe( - // debounceTime(400), - // distinctUntilChanged((a, b) => isEqual(a.value, b.value)), - // skip(1) // skip the first input update because initial filters will be built by initialize. - // ) - // .subscribe(this.buildFilter) - // ); }; public async publishNewRange() { - // console.log('publish changes'); const { range } = this.unpublishedChanges.getValue(); - if (!range) return; + if (!range) return; // this means there are no unpublished changes to publish const [newLowerBound, newUpperBound] = range; - const updatedLowerBound = typeof newLowerBound === 'number' ? String(newLowerBound) : newLowerBound; const updatedUpperBound = typeof newUpperBound === 'number' ? String(newUpperBound) : newUpperBound; this.dispatch.setSelectedRange([updatedLowerBound, updatedUpperBound]); await this.buildFilter(); - this.unpublishedChanges.next({}); + this.unpublishedChanges.next({}); // clear out unpublished changes } private getCurrentDataViewAndField = async (): Promise<{ @@ -329,7 +314,6 @@ export class RangeSliderEmbeddable extends Embeddable { throw e; }); - console.log('set min max', min, max); this.dispatch.setMinMax({ min: `${min ?? ''}`, max: `${max ?? ''}`, @@ -347,7 +331,6 @@ export class RangeSliderEmbeddable extends Embeddable => { - console.log('fetch min max'); const searchSource = await this.dataService.searchSource.create(); searchSource.setField('size', 0); searchSource.setField('index', dataView); @@ -483,8 +466,6 @@ export class RangeSliderEmbeddable extends Embeddable { From d85970660319913f1df822525dcca4c20907c910 Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Thu, 8 Jun 2023 16:44:37 -0600 Subject: [PATCH 05/22] Fix reset dashboard bug --- .../control_group_panel_diff_system.ts | 14 ++++++++++++++ .../controls/common/range_slider/types.ts | 2 +- .../range_slider/components/range_slider.scss | 6 +++--- .../components/range_slider_button.tsx | 15 ++++++--------- .../components/range_slider_control.tsx | 17 ++--------------- .../components/range_slider_popover.tsx | 16 ++++++---------- .../embeddable/range_slider_embeddable.tsx | 5 +---- 7 files changed, 33 insertions(+), 42 deletions(-) diff --git a/src/plugins/controls/common/control_group/control_group_panel_diff_system.ts b/src/plugins/controls/common/control_group/control_group_panel_diff_system.ts index 2b89bc55ba2c0..7630db7785991 100644 --- a/src/plugins/controls/common/control_group/control_group_panel_diff_system.ts +++ b/src/plugins/controls/common/control_group/control_group_panel_diff_system.ts @@ -10,6 +10,7 @@ import deepEqual from 'fast-deep-equal'; import { omit, isEqual } from 'lodash'; import { OPTIONS_LIST_DEFAULT_SORT } from '../options_list/suggestions_sorting'; import { OptionsListEmbeddableInput, OPTIONS_LIST_CONTROL } from '../options_list/types'; +import { RangeSliderEmbeddableInput, RANGE_SLIDER_CONTROL } from '../range_slider/types'; import { ControlPanelState } from './types'; @@ -26,6 +27,19 @@ export const genericControlPanelDiffSystem: DiffSystem = { export const ControlPanelDiffSystems: { [key: string]: DiffSystem; } = { + [RANGE_SLIDER_CONTROL]: { + getPanelIsEqual: (initialInput, newInput) => { + if (!deepEqual(omit(initialInput, 'explicitInput'), omit(newInput, 'explicitInput'))) { + return false; + } + + const { value: valueA = ['', ''] }: Partial = + initialInput.explicitInput; + const { value: valueB = ['', ''] }: Partial = + newInput.explicitInput; + return isEqual(valueA, valueB); + }, + }, [OPTIONS_LIST_CONTROL]: { getPanelIsEqual: (initialInput, newInput) => { if (!deepEqual(omit(initialInput, 'explicitInput'), omit(newInput, 'explicitInput'))) { diff --git a/src/plugins/controls/common/range_slider/types.ts b/src/plugins/controls/common/range_slider/types.ts index 51c6d9e07e241..fdfc65d1d7c36 100644 --- a/src/plugins/controls/common/range_slider/types.ts +++ b/src/plugins/controls/common/range_slider/types.ts @@ -13,7 +13,7 @@ export const RANGE_SLIDER_CONTROL = 'rangeSliderControl'; export type RangeValue = [string, string]; export interface RangeSliderEmbeddableInput extends DataControlInput { - value: RangeValue; + value?: RangeValue; } export type RangeSliderInputWithType = Partial & { type: string }; diff --git a/src/plugins/controls/public/range_slider/components/range_slider.scss b/src/plugins/controls/public/range_slider/components/range_slider.scss index a06e3445e39f8..abdd460da7286 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider.scss +++ b/src/plugins/controls/public/range_slider/components/range_slider.scss @@ -20,12 +20,12 @@ width: 100%; height: 100%; background-color: $euiFormBackgroundColor; - padding: 0px; + padding: 0; .euiFormControlLayout__childrenWrapper { border-radius: 0 $euiFormControlBorderRadius $euiFormControlBorderRadius 0 !important; } - + .euiToolTipAnchor { width: 100%; } @@ -39,7 +39,7 @@ &:invalid { color: $euiTextSubduedColor; text-decoration: line-through; - font-weight: $euiFontWeightRegular;; + font-weight: $euiFontWeightRegular; background-image: none; // hide the red bottom border } diff --git a/src/plugins/controls/public/range_slider/components/range_slider_button.tsx b/src/plugins/controls/public/range_slider/components/range_slider_button.tsx index af2a750dd85b5..09600fb3e744e 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_button.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_button.tsx @@ -16,12 +16,8 @@ import './range_slider.scss'; export const RangeSliderButton = ({ onClick, - onChange, - currentRange, }: { onClick: (event: React.MouseEvent) => void; - onChange: (newRange: [string, string]) => void; - currentRange: [string, string]; }) => { const rangeSlider = useRangeSlider(); @@ -30,7 +26,8 @@ export const RangeSliderButton = ({ const isInvalid = rangeSlider.select((state) => state.componentState.isInvalid); const id = rangeSlider.select((state) => state.explicitInput.id); - const value = rangeSlider.select((state) => state.explicitInput.value); + const value = rangeSlider.select((state) => state.explicitInput.value) ?? ['', '']; + const isLoading = rangeSlider.select((state) => state.output.loading); return ( @@ -44,9 +41,9 @@ export const RangeSliderButton = ({ { - onChange([event.target.value, value[1]]); + rangeSlider.dispatch.setSelectedRange([event.target.value, value[1]]); }} placeholder={String(min)} isInvalid={isInvalid} @@ -57,9 +54,9 @@ export const RangeSliderButton = ({ { - onChange([value[0], event.target.value]); + rangeSlider.dispatch.setSelectedRange([value[0], event.target.value]); }} placeholder={String(max)} isInvalid={isInvalid} diff --git a/src/plugins/controls/public/range_slider/components/range_slider_control.tsx b/src/plugins/controls/public/range_slider/components/range_slider_control.tsx index d44ce07c7a7be..8bf65ee036ef8 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_control.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_control.tsx @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import React, { FC, useState, useRef, useEffect } from 'react'; +import React, { FC, useState, useRef } from 'react'; import { EuiInputPopover } from '@elastic/eui'; @@ -24,18 +24,9 @@ export const RangeSliderControl: FC = () => { const rangeSlider = useRangeSlider(); const error = rangeSlider.select((state) => state.componentState.error); - const value = rangeSlider.select((state) => state.explicitInput.value); - - const [currentRange, setCurrentRange] = useState(value); - - useEffect(() => { - rangeSlider.dispatch.setSelectedRange(currentRange); - }, [currentRange, rangeSlider.dispatch]); const button = ( { // the popover should remain open if the click target is one of the number inputs if (isPopoverOpen && event.target instanceof HTMLInputElement) { @@ -68,11 +59,7 @@ export const RangeSliderControl: FC = () => { rangeRef.current?.onResize(width); }} > - + ); }; diff --git a/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx b/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx index a1948a22fc681..6555e1cc97a26 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx @@ -28,10 +28,8 @@ import { useRangeSlider } from '../embeddable/range_slider_embeddable'; export type EuiDualRangeRef = EuiDualRangeClass & ComponentProps; export const RangeSliderPopover: FC<{ - onChange: (newValue: [string, string]) => void; - currentRange: [string, string]; rangeRef?: Ref; -}> = ({ onChange, currentRange, rangeRef }) => { +}> = ({ rangeRef }) => { const [fieldFormatter, setFieldFormatter] = useState(() => (toFormat: string) => toFormat); // Controls Services Context @@ -45,6 +43,7 @@ export const RangeSliderPopover: FC<{ const id = rangeSlider.select((state) => state.explicitInput.id); const title = rangeSlider.select((state) => state.explicitInput.title); + const value = rangeSlider.select((state) => state.explicitInput.value) ?? ['', '']; const min = rangeSlider.select((state) => state.componentState.min); const max = rangeSlider.select((state) => state.componentState.max); @@ -56,10 +55,7 @@ export const RangeSliderPopover: FC<{ const [rangeSliderMax, setRangeSliderMax] = useState(max); useMount(() => { - const [lowerBoundSelection, upperBoundSelection] = [ - parseFloat(currentRange[0]), - parseFloat(currentRange[1]), - ]; + const [lowerBoundSelection, upperBoundSelection] = [parseFloat(value[0]), parseFloat(value[1])]; setRangeSliderMin( Math.min( @@ -117,9 +113,9 @@ export const RangeSliderPopover: FC<{ min={rangeSliderMin} max={rangeSliderMax} onChange={([minSelection, maxSelection]) => { - onChange([String(minSelection), String(maxSelection)]); + rangeSlider.dispatch.setSelectedRange([String(minSelection), String(maxSelection)]); }} - value={currentRange} + value={value} ticks={ticks} levels={levels} showTicks @@ -139,7 +135,7 @@ export const RangeSliderPopover: FC<{ iconType="eraser" color="danger" onClick={() => { - onChange(['', '']); + rangeSlider.dispatch.setSelectedRange(['', '']); }} aria-label={RangeSliderStrings.popover.getClearRangeButtonTitle()} data-test-subj="rangeSlider__clearRangeButton" diff --git a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx index fa395dd1616e7..e6db556a44b06 100644 --- a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx +++ b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx @@ -41,7 +41,6 @@ import { ControlsDataService } from '../../services/data/types'; import { RangeSliderControl } from '../components/range_slider_control'; import { ControlsDataViewsService } from '../../services/data_views/types'; import { getDefaultComponentState, rangeSliderReducers } from '../range_slider_reducers'; -import { RangeValue } from '@kbn/controls-plugin/common/range_slider/types'; const diffDataFetchProps = ( current?: RangeSliderDataFetchProps, @@ -137,7 +136,6 @@ export class RangeSliderEmbeddable extends Embeddable { const initialValue = this.getInput().value; if (!initialValue) { - this.unpublishedChanges.next({ range: ['', ''] }); this.setInitializationFinished(); } @@ -150,7 +148,6 @@ export class RangeSliderEmbeddable extends Embeddable { if (initialValue) { - await this.buildFilter(); this.setInitializationFinished(); } this.setupSubscriptions(); @@ -191,7 +188,7 @@ export class RangeSliderEmbeddable extends Embeddable isEqual(a.value, b.value))) .subscribe(({ value }) => { - this.unpublishedChanges.next({ range: value }); + this.unpublishedChanges.next({ range: value ?? ['', ''] }); }) ); From 4f7a0f0af0e26525a6feaaddcc542a9941dffd0b Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Fri, 9 Jun 2023 07:47:42 -0600 Subject: [PATCH 06/22] Re-add accidentally deleted `data-test-subj` --- .../public/range_slider/components/range_slider_button.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/plugins/controls/public/range_slider/components/range_slider_button.tsx b/src/plugins/controls/public/range_slider/components/range_slider_button.tsx index 09600fb3e744e..e6687821ab1c2 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_button.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_button.tsx @@ -48,6 +48,7 @@ export const RangeSliderButton = ({ placeholder={String(min)} isInvalid={isInvalid} className={'rangeSliderAnchor__fieldNumber'} + data-test-subj={'rangeSlider__lowerBoundFieldNumber'} /> } endControl={ @@ -61,6 +62,7 @@ export const RangeSliderButton = ({ placeholder={String(max)} isInvalid={isInvalid} className={'rangeSliderAnchor__fieldNumber'} + data-test-subj={'rangeSlider__upperBoundFieldNumber'} /> } /> From 02acc60620d5cd79b8297954feb036040c123ad6 Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Fri, 9 Jun 2023 08:16:53 -0600 Subject: [PATCH 07/22] Fix default min and max --- .../components/range_slider_popover.tsx | 36 +++++++++++-------- .../embeddable/range_slider_embeddable.tsx | 9 ++--- .../range_slider/range_slider_reducers.ts | 4 +-- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx b/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx index 6555e1cc97a26..f9747dae63db5 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx @@ -108,21 +108,27 @@ export const RangeSliderPopover: FC<{ responsive={false} > - { - rangeSlider.dispatch.setSelectedRange([String(minSelection), String(maxSelection)]); - }} - value={value} - ticks={ticks} - levels={levels} - showTicks - fullWidth - ref={rangeRef} - data-test-subj="rangeSlider__slider" - /> + {min !== -Infinity && max !== Infinity ? ( + { + rangeSlider.dispatch.setSelectedRange([String(minSelection), String(maxSelection)]); + }} + value={value} + ticks={ticks} + levels={levels} + showTicks + fullWidth + ref={rangeRef} + data-test-subj="rangeSlider__slider" + /> + ) : ( + + {RangeSliderStrings.popover.getNoAvailableDataHelpText()} + + )} {isInvalid && ( {RangeSliderStrings.popover.getNoDataHelpText()} diff --git a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx index e6db556a44b06..90c13e2114f62 100644 --- a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx +++ b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx @@ -373,14 +373,9 @@ export class RangeSliderEmbeddable extends Embeddable { const { componentState: { min: availableMin, max: availableMax }, - explicitInput: { - query, - timeRange, - filters = [], - ignoreParentSettings, - value: [selectedMin, selectedMax], - }, + explicitInput: { query, timeRange, filters = [], ignoreParentSettings, value }, } = this.getState(); + const [selectedMin, selectedMax] = value ?? ['', '']; const hasData = availableMin !== undefined && availableMax !== undefined; const hasLowerSelection = !isEmpty(selectedMin); const hasUpperSelection = !isEmpty(selectedMax); diff --git a/src/plugins/controls/public/range_slider/range_slider_reducers.ts b/src/plugins/controls/public/range_slider/range_slider_reducers.ts index b2b356e9e453f..c1b5c93c4ce25 100644 --- a/src/plugins/controls/public/range_slider/range_slider_reducers.ts +++ b/src/plugins/controls/public/range_slider/range_slider_reducers.ts @@ -16,9 +16,9 @@ import { RangeSliderReduxState } from './types'; import { RangeValue } from '../../common/range_slider/types'; export const getDefaultComponentState = (): RangeSliderReduxState['componentState'] => ({ - min: '', - max: '', isInvalid: false, + min: -Infinity, + max: Infinity, }); export const rangeSliderReducers = { From 90fee54b0866688adabc91399f5e51625f1b12d2 Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Fri, 9 Jun 2023 12:47:13 -0600 Subject: [PATCH 08/22] Fix functional test + error state --- .../components/range_slider_button.tsx | 2 +- .../embeddable/range_slider_embeddable.tsx | 60 +++++++++---------- .../controls/range_slider.ts | 19 +++--- .../controls/replace_controls.ts | 4 +- .../controls/time_slider.ts | 2 +- .../page_objects/dashboard_page_controls.ts | 11 ++-- .../dashboard_to_dashboard_drilldown.ts | 2 +- 7 files changed, 48 insertions(+), 52 deletions(-) diff --git a/src/plugins/controls/public/range_slider/components/range_slider_button.tsx b/src/plugins/controls/public/range_slider/components/range_slider_button.tsx index e6687821ab1c2..285e5168e6331 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_button.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_button.tsx @@ -33,10 +33,10 @@ export const RangeSliderButton = ({ return ( - new Error(`field ${fieldName} not found in index pattern`); + new Error( + i18n.translate('controls.rangeSlider.errors.fieldNotFound', { + defaultMessage: 'Could not locate field: {fieldName}', + values: { fieldName }, + }) + ); export const RangeSliderControlContext = createContext(null); export const useRangeSlider = (): RangeSliderEmbeddable => { @@ -139,19 +144,18 @@ export class RangeSliderEmbeddable extends Embeddable { - batch(() => { - this.dispatch.setLoading(false); - this.dispatch.setErrorMessage(e.message); - }); - }) - .then(async () => { - if (initialValue) { - this.setInitializationFinished(); - } - this.setupSubscriptions(); + try { + await this.runRangeSliderQuery(); + if (initialValue) { + this.setInitializationFinished(); + } + } catch (e) { + batch(() => { + this.dispatch.setLoading(false); + this.dispatch.setErrorMessage(e.message); }); + } + this.setupSubscriptions(); }; private setupSubscriptions = () => { @@ -172,15 +176,14 @@ export class RangeSliderEmbeddable extends Embeddable - this.runRangeSliderQuery() - .catch((e) => { - this.dispatch.setErrorMessage(e.message); - }) - .then(async () => { - await this.buildFilter(); - }) - ) + dataFetchPipe.subscribe(async () => { + try { + await this.runRangeSliderQuery(); + await this.buildFilter(); + } catch (e) { + this.dispatch.setErrorMessage(e.message); + } + }) ); // write to unpublished changes when value changes @@ -202,7 +205,7 @@ export class RangeSliderEmbeddable extends Embeddable { - await dashboardControls.rangeSliderWaitForLoading(); const secondId = (await dashboardControls.getAllControlIds())[1]; + await dashboardControls.rangeSliderWaitForLoading(secondId); await dashboardControls.validateRange('placeholder', secondId, '100', '1000'); await dashboard.clearUnsavedChanges(); }); @@ -183,15 +184,15 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { it('making changes to range causes unsaved changes', async () => { const firstId = (await dashboardControls.getAllControlIds())[0]; - await dashboardControls.rangeSliderSetLowerBound(firstId, '0'); + await dashboardControls.rangeSliderSetLowerBound(firstId, '2'); await dashboardControls.rangeSliderSetUpperBound(firstId, '3'); - await dashboardControls.rangeSliderWaitForLoading(); + await dashboardControls.rangeSliderWaitForLoading(firstId); await testSubjects.existOrFail('dashboardUnsavedChangesBadge'); }); it('changes to range can be discarded', async () => { const firstId = (await dashboardControls.getAllControlIds())[0]; - await dashboardControls.validateRange('value', firstId, '0', '3'); + await dashboardControls.validateRange('value', firstId, '2', '3'); await dashboard.clickCancelOutOfEditMode(); await dashboardControls.validateRange('value', firstId, '', ''); }); @@ -216,7 +217,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await dashboardControls.rangeSliderSetUpperBound(firstId, '400'); }); - it('disables range slider when no data available', async () => { + it('hides range slider in popover when no data available', async () => { await dashboardControls.createControl({ controlType: RANGE_SLIDER_CONTROL, dataViewTitle: 'logstash-*', @@ -226,9 +227,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const secondId = (await dashboardControls.getAllControlIds())[1]; await dashboardControls.rangeSliderOpenPopover(secondId); await dashboardControls.rangeSliderPopoverAssertOpen(); - expect( - await dashboardControls.rangeSliderGetDualRangeAttribute(secondId, 'disabled') - ).to.be('true'); + await testSubjects.missingOrFail('rangeSlider__slider'); expect((await testSubjects.getVisibleText('rangeSlider__helpText')).length).to.be.above(0); }); }); @@ -250,7 +249,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { it('Applies dashboard query to range slider control', async () => { const firstId = (await dashboardControls.getAllControlIds())[0]; - await dashboardControls.rangeSliderWaitForLoading(); + await dashboardControls.rangeSliderWaitForLoading(firstId); await dashboardControls.validateRange('placeholder', firstId, '100', '300'); await queryBar.setQuery(''); await queryBar.submitQuery(); diff --git a/test/functional/apps/dashboard_elements/controls/replace_controls.ts b/test/functional/apps/dashboard_elements/controls/replace_controls.ts index ff4efebf9cac9..616583b8c69ca 100644 --- a/test/functional/apps/dashboard_elements/controls/replace_controls.ts +++ b/test/functional/apps/dashboard_elements/controls/replace_controls.ts @@ -40,7 +40,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const replaceWithRangeSlider = async (controlId: string) => { await changeFieldType(controlId, 'weightLbs', RANGE_SLIDER_CONTROL); await retry.try(async () => { - await dashboardControls.rangeSliderWaitForLoading(); + await dashboardControls.rangeSliderWaitForLoading(controlId); await dashboardControls.verifyControlType(controlId, 'range-slider-control'); }); }; @@ -89,8 +89,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { dataViewTitle: 'animals-*', fieldName: 'weightLbs', }); - await dashboardControls.rangeSliderWaitForLoading(); controlId = (await dashboardControls.getAllControlIds())[0]; + await dashboardControls.rangeSliderWaitForLoading(controlId); }); afterEach(async () => { diff --git a/test/functional/apps/dashboard_elements/controls/time_slider.ts b/test/functional/apps/dashboard_elements/controls/time_slider.ts index ef75d4e74b24e..06d2a9b10024c 100644 --- a/test/functional/apps/dashboard_elements/controls/time_slider.ts +++ b/test/functional/apps/dashboard_elements/controls/time_slider.ts @@ -96,8 +96,8 @@ 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]; + await dashboardControls.rangeSliderWaitForLoading(secondId); await dashboardControls.validateRange('placeholder', secondId, '101', '1000'); }); diff --git a/test/functional/page_objects/dashboard_page_controls.ts b/test/functional/page_objects/dashboard_page_controls.ts index 8234699225653..9819484f32354 100644 --- a/test/functional/page_objects/dashboard_page_controls.ts +++ b/test/functional/page_objects/dashboard_page_controls.ts @@ -625,10 +625,7 @@ export class DashboardPageControls extends FtrService { attribute ); } - public async rangeSliderGetDualRangeAttribute(controlId: string, attribute: string) { - this.log.debug(`Getting range slider dual range ${attribute} for ${controlId}`); - return await this.testSubjects.getAttribute(`rangeSlider__slider`, attribute); - } + public async rangeSliderSetLowerBound(controlId: string, value: string) { this.log.debug(`Setting range slider lower bound to ${value}`); await this.testSubjects.setValue( @@ -666,8 +663,10 @@ export class DashboardPageControls extends FtrService { }); } - public async rangeSliderWaitForLoading() { - await this.testSubjects.waitForDeleted('range-slider-loading-spinner'); + public async rangeSliderWaitForLoading(controlId: string) { + await this.find.waitForDeletedByCssSelector( + `[data-test-subj="range-slider-control-${controlId}"] .euiLoadingSpinner` + ); } public async rangeSliderClearSelection(controlId: string) { diff --git a/x-pack/test/functional/apps/dashboard/group3/drilldowns/dashboard_to_dashboard_drilldown.ts b/x-pack/test/functional/apps/dashboard/group3/drilldowns/dashboard_to_dashboard_drilldown.ts index 7cd8d5c8a455d..e7afd4f9761da 100644 --- a/x-pack/test/functional/apps/dashboard/group3/drilldowns/dashboard_to_dashboard_drilldown.ts +++ b/x-pack/test/functional/apps/dashboard/group3/drilldowns/dashboard_to_dashboard_drilldown.ts @@ -309,7 +309,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.dashboardControls.optionsListOpenPopover(optionsListControl); await PageObjects.dashboardControls.optionsListPopoverSelectOption('CN'); await PageObjects.dashboardControls.optionsListPopoverSelectOption('US'); - await PageObjects.dashboardControls.rangeSliderWaitForLoading(); // wait for range slider to respond to options list selections before proceeding + await PageObjects.dashboardControls.rangeSliderWaitForLoading(rangeSliderControl); // wait for range slider to respond to options list selections before proceeding await PageObjects.dashboardControls.rangeSliderSetLowerBound(rangeSliderControl, '1000'); await PageObjects.dashboardControls.rangeSliderSetUpperBound(rangeSliderControl, '15000'); await PageObjects.dashboard.clickQuickSave(); From d4e872980d17495af413b4e4e42a24154fac151b Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Fri, 9 Jun 2023 15:25:46 -0600 Subject: [PATCH 09/22] Move debounce back to component --- .../components/range_slider_button.tsx | 13 ++++--- .../components/range_slider_control.tsx | 29 +++++++++++++-- .../components/range_slider_popover.tsx | 8 ++-- .../embeddable/range_slider_embeddable.tsx | 37 ++++--------------- 4 files changed, 45 insertions(+), 42 deletions(-) diff --git a/src/plugins/controls/public/range_slider/components/range_slider_button.tsx b/src/plugins/controls/public/range_slider/components/range_slider_button.tsx index 285e5168e6331..709738abf7db4 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_button.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_button.tsx @@ -10,13 +10,17 @@ import React from 'react'; import { EuiFieldNumber, EuiFormControlLayoutDelimited } from '@elastic/eui'; -import { useRangeSlider } from '../embeddable/range_slider_embeddable'; - import './range_slider.scss'; +import { RangeValue } from '../../../common/range_slider/types'; +import { useRangeSlider } from '../embeddable/range_slider_embeddable'; export const RangeSliderButton = ({ + value, onClick, + onChange, }: { + value: RangeValue; + onChange: (newRange: RangeValue) => void; onClick: (event: React.MouseEvent) => void; }) => { const rangeSlider = useRangeSlider(); @@ -26,7 +30,6 @@ export const RangeSliderButton = ({ const isInvalid = rangeSlider.select((state) => state.componentState.isInvalid); const id = rangeSlider.select((state) => state.explicitInput.id); - const value = rangeSlider.select((state) => state.explicitInput.value) ?? ['', '']; const isLoading = rangeSlider.select((state) => state.output.loading); @@ -43,7 +46,7 @@ export const RangeSliderButton = ({ fullWidth value={value[0] === String(min) ? '' : value[0]} onChange={(event) => { - rangeSlider.dispatch.setSelectedRange([event.target.value, value[1]]); + onChange([event.target.value, value[1]]); }} placeholder={String(min)} isInvalid={isInvalid} @@ -57,7 +60,7 @@ export const RangeSliderButton = ({ fullWidth value={value[1] === String(max) ? '' : value[1]} onChange={(event) => { - rangeSlider.dispatch.setSelectedRange([value[0], event.target.value]); + onChange([value[0], event.target.value]); }} placeholder={String(max)} isInvalid={isInvalid} diff --git a/src/plugins/controls/public/range_slider/components/range_slider_control.tsx b/src/plugins/controls/public/range_slider/components/range_slider_control.tsx index 8bf65ee036ef8..c9a3f372f460e 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_control.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_control.tsx @@ -6,16 +6,18 @@ * Side Public License, v 1. */ -import React, { FC, useState, useRef } from 'react'; +import { debounce } from 'lodash'; +import React, { FC, useState, useRef, useMemo, useEffect } from 'react'; import { EuiInputPopover } from '@elastic/eui'; import { useRangeSlider } from '../embeddable/range_slider_embeddable'; import { RangeSliderPopover, EuiDualRangeRef } from './range_slider_popover'; -import './range_slider.scss'; import { ControlError } from '../../control_group/component/control_error_component'; +import { RangeValue } from '../../../common/range_slider/types'; import { RangeSliderButton } from './range_slider_button'; +import './range_slider.scss'; export const RangeSliderControl: FC = () => { const rangeRef = useRef(null); @@ -24,9 +26,29 @@ export const RangeSliderControl: FC = () => { const rangeSlider = useRangeSlider(); const error = rangeSlider.select((state) => state.componentState.error); + const value = rangeSlider.select((state) => state.explicitInput.value); + const [displayedValue, setDisplayedValue] = useState(value ?? ['', '']); + + const debouncedOnChange = useMemo( + () => + debounce((newRange: RangeValue) => { + rangeSlider.dispatch.setSelectedRange(newRange); + }, 750), + [rangeSlider.dispatch] + ); + + useEffect(() => { + debouncedOnChange(displayedValue); + }, [debouncedOnChange, displayedValue]); + + useEffect(() => { + setDisplayedValue(value ?? ['', '']); + }, [value]); const button = ( { // the popover should remain open if the click target is one of the number inputs if (isPopoverOpen && event.target instanceof HTMLInputElement) { @@ -49,7 +71,6 @@ export const RangeSliderControl: FC = () => { anchorClassName="rangeSlider__anchorOverride" panelClassName="rangeSlider__panelOverride" closePopover={() => { - rangeSlider.publishNewRange(); setIsPopoverOpen(false); }} anchorPosition="downCenter" @@ -59,7 +80,7 @@ export const RangeSliderControl: FC = () => { rangeRef.current?.onResize(width); }} > - + ); }; diff --git a/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx b/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx index f9747dae63db5..ad55b0ded7f5b 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx @@ -22,14 +22,17 @@ import type { EuiDualRangeClass } from '@elastic/eui/src/components/form/range/d import { pluginServices } from '../../services'; import { RangeSliderStrings } from './range_slider_strings'; +import { RangeValue } from '../../../common/range_slider/types'; import { useRangeSlider } from '../embeddable/range_slider_embeddable'; // Unfortunately, wrapping EuiDualRange in `withEuiTheme` has created this annoying/verbose typing export type EuiDualRangeRef = EuiDualRangeClass & ComponentProps; export const RangeSliderPopover: FC<{ + value: RangeValue; + onChange: (newRange: RangeValue) => void; rangeRef?: Ref; -}> = ({ rangeRef }) => { +}> = ({ onChange, value, rangeRef }) => { const [fieldFormatter, setFieldFormatter] = useState(() => (toFormat: string) => toFormat); // Controls Services Context @@ -43,7 +46,6 @@ export const RangeSliderPopover: FC<{ const id = rangeSlider.select((state) => state.explicitInput.id); const title = rangeSlider.select((state) => state.explicitInput.title); - const value = rangeSlider.select((state) => state.explicitInput.value) ?? ['', '']; const min = rangeSlider.select((state) => state.componentState.min); const max = rangeSlider.select((state) => state.componentState.max); @@ -114,7 +116,7 @@ export const RangeSliderPopover: FC<{ min={rangeSliderMin} max={rangeSliderMax} onChange={([minSelection, maxSelection]) => { - rangeSlider.dispatch.setSelectedRange([String(minSelection), String(maxSelection)]); + onChange([String(minSelection), String(maxSelection)]); }} value={value} ticks={ticks} diff --git a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx index aec27164041a5..a01c182cc564e 100644 --- a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx +++ b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx @@ -105,10 +105,6 @@ export class RangeSliderEmbeddable extends Embeddable = new BehaviorSubject({}); - private cleanupStateTools: () => void; constructor( @@ -186,37 +182,18 @@ export class RangeSliderEmbeddable extends Embeddable isEqual(a.value, b.value))) - .subscribe(({ value }) => { - this.unpublishedChanges.next({ range: value ?? ['', ''] }); - }) - ); - - // debounce publishing of unpublished changes for smoother selection experience - this.subscriptions.add( - this.unpublishedChanges.pipe(debounceTime(750)).subscribe(() => { - this.publishNewRange(); - }) + .pipe( + // debounceTime(400), + distinctUntilChanged((a, b) => isEqual(a.value, b.value)), + skip(1) // skip the first input update because initial filters will be built by initialize. + ) + .subscribe(this.buildFilter) ); }; - public async publishNewRange() { - const { range } = this.unpublishedChanges.getValue(); - if (!range || this.getState().componentState.error) return; - - const [newLowerBound, newUpperBound] = range; - const updatedLowerBound = - typeof newLowerBound === 'number' ? String(newLowerBound) : newLowerBound; - const updatedUpperBound = - typeof newUpperBound === 'number' ? String(newUpperBound) : newUpperBound; - this.dispatch.setSelectedRange([updatedLowerBound, updatedUpperBound]); - await this.buildFilter(); - this.unpublishedChanges.next({}); // clear out unpublished changes - } - private getCurrentDataViewAndField = async (): Promise<{ dataView?: DataView; field?: DataViewField; From cbce8a0d4a2380af148e44e852ab4213ff57e801 Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Fri, 9 Jun 2023 15:56:30 -0600 Subject: [PATCH 10/22] Fix loading + prevent double update on reset --- .../public/range_slider/components/range_slider_control.tsx | 1 + .../range_slider/embeddable/range_slider_embeddable.tsx | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/src/plugins/controls/public/range_slider/components/range_slider_control.tsx b/src/plugins/controls/public/range_slider/components/range_slider_control.tsx index c9a3f372f460e..f905156a6c61a 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_control.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_control.tsx @@ -42,6 +42,7 @@ export const RangeSliderControl: FC = () => { }, [debouncedOnChange, displayedValue]); useEffect(() => { + // console.log('value changed'); setDisplayedValue(value ?? ['', '']); }, [value]); diff --git a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx index a01c182cc564e..97e2407be5b18 100644 --- a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx +++ b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx @@ -286,6 +286,8 @@ export class RangeSliderEmbeddable extends Embeddable { throw e; }); + + this.dispatch.setLoading(false); this.dispatch.setMinMax({ min: `${min ?? '-Infinity'}`, max: `${max ?? 'Infinity'}`, @@ -350,6 +352,9 @@ export class RangeSliderEmbeddable extends Embeddable Date: Fri, 9 Jun 2023 16:01:42 -0600 Subject: [PATCH 11/22] Cleaner fix --- .../range_slider/embeddable/range_slider_embeddable.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx index 97e2407be5b18..e856c0ebfb80d 100644 --- a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx +++ b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx @@ -142,6 +142,7 @@ export class RangeSliderEmbeddable extends Embeddable isEqual(a.value, b.value)), + distinctUntilChanged((a, b) => isEqual(a.value ?? ['', ''], b.value ?? ['', ''])), skip(1) // skip the first input update because initial filters will be built by initialize. ) .subscribe(this.buildFilter) @@ -287,7 +288,6 @@ export class RangeSliderEmbeddable extends Embeddable Date: Mon, 12 Jun 2023 09:43:12 -0600 Subject: [PATCH 12/22] Fix replace control tests --- .../components/range_slider_control.tsx | 1 - .../page_objects/dashboard_page_controls.ts | 27 +++++++++++++++---- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/plugins/controls/public/range_slider/components/range_slider_control.tsx b/src/plugins/controls/public/range_slider/components/range_slider_control.tsx index f905156a6c61a..c9a3f372f460e 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_control.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_control.tsx @@ -42,7 +42,6 @@ export const RangeSliderControl: FC = () => { }, [debouncedOnChange, displayedValue]); useEffect(() => { - // console.log('value changed'); setDisplayedValue(value ?? ['', '']); }, [value]); diff --git a/test/functional/page_objects/dashboard_page_controls.ts b/test/functional/page_objects/dashboard_page_controls.ts index 9819484f32354..f05a93b7bd339 100644 --- a/test/functional/page_objects/dashboard_page_controls.ts +++ b/test/functional/page_objects/dashboard_page_controls.ts @@ -334,11 +334,28 @@ export class DashboardPageControls extends FtrService { } public async verifyControlType(controlId: string, expectedType: string) { - const controlButton = await this.find.byXPath( - `//div[@id='controlFrame--${controlId}']//button` - ); - const testSubj = await controlButton.getAttribute('data-test-subj'); - expect(testSubj).to.equal(`${expectedType}-${controlId}`); + let controlButton; + switch (expectedType) { + case OPTIONS_LIST_CONTROL: { + controlButton = await this.find.byXPath(`//div[@id='controlFrame--${controlId}']//button`); + break; + } + case RANGE_SLIDER_CONTROL: { + controlButton = await this.find.byXPath( + `//div[@id='controlFrame--${controlId}']//div[contains(@class, 'rangeSliderAnchor__button')]` + ); + break; + } + default: { + this.log.error('An invalid control type was provided.'); + break; + } + } + + if (controlButton) { + const testSubj = await controlButton.getAttribute('data-test-subj'); + expect(testSubj).to.equal(`${expectedType}-${controlId}`); + } } // Options list functions From 4a3a8ed88f19751293f7a7b543190bfcbdeb3b53 Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Mon, 12 Jun 2023 09:49:07 -0600 Subject: [PATCH 13/22] Clean up --- .../range_slider/embeddable/range_slider_embeddable.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx index e856c0ebfb80d..d7546b9d8e2f5 100644 --- a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx +++ b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx @@ -12,8 +12,8 @@ import { isEmpty } from 'lodash'; import { batch } from 'react-redux'; import { get, isEqual } from 'lodash'; import deepEqual from 'fast-deep-equal'; -import { Subscription, lastValueFrom, BehaviorSubject } from 'rxjs'; -import { debounceTime, distinctUntilChanged, skip, map } from 'rxjs/operators'; +import { Subscription, lastValueFrom } from 'rxjs'; +import { distinctUntilChanged, skip, map } from 'rxjs/operators'; import { compareFilters, @@ -187,7 +187,6 @@ export class RangeSliderEmbeddable extends Embeddable isEqual(a.value ?? ['', ''], b.value ?? ['', ''])), skip(1) // skip the first input update because initial filters will be built by initialize. ) From 21a3c4b0328bfa97d53c6bb0c3038722c0fda654 Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Mon, 12 Jun 2023 10:35:56 -0600 Subject: [PATCH 14/22] Fix keyboard controls for popover --- .../components/range_slider_button.tsx | 26 ++++++++++++++++--- .../components/range_slider_control.tsx | 9 ++----- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/plugins/controls/public/range_slider/components/range_slider_button.tsx b/src/plugins/controls/public/range_slider/components/range_slider_button.tsx index 709738abf7db4..c19fbd9f92a59 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_button.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_button.tsx @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import React from 'react'; +import React, { useCallback } from 'react'; import { EuiFieldNumber, EuiFormControlLayoutDelimited } from '@elastic/eui'; @@ -16,12 +16,14 @@ import { useRangeSlider } from '../embeddable/range_slider_embeddable'; export const RangeSliderButton = ({ value, - onClick, onChange, + isPopoverOpen, + setIsPopoverOpen, }: { value: RangeValue; + isPopoverOpen: boolean; + setIsPopoverOpen: (open: boolean) => void; onChange: (newRange: RangeValue) => void; - onClick: (event: React.MouseEvent) => void; }) => { const rangeSlider = useRangeSlider(); @@ -33,10 +35,28 @@ export const RangeSliderButton = ({ const isLoading = rangeSlider.select((state) => state.output.loading); + const onClick = useCallback( + (event) => { + // the popover should remain open if the click/focus target is one of the number inputs + if (isPopoverOpen && event.target instanceof HTMLInputElement) { + return; + } + setIsPopoverOpen(true); + }, + [isPopoverOpen, setIsPopoverOpen] + ); + return ( { + // the popover should be closed if the next element to recieve focus **after** blur is not a number input + if (isPopoverOpen && !(event.relatedTarget instanceof HTMLInputElement)) { + setIsPopoverOpen(false); + } + }} isLoading={isLoading} className="rangeSliderAnchor__button" data-test-subj={`range-slider-control-${id}`} diff --git a/src/plugins/controls/public/range_slider/components/range_slider_control.tsx b/src/plugins/controls/public/range_slider/components/range_slider_control.tsx index c9a3f372f460e..8a9503ea48a5a 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_control.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_control.tsx @@ -49,13 +49,8 @@ export const RangeSliderControl: FC = () => { { - // the popover should remain open if the click target is one of the number inputs - if (isPopoverOpen && event.target instanceof HTMLInputElement) { - return; - } - setIsPopoverOpen(!isPopoverOpen); - }} + isPopoverOpen={isPopoverOpen} + setIsPopoverOpen={setIsPopoverOpen} /> ); From 5fcfae22562a58214cc1346b5ed1b8dc85131ccc Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Mon, 12 Jun 2023 10:56:49 -0600 Subject: [PATCH 15/22] Remove focus/blur logic --- .../public/range_slider/components/range_slider_button.tsx | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/plugins/controls/public/range_slider/components/range_slider_button.tsx b/src/plugins/controls/public/range_slider/components/range_slider_button.tsx index c19fbd9f92a59..d24f27e25979b 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_button.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_button.tsx @@ -50,13 +50,6 @@ export const RangeSliderButton = ({ { - // the popover should be closed if the next element to recieve focus **after** blur is not a number input - if (isPopoverOpen && !(event.relatedTarget instanceof HTMLInputElement)) { - setIsPopoverOpen(false); - } - }} isLoading={isLoading} className="rangeSliderAnchor__button" data-test-subj={`range-slider-control-${id}`} From dd2e4b6145555b44f2bca0c1dfe847c7752402df Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Mon, 12 Jun 2023 13:12:35 -0600 Subject: [PATCH 16/22] Fix close popover for range slider --- test/functional/page_objects/dashboard_page_controls.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/functional/page_objects/dashboard_page_controls.ts b/test/functional/page_objects/dashboard_page_controls.ts index f05a93b7bd339..41435c5b677c3 100644 --- a/test/functional/page_objects/dashboard_page_controls.ts +++ b/test/functional/page_objects/dashboard_page_controls.ts @@ -668,7 +668,8 @@ export class DashboardPageControls extends FtrService { public async rangeSliderEnsurePopoverIsClosed(controlId: string) { this.log.debug(`Opening popover for Range Slider: ${controlId}`); - await this.testSubjects.click(`range-slider-control-${controlId}`); + const controlLabel = await this.find.byXPath(`//div[@data-control-id='${controlId}']//label`); + await controlLabel.click(); await this.testSubjects.waitForDeleted(`rangeSlider-control-actions`); } From 297da5ab99e8938c3fdc7dd97962f521d4463218 Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Mon, 12 Jun 2023 16:09:01 -0600 Subject: [PATCH 17/22] Fix reload infinite loading bug --- .../range_slider/embeddable/range_slider_embeddable.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx index d7546b9d8e2f5..991bba2510d75 100644 --- a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx +++ b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx @@ -437,10 +437,13 @@ export class RangeSliderEmbeddable extends Embeddable { - this.runRangeSliderQuery().catch((e) => { + public reload = async () => { + try { + await this.runRangeSliderQuery(); + await this.buildFilter(); + } catch (e) { this.dispatch.setErrorMessage(e.message); - }); + } }; public destroy = () => { From 0ef557dc7cc398e2eb14a9bd7de15b7a2d035f41 Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Wed, 14 Jun 2023 12:01:36 -0600 Subject: [PATCH 18/22] Fix error state when there is no data --- .../range_slider/components/range_slider_popover.tsx | 12 +++++++----- .../embeddable/range_slider_embeddable.tsx | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx b/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx index ad55b0ded7f5b..cbf349a5b28fa 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx @@ -75,6 +75,8 @@ export const RangeSliderPopover: FC<{ ); }); + console.log(rangeSliderMin, rangeSliderMax); + // derive field formatter from fieldSpec and dataViewId useEffect(() => { (async () => { @@ -90,6 +92,7 @@ export const RangeSliderPopover: FC<{ }, [fieldSpec, dataViewId, getDataViewById]); const ticks = useMemo(() => { + console.log(min, max); return [ { value: min, label: fieldFormatter(String(min)) }, { value: max, label: fieldFormatter(String(max)) }, @@ -126,14 +129,13 @@ export const RangeSliderPopover: FC<{ ref={rangeRef} data-test-subj="rangeSlider__slider" /> - ) : ( + ) : isInvalid ? ( - {RangeSliderStrings.popover.getNoAvailableDataHelpText()} + {RangeSliderStrings.popover.getNoDataHelpText()} - )} - {isInvalid && ( + ) : ( - {RangeSliderStrings.popover.getNoDataHelpText()} + {RangeSliderStrings.popover.getNoAvailableDataHelpText()} )} diff --git a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx index 991bba2510d75..4ea8bc0d71222 100644 --- a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx +++ b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx @@ -353,7 +353,7 @@ export class RangeSliderEmbeddable extends Embeddable Date: Wed, 14 Jun 2023 12:18:27 -0600 Subject: [PATCH 19/22] Clean up logs --- .../public/range_slider/components/range_slider_popover.tsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx b/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx index cbf349a5b28fa..65d61a467f309 100644 --- a/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx +++ b/src/plugins/controls/public/range_slider/components/range_slider_popover.tsx @@ -75,8 +75,6 @@ export const RangeSliderPopover: FC<{ ); }); - console.log(rangeSliderMin, rangeSliderMax); - // derive field formatter from fieldSpec and dataViewId useEffect(() => { (async () => { @@ -92,7 +90,6 @@ export const RangeSliderPopover: FC<{ }, [fieldSpec, dataViewId, getDataViewById]); const ticks = useMemo(() => { - console.log(min, max); return [ { value: min, label: fieldFormatter(String(min)) }, { value: max, label: fieldFormatter(String(max)) }, From ae4638dc8a473708bd363d140583fb55ac3a8b34 Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Wed, 14 Jun 2023 15:03:17 -0600 Subject: [PATCH 20/22] Better fix for error state --- .../embeddable/range_slider_embeddable.tsx | 83 ++++++++++--------- 1 file changed, 44 insertions(+), 39 deletions(-) diff --git a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx index 4ea8bc0d71222..7b68f513238a7 100644 --- a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx +++ b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx @@ -20,8 +20,6 @@ import { buildRangeFilter, COMPARE_ALL_OPTIONS, RangeFilterParams, - Filter, - Query, } from '@kbn/es-query'; import { i18n } from '@kbn/i18n'; import { KibanaThemeProvider } from '@kbn/kibana-react-plugin/public'; @@ -173,7 +171,7 @@ export class RangeSliderEmbeddable extends Embeddable { + dataFetchPipe.subscribe(async (changes) => { try { await this.runRangeSliderQuery(); await this.buildFilter(); @@ -241,15 +239,7 @@ export class RangeSliderEmbeddable extends Embeddable { @@ -259,30 +249,9 @@ export class RangeSliderEmbeddable extends Embeddable { throw e; }); @@ -296,18 +265,37 @@ export class RangeSliderEmbeddable extends Embeddable => { const searchSource = await this.dataService.searchSource.create(); searchSource.setField('size', 0); searchSource.setField('index', dataView); + const embeddableInput = this.getInput(); + const { ignoreParentSettings, query, timeRange: globalTimeRange, timeslice } = embeddableInput; + let { filters = [] } = embeddableInput; + + const timeRange = + timeslice !== undefined + ? { + from: new Date(timeslice[0]).toISOString(), + to: new Date(timeslice[1]).toISOString(), + mode: 'absolute' as 'absolute', + } + : globalTimeRange; + if (!ignoreParentSettings?.ignoreTimerange && timeRange) { + const timeFilter = this.dataService.timefilter.createFilter(dataView, timeRange); + if (timeFilter) { + filters = filters.concat(timeFilter); + } + } + + if (ignoreParentSettings?.ignoreFilters) { + filters = []; + } + searchSource.setField('filter', filters); if (query) { @@ -349,15 +337,32 @@ export class RangeSliderEmbeddable extends Embeddable { const { componentState: { min: availableMin, max: availableMax }, - explicitInput: { query, timeRange, filters = [], ignoreParentSettings, value }, + explicitInput: { value }, } = this.getState(); + const embeddableInput = this.getInput(); + const { + ignoreParentSettings, + filters = [], + query, + timeRange: globalTimeRange, + timeslice, + } = embeddableInput; + const [selectedMin, selectedMax] = value ?? ['', '']; - const hasData = !Boolean(availableMin) && !Boolean(availableMax); + const hasData = availableMin !== undefined && availableMax !== undefined; const hasLowerSelection = !isEmpty(selectedMin); const hasUpperSelection = !isEmpty(selectedMax); const hasEitherSelection = hasLowerSelection || hasUpperSelection; + const timeRange = + timeslice !== undefined + ? { + from: new Date(timeslice[0]).toISOString(), + to: new Date(timeslice[1]).toISOString(), + mode: 'absolute' as 'absolute', + } + : globalTimeRange; const { dataView, field } = await this.getCurrentDataViewAndField(); if (!dataView || !field) return; From 6ac785af662b296bfb02564ca61cb93805095c15 Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Wed, 14 Jun 2023 15:26:15 -0600 Subject: [PATCH 21/22] Cleaner implementation --- .../embeddable/range_slider_embeddable.tsx | 31 +++---------------- 1 file changed, 5 insertions(+), 26 deletions(-) diff --git a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx index 7b68f513238a7..7f2aaa60f9e10 100644 --- a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx +++ b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx @@ -20,6 +20,7 @@ import { buildRangeFilter, COMPARE_ALL_OPTIONS, RangeFilterParams, + Filter, } from '@kbn/es-query'; import { i18n } from '@kbn/i18n'; import { KibanaThemeProvider } from '@kbn/kibana-react-plugin/public'; @@ -96,6 +97,7 @@ export class RangeSliderEmbeddable extends Embeddable Date: Wed, 14 Jun 2023 15:37:18 -0600 Subject: [PATCH 22/22] Clean up more --- .../embeddable/range_slider_embeddable.tsx | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx index 7f2aaa60f9e10..ad0c2aae0eaf3 100644 --- a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx +++ b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.tsx @@ -251,6 +251,26 @@ export class RangeSliderEmbeddable extends Embeddable