diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx index 3a299de0fca6a3..f1bf5372d12891 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx @@ -70,6 +70,9 @@ export function LayerPanel( updateDatasource, toggleFullscreen, isFullscreen, + updateAll, + updateDatasourceAsync, + visualizationState, } = props; const datasourcePublicAPI = framePublicAPI.datasourceLayers[layerId]; @@ -114,7 +117,11 @@ export function LayerPanel( activeData: props.framePublicAPI.activeData, }; - const { groups } = activeVisualization.getConfiguration(layerVisualizationConfigProps); + const { groups } = useMemo( + () => activeVisualization.getConfiguration(layerVisualizationConfigProps), + // eslint-disable-next-line react-hooks/exhaustive-deps + [layerDatasourceConfigProps.frame, layerDatasourceDropProps, activeVisualization] + ); const isEmptyLayer = !groups.some((d) => d.accessors.length > 0); const { activeId, activeGroup } = activeDimension; @@ -204,6 +211,58 @@ export function LayerPanel( const isDimensionPanelOpen = Boolean(activeId); + const updateDataLayerState = useCallback( + (newState: unknown, { isDimensionComplete = true }: { isDimensionComplete?: boolean } = {}) => { + if (!activeGroup || !activeId) { + return; + } + if (allAccessors.includes(activeId)) { + if (isDimensionComplete) { + updateDatasourceAsync(datasourceId, newState); + } else { + // The datasource can indicate that the previously-valid column is no longer + // complete, which clears the visualization. This keeps the flyout open and reuses + // the previous columnId + updateAll( + datasourceId, + newState, + activeVisualization.removeDimension({ + layerId, + columnId: activeId, + prevState: visualizationState, + }) + ); + } + } else if (isDimensionComplete) { + updateAll( + datasourceId, + newState, + activeVisualization.setDimension({ + layerId, + groupId: activeGroup.groupId, + columnId: activeId, + prevState: visualizationState, + }) + ); + setActiveDimension({ ...activeDimension, isNew: false }); + } else { + updateDatasourceAsync(datasourceId, newState); + } + }, + // eslint-disable-next-line react-hooks/exhaustive-deps + [ + activeDimension, + activeGroup, + activeId, + activeVisualization, + datasourceId, + layerId, + updateAll, + updateDatasourceAsync, + visualizationState, + ] + ); + return ( <>
{ - if (allAccessors.includes(activeId)) { - if (isDimensionComplete) { - props.updateDatasourceAsync(datasourceId, newState); - } else { - // The datasource can indicate that the previously-valid column is no longer - // complete, which clears the visualization. This keeps the flyout open and reuses - // the previous columnId - props.updateAll( - datasourceId, - newState, - activeVisualization.removeDimension({ - layerId, - columnId: activeId, - prevState: props.visualizationState, - }) - ); - } - } else if (isDimensionComplete) { - props.updateAll( - datasourceId, - newState, - activeVisualization.setDimension({ - layerId, - groupId: activeGroup.groupId, - columnId: activeId, - prevState: props.visualizationState, - }) - ); - setActiveDimension({ ...activeDimension, isNew: false }); - } else { - props.updateDatasourceAsync(datasourceId, newState); - } - }, + setState: updateDataLayerState, }} /> )} diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx index 351b4009240ebb..52488cb32ae837 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx @@ -570,9 +570,8 @@ describe('editor_frame', () => { setDatasourceState(updatedState); }); - // TODO: temporary regression // validation requires to calls this getConfiguration API - expect(mockVisualization.getConfiguration).toHaveBeenCalledTimes(9); + expect(mockVisualization.getConfiguration).toHaveBeenCalledTimes(6); expect(mockVisualization.getConfiguration).toHaveBeenLastCalledWith( expect.objectContaining({ state: updatedState, @@ -645,9 +644,8 @@ describe('editor_frame', () => { setDatasourceState({}); }); - // TODO: temporary regression, selectors will help // validation requires to calls this getConfiguration API - expect(mockVisualization.getConfiguration).toHaveBeenCalledTimes(9); + expect(mockVisualization.getConfiguration).toHaveBeenCalledTimes(6); expect(mockVisualization.getConfiguration).toHaveBeenLastCalledWith( expect.objectContaining({ frame: expect.objectContaining({ @@ -1124,8 +1122,7 @@ describe('editor_frame', () => { }); // validation requires to calls this getConfiguration API - // TODO: why so many times? - expect(mockVisualization.getConfiguration).toHaveBeenCalledTimes(10); + expect(mockVisualization.getConfiguration).toHaveBeenCalledTimes(7); expect(mockVisualization.getConfiguration).toHaveBeenCalledWith( expect.objectContaining({ state: suggestionVisState, diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx index 161b0125a172a3..cc65bb126d2d9e 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React, { useEffect, useReducer, useState, useCallback, useRef } from 'react'; +import React, { useEffect, useReducer, useState, useCallback, useRef, useMemo } from 'react'; import { CoreStart } from 'kibana/public'; import { isEqual } from 'lodash'; import { PaletteRegistry } from 'src/plugins/charts/public'; @@ -118,56 +118,74 @@ export function EditorFrame(props: EditorFrameProps) { ); const datasourceLayers = createDatasourceLayers(props.datasourceMap, state.datasourceStates); - const framePublicAPI: FramePublicAPI = { - datasourceLayers, - activeData, - dateRange, - query, - filters, - searchSessionId, - availablePalettes: props.palettes, - - addNewLayer() { - const newLayerId = generateId(); - - dispatch({ - type: 'UPDATE_LAYER', - datasourceId: state.activeDatasourceId!, - layerId: newLayerId, - updater: props.datasourceMap[state.activeDatasourceId!].insertLayer, - }); + const framePublicAPI: FramePublicAPI = useMemo( + () => ({ + datasourceLayers, + activeData, + dateRange, + query, + filters, + searchSessionId, + availablePalettes: props.palettes, - return newLayerId; - }, + addNewLayer() { + const newLayerId = generateId(); - removeLayers(layerIds: string[]) { - if (activeVisualization && activeVisualization.removeLayer && state.visualization.state) { dispatch({ - type: 'UPDATE_VISUALIZATION_STATE', - visualizationId: activeVisualization.id, - updater: layerIds.reduce( - (acc, layerId) => - activeVisualization.removeLayer ? activeVisualization.removeLayer(acc, layerId) : acc, - state.visualization.state - ), + type: 'UPDATE_LAYER', + datasourceId: state.activeDatasourceId!, + layerId: newLayerId, + updater: props.datasourceMap[state.activeDatasourceId!].insertLayer, }); - } - layerIds.forEach((layerId) => { - const layerDatasourceId = Object.entries(props.datasourceMap).find( - ([datasourceId, datasource]) => - state.datasourceStates[datasourceId] && - datasource.getLayers(state.datasourceStates[datasourceId].state).includes(layerId) - )![0]; - dispatch({ - type: 'UPDATE_LAYER', - layerId, - datasourceId: layerDatasourceId, - updater: props.datasourceMap[layerDatasourceId].removeLayer, + return newLayerId; + }, + + removeLayers(layerIds: string[]) { + if (activeVisualization && activeVisualization.removeLayer && state.visualization.state) { + dispatch({ + type: 'UPDATE_VISUALIZATION_STATE', + visualizationId: activeVisualization.id, + updater: layerIds.reduce( + (acc, layerId) => + activeVisualization.removeLayer + ? activeVisualization.removeLayer(acc, layerId) + : acc, + state.visualization.state + ), + }); + } + + layerIds.forEach((layerId) => { + const layerDatasourceId = Object.entries(props.datasourceMap).find( + ([datasourceId, datasource]) => + state.datasourceStates[datasourceId] && + datasource.getLayers(state.datasourceStates[datasourceId].state).includes(layerId) + )![0]; + dispatch({ + type: 'UPDATE_LAYER', + layerId, + datasourceId: layerDatasourceId, + updater: props.datasourceMap[layerDatasourceId].removeLayer, + }); }); - }); - }, - }; + }, + }), + [ + activeData, + activeVisualization, + datasourceLayers, + dateRange, + query, + filters, + searchSessionId, + props.palettes, + props.datasourceMap, + state.activeDatasourceId, + state.datasourceStates, + state.visualization.state, + ] + ); useEffect( () => { 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 e510e88bd2cf82..dffb0e75f2109b 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 @@ -7,6 +7,7 @@ import { SavedObjectReference } from 'kibana/public'; import { Ast } from '@kbn/interpreter/common'; +import memoizeOne from 'memoize-one'; import { Datasource, DatasourcePublicAPI, @@ -53,7 +54,7 @@ export async function initializeDatasources( return states; } -export function createDatasourceLayers( +export const createDatasourceLayers = memoizeOne(function createDatasourceLayers( datasourceMap: Record, datasourceStates: Record ) { @@ -73,7 +74,7 @@ export function createDatasourceLayers( }); }); return datasourceLayers; -} +}); export async function persistedStateToExpression( datasources: Record, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx index c4df3c17db3b7b..74b47b55fdd0e9 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx @@ -109,6 +109,11 @@ export function DimensionEditor(props: DimensionEditorProps) { const selectedOperationDefinition = selectedColumn && operationDefinitionMap[selectedColumn.operationType]; + const updateLayer = useCallback( + (newLayer) => setState((prevState) => mergeLayer({ state: prevState, layerId, newLayer })), + [layerId, setState] + ); + const setStateWrapper = ( setter: IndexPatternLayer | ((prevLayer: IndexPatternLayer) => IndexPatternLayer) ) => { @@ -163,7 +168,7 @@ export function DimensionEditor(props: DimensionEditorProps) { // Operations are compatible if they match inputs. They are always compatible in // the empty state. Field-based operations are not compatible with field-less operations. - const operationsWithCompatibility = [...possibleOperations].map((operationType) => { + const operationsWithCompatibility = possibleOperations.map((operationType) => { const definition = operationDefinitionMap[operationType]; const currentField = @@ -408,13 +413,8 @@ export function DimensionEditor(props: DimensionEditorProps) { | IndexPatternLayer | ((prevLayer: IndexPatternLayer) => IndexPatternLayer) ) => { - setState( - mergeLayer({ - state, - layerId, - newLayer: - typeof setter === 'function' ? setter(state.layers[layerId]) : setter, - }) + updateLayer( + typeof setter === 'function' ? setter(state.layers[layerId]) : setter ); }} validation={validation} @@ -629,20 +629,16 @@ export function DimensionEditor(props: DimensionEditorProps) { const onFormatChange = useCallback( (newFormat) => { - setState( - mergeLayer({ - state, - layerId, - newLayer: updateColumnParam({ - layer: state.layers[layerId], - columnId, - paramName: 'format', - value: newFormat, - }), + updateLayer( + updateColumnParam({ + layer: state.layers[layerId], + columnId, + paramName: 'format', + value: newFormat, }) ); }, - [columnId, layerId, setState, state] + [columnId, layerId, state.layers, updateLayer] ); return ( @@ -702,27 +698,21 @@ export function DimensionEditor(props: DimensionEditorProps) { { - setState( - mergeLayer({ - state, - layerId, - newLayer: { - columns: { - ...state.layers[layerId].columns, - [columnId]: { - ...selectedColumn, - label: value, - customLabel: - operationDefinitionMap[selectedColumn.operationType].getDefaultLabel( - selectedColumn, - state.indexPatterns[state.layers[layerId].indexPatternId], - state.layers[layerId].columns - ) !== value, - }, - }, + updateLayer({ + columns: { + ...state.layers[layerId].columns, + [columnId]: { + ...selectedColumn, + label: value, + customLabel: + operationDefinitionMap[selectedColumn.operationType].getDefaultLabel( + selectedColumn, + state.indexPatterns[state.layers[layerId].indexPatternId], + state.layers[layerId].columns + ) !== value, }, - }) - ); + }, + }); }} /> )} @@ -731,9 +721,7 @@ export function DimensionEditor(props: DimensionEditorProps) { - setState(mergeLayer({ state, layerId, newLayer: { columnOrder } })) - } + setColumns={(columnOrder) => updateLayer({ columnOrder })} getFieldByName={currentIndexPattern.getFieldByName} /> )} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx index 7e45b295215631..afcecdf5be9b8a 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx @@ -606,7 +606,8 @@ describe('IndexPatternDimensionEditorPanel', () => { .simulate('change', { target: { value: 'New Label' } }); }); - expect(setState).toHaveBeenCalledWith({ + expect(setState.mock.calls[0]).toEqual([expect.any(Function)]); + expect(setState.mock.calls[0][0](state)).toEqual({ ...state, layers: { first: { @@ -709,7 +710,8 @@ describe('IndexPatternDimensionEditorPanel', () => { .simulate('change', { target: { value: 'Maximum of bytes' } }); }); - expect(setState).toHaveBeenCalledWith({ + expect(setState.mock.calls[0]).toEqual([expect.any(Function)]); + expect(setState.mock.calls[0][0](state)).toEqual({ ...state, layers: { first: { @@ -1996,7 +1998,8 @@ describe('IndexPatternDimensionEditorPanel', () => { .prop('onChange')!([{ value: 'bytes', label: 'Bytes' }]); }); - expect(setState).toHaveBeenCalledWith({ + expect(setState.mock.calls[0]).toEqual([expect.any(Function)]); + expect(setState.mock.calls[0][0](state)).toEqual({ ...state, layers: { first: { @@ -2080,7 +2083,8 @@ describe('IndexPatternDimensionEditorPanel', () => { .prop('onChange')!({ currentTarget: { value: '0' } }); }); - expect(setState).toHaveBeenCalledWith({ + expect(setState.mock.calls[0]).toEqual([expect.any(Function)]); + expect(setState.mock.calls[0][0](state)).toEqual({ ...state, layers: { first: {