From b73d939d6cef5431e55f8d851c9afcb9169bda6a Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Thu, 30 Sep 2021 18:03:40 +0200 Subject: [PATCH] [Lens] refactor - move debounce one layer up for thresholds (remove updater) (#113222) * [Lens] remove updater from vis * removing blur handle * fix * blur fix Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../editor_frame/state_helpers.ts | 4 +- .../pie_visualization/render_function.tsx | 6 +- .../state_management/init_middleware/index.ts | 6 +- .../state_management/optimizing_middleware.ts | 4 +- .../lens/public/state_management/selectors.ts | 6 +- .../public/xy_visualization/expression.tsx | 10 +-- .../xy_config_panel/threshold_panel.tsx | 79 +++++++++++-------- 7 files changed, 61 insertions(+), 54 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_helpers.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_helpers.ts index 5e059a1e2e8b..a09cf269f753 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_helpers.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_helpers.ts @@ -57,7 +57,7 @@ export async function initializeDatasources( return states; } -export const createDatasourceLayers = memoizeOne(function createDatasourceLayers( +export const getDatasourceLayers = memoizeOne(function getDatasourceLayers( datasourceStates: DatasourceStates, datasourceMap: DatasourceMap ) { @@ -111,7 +111,7 @@ export async function persistedStateToExpression( { isFullEditor: false } ); - const datasourceLayers = createDatasourceLayers(datasourceStates, datasourceMap); + const datasourceLayers = getDatasourceLayers(datasourceStates, datasourceMap); const datasourceId = getActiveDatasourceIdFromDoc(doc); if (datasourceId == null) { diff --git a/x-pack/plugins/lens/public/pie_visualization/render_function.tsx b/x-pack/plugins/lens/public/pie_visualization/render_function.tsx index e79066b0145d..8babb11c856b 100644 --- a/x-pack/plugins/lens/public/pie_visualization/render_function.tsx +++ b/x-pack/plugins/lens/public/pie_visualization/render_function.tsx @@ -215,11 +215,11 @@ export function PieComponent( }, }); - const [state, setState] = useState({ isReady: false }); + const [isReady, setIsReady] = useState(false); // It takes a cycle for the chart to render. This prevents // reporting from printing a blank chart placeholder. useEffect(() => { - setState({ isReady: true }); + setIsReady(true); }, []); const hasNegative = firstTable.rows.some((row) => { @@ -273,7 +273,7 @@ export function PieComponent( reportTitle={props.args.title} reportDescription={props.args.description} className="lnsPieExpression__container" - isReady={state.isReady} + isReady={isReady} > (store: MiddlewareAP store.dispatch ); return (next: Dispatch) => (action: PayloadAction) => { - if (lensSlice.actions.loadInitial.match(action)) { + if (loadInitialAction.match(action)) { return loadInitial(store, storeDeps, action.payload); - } else if (lensSlice.actions.navigateAway.match(action)) { + } else if (navigateAway.match(action)) { return unsubscribeFromExternalContext(); } next(action); diff --git a/x-pack/plugins/lens/public/state_management/optimizing_middleware.ts b/x-pack/plugins/lens/public/state_management/optimizing_middleware.ts index f1293a84255b..610be69166ba 100644 --- a/x-pack/plugins/lens/public/state_management/optimizing_middleware.ts +++ b/x-pack/plugins/lens/public/state_management/optimizing_middleware.ts @@ -7,12 +7,12 @@ import { Dispatch, MiddlewareAPI, Action } from '@reduxjs/toolkit'; import { isEqual } from 'lodash'; -import { lensSlice } from './lens_slice'; +import { onActiveDataChange } from '.'; /** cancels updates to the store that don't change the state */ export const optimizingMiddleware = () => (store: MiddlewareAPI) => { return (next: Dispatch) => (action: Action) => { - if (lensSlice.actions.onActiveDataChange.match(action)) { + if (onActiveDataChange.match(action)) { if (isEqual(store.getState().lens.activeData, action.payload)) { return; } diff --git a/x-pack/plugins/lens/public/state_management/selectors.ts b/x-pack/plugins/lens/public/state_management/selectors.ts index 360ca48c2d27..c1d1700d8b3b 100644 --- a/x-pack/plugins/lens/public/state_management/selectors.ts +++ b/x-pack/plugins/lens/public/state_management/selectors.ts @@ -10,7 +10,7 @@ import { SavedObjectReference } from 'kibana/server'; import { LensState } from './types'; import { extractFilterReferences } from '../persistence'; import { Datasource, DatasourceMap, VisualizationMap } from '../types'; -import { createDatasourceLayers } from '../editor_frame_service/editor_frame'; +import { getDatasourceLayers } from '../editor_frame_service/editor_frame'; export const selectPersistedDoc = (state: LensState) => state.lens.persistedDoc; export const selectQuery = (state: LensState) => state.lens.query; @@ -141,13 +141,13 @@ export const selectAreDatasourcesLoaded = createSelector( export const selectDatasourceLayers = createSelector( [selectDatasourceStates, selectDatasourceMap], - (datasourceStates, datasourceMap) => createDatasourceLayers(datasourceStates, datasourceMap) + (datasourceStates, datasourceMap) => getDatasourceLayers(datasourceStates, datasourceMap) ); export const selectFramePublicAPI = createSelector( [selectDatasourceStates, selectActiveData, selectDatasourceMap], (datasourceStates, activeData, datasourceMap) => ({ - datasourceLayers: createDatasourceLayers(datasourceStates, datasourceMap), + datasourceLayers: getDatasourceLayers(datasourceStates, datasourceMap), activeData, }) ); diff --git a/x-pack/plugins/lens/public/xy_visualization/expression.tsx b/x-pack/plugins/lens/public/xy_visualization/expression.tsx index 863289c31bba..bc80b4569977 100644 --- a/x-pack/plugins/lens/public/xy_visualization/expression.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/expression.tsx @@ -191,20 +191,18 @@ function getIconForSeriesType(seriesType: SeriesType): IconType { const MemoizedChart = React.memo(XYChart); export function XYChartReportable(props: XYChartRenderProps) { - const [state, setState] = useState({ - isReady: false, - }); + const [isReady, setIsReady] = useState(false); // It takes a cycle for the XY chart to render. This prevents // reporting from printing a blank chart placeholder. useEffect(() => { - setState({ isReady: true }); - }, [setState]); + setIsReady(true); + }, [setIsReady]); return ( diff --git a/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/threshold_panel.tsx b/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/threshold_panel.tsx index 1e5b90e41b62..087eee9005c0 100644 --- a/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/threshold_panel.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/threshold_panel.tsx @@ -6,12 +6,12 @@ */ import './xy_config_panel.scss'; -import React, { useCallback } from 'react'; +import React, { useCallback, useState } from 'react'; import { i18n } from '@kbn/i18n'; import { EuiButtonGroup, EuiComboBox, EuiFormRow, EuiIcon, EuiRange } from '@elastic/eui'; import type { PaletteRegistry } from 'src/plugins/charts/public'; import type { VisualizationDimensionEditorProps } from '../../types'; -import { State } from '../types'; +import { State, XYState } from '../types'; import { FormatFactory } from '../../../common'; import { YConfig } from '../../../common/expressions'; import { LineStyle, FillStyle } from '../../../common/expressions/xy_chart'; @@ -116,29 +116,35 @@ export const ThresholdPanel = ( } ) => { const { state, setState, layerId, accessor } = props; - const index = state.layers.findIndex((l) => l.layerId === layerId); - const layer = state.layers[index]; + + const { inputValue: localState, handleInputChange: setLocalState } = useDebouncedValue({ + value: state, + onChange: setState, + }); + + const index = localState.layers.findIndex((l) => l.layerId === layerId); + const layer = localState.layers[index]; const setYConfig = useCallback( (yConfig: Partial | undefined) => { if (yConfig == null) { return; } - setState((currState) => { - const currLayer = currState.layers[index]; - const newYConfigs = [...(currLayer.yConfig || [])]; - const existingIndex = newYConfigs.findIndex( - (yAxisConfig) => yAxisConfig.forAccessor === accessor - ); - if (existingIndex !== -1) { - newYConfigs[existingIndex] = { ...newYConfigs[existingIndex], ...yConfig }; - } else { - newYConfigs.push({ forAccessor: accessor, ...yConfig }); - } - return updateLayer(currState, { ...currLayer, yConfig: newYConfigs }, index); - }); + const newYConfigs = [...(layer.yConfig || [])]; + const existingIndex = newYConfigs.findIndex( + (yAxisConfig) => yAxisConfig.forAccessor === accessor + ); + if (existingIndex !== -1) { + newYConfigs[existingIndex] = { ...newYConfigs[existingIndex], ...yConfig }; + } else { + newYConfigs.push({ + forAccessor: accessor, + ...yConfig, + }); + } + setLocalState(updateLayer(localState, { ...layer, yConfig: newYConfigs }, index)); }, - [accessor, index, setState] + [accessor, index, localState, layer, setLocalState] ); const currentYConfig = layer.yConfig?.find((yConfig) => yConfig.forAccessor === accessor); @@ -291,35 +297,38 @@ const LineThicknessSlider = ({ value: number; onChange: (value: number) => void; }) => { - const onChangeWrapped = useCallback( - (newValue) => { - if (Number.isInteger(newValue)) { - onChange(getSafeValue(newValue, newValue, minRange, maxRange)); - } - }, - [onChange] - ); - const { inputValue, handleInputChange } = useDebouncedValue( - { value, onChange: onChangeWrapped }, - { allowFalsyValue: true } - ); + const [unsafeValue, setUnsafeValue] = useState(String(value)); return ( { - const newValue = e.currentTarget.value; - handleInputChange(newValue === '' ? '' : Number(newValue)); + onChange={({ currentTarget: { value: newValue } }) => { + setUnsafeValue(newValue); + const convertedValue = newValue === '' ? '' : Number(newValue); + const safeValue = getSafeValue(Number(newValue), Number(newValue), minRange, maxRange); + // only update onChange is the value is valid and in range + if (convertedValue === safeValue) { + onChange(safeValue); + } }} onBlur={() => { - handleInputChange(getSafeValue(inputValue, value, minRange, maxRange)); + if (unsafeValue !== String(value)) { + const safeValue = getSafeValue( + unsafeValue === '' ? unsafeValue : Number(unsafeValue), + value, + minRange, + maxRange + ); + onChange(safeValue); + setUnsafeValue(String(safeValue)); + } }} /> );