diff --git a/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/metric_vis.tsx b/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/metric_vis.tsx index 752a6b8369927..f1870105a26d2 100644 --- a/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/metric_vis.tsx +++ b/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/metric_vis.tsx @@ -274,6 +274,8 @@ export const MetricVis = ({ const minHeight = chartBaseTheme.metric.minHeight; + // this needs to use a useEffect as it depends on the scrollContainerRef + // which is not available on the first render useEffect(() => { const minimumRequiredVerticalSpace = minHeight * numRows; setScrollChildHeight( diff --git a/x-pack/platform/plugins/shared/lens/public/app_plugin/app.tsx b/x-pack/platform/plugins/shared/lens/public/app_plugin/app.tsx index 9646d3bf12c81..0b218239bb18e 100644 --- a/x-pack/platform/plugins/shared/lens/public/app_plugin/app.tsx +++ b/x-pack/platform/plugins/shared/lens/public/app_plugin/app.tsx @@ -137,10 +137,12 @@ export function App({ // Used to show a popover that guides the user towards changing the date range when no data is available. const [indicateNoData, setIndicateNoData] = useState(false); const [isSaveModalVisible, setIsSaveModalVisible] = useState(false); - const [lastKnownDoc, setLastKnownDoc] = useState(undefined); - const [initialDocFromContext, setInitialDocFromContext] = useState( - undefined - ); + // keeping the initial doc state created by the context + const initialDocFromContextRef = useRef(undefined); + if (!initialDocFromContextRef.current && currentDoc) { + initialDocFromContextRef.current = currentDoc; + } + const [shouldCloseAndSaveTextBasedQuery, setShouldCloseAndSaveTextBasedQuery] = useState(false); const savedObjectId = initialInput?.savedObjectId; @@ -155,12 +157,6 @@ export function App({ : undefined; const initialContextIsEmbedded = Boolean(legacyEditorAppName); - useEffect(() => { - if (currentDoc) { - setLastKnownDoc(currentDoc); - } - }, [currentDoc]); - const showNoDataPopover = useCallback(() => { setIndicateNoData(true); }, [setIndicateNoData]); @@ -187,14 +183,14 @@ export function App({ (refDoc: LensDocument | undefined) => { return isLensEqual( refDoc, - lastKnownDoc, + currentDoc, data.query.filterManager.inject.bind(data.query.filterManager), datasourceMap, visualizationMap, annotationGroups ); }, - [annotationGroups, data.query.filterManager, datasourceMap, lastKnownDoc, visualizationMap] + [annotationGroups, data.query.filterManager, datasourceMap, currentDoc, visualizationMap] ); useEffect(() => { @@ -223,7 +219,7 @@ export function App({ }); }, [ onAppLeave, - lastKnownDoc, + currentDoc, isSaveable, persistedDoc, application.capabilities.visualize_v2.save, @@ -308,7 +304,7 @@ export function App({ try { const newState = await runSaveLensVisualization( { - lastKnownDoc, + lastKnownDoc: currentDoc, savedObjectsTagging, initialInput, redirectToOrigin, @@ -338,7 +334,7 @@ export function App({ visualization.state, activeVisualization, dispatch, - lastKnownDoc, + currentDoc, savedObjectsTagging, initialInput, redirectToOrigin, @@ -353,13 +349,6 @@ export function App({ ] ); - // keeping the initial doc state created by the context - useEffect(() => { - if (lastKnownDoc && !initialDocFromContext) { - setInitialDocFromContext(lastKnownDoc); - } - }, [lastKnownDoc, initialDocFromContext]); - const { shouldShowGoBackToVizEditorModal, goBackToOriginatingApp, @@ -370,7 +359,7 @@ export function App({ onAppLeave, legacyEditorAppName, legacyEditorAppUrl, - initialDocFromContext, + initialDocFromContext: initialDocFromContextRef.current, persistedDoc, isLensEqual: isLensEqualWrapper, }); @@ -499,7 +488,7 @@ export function App({ setIsSaveModalVisible(false); }} getAppNameFromId={() => getOriginatingAppName()} - lastKnownDoc={lastKnownDoc} + lastKnownDoc={currentDoc} onAppLeave={onAppLeave} persistedDoc={persistedDoc} initialInput={initialInput} diff --git a/x-pack/platform/plugins/shared/lens/public/app_plugin/get_application_user_messages.tsx b/x-pack/platform/plugins/shared/lens/public/app_plugin/get_application_user_messages.tsx index 373dcfb93d8d5..f46b1387949ce 100644 --- a/x-pack/platform/plugins/shared/lens/public/app_plugin/get_application_user_messages.tsx +++ b/x-pack/platform/plugins/shared/lens/public/app_plugin/get_application_user_messages.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React, { useEffect, useState } from 'react'; +import React, { useCallback, useMemo, useState } from 'react'; import { i18n } from '@kbn/i18n'; import { RedirectAppLinks } from '@kbn/shared-ux-link-redirect-app'; import { FormattedMessage } from '@kbn/i18n-react'; @@ -292,14 +292,8 @@ export const useApplicationUserMessages = ({ visualizationState?: VisualizationState; visualization?: Visualization; }) => { - const [userMessages, setUserMessages] = useState([]); - // these are messages managed from other parts of Lens - const [additionalUserMessages, setAdditionalUserMessages] = useState>( - {} - ); - - useEffect(() => { - setUserMessages([ + const userMessages = useMemo( + () => [ ...(datasourceState && datasourceState.state && datasource && activeDatasourceId ? datasource.getUserMessages(datasourceState.state, { frame: framePublicAPI, @@ -330,52 +324,63 @@ export const useApplicationUserMessages = ({ core: coreStart, dataViews: framePublicAPI.dataViews, }), - ]); - }, [ - activeDatasourceId, - datasource, - datasourceState, - dispatch, - framePublicAPI, - visualization, - visualizationState, - visualizationType, - coreStart, - ]); + ], + [ + activeDatasourceId, + datasource, + datasourceState, + dispatch, + framePublicAPI, + visualization, + visualizationState, + visualizationType, + coreStart, + ] + ); + // these are messages managed from other parts of Lens + const [additionalUserMessages, setAdditionalUserMessages] = useState>( + {} + ); - const getUserMessages: UserMessagesGetter = (locationId, filterArgs) => - filterAndSortUserMessages( - userMessages.concat(Object.values(additionalUserMessages)), - locationId, - filterArgs ?? {} - ); + const getUserMessages: UserMessagesGetter = useCallback( + (locationId, filterArgs) => + filterAndSortUserMessages( + userMessages.concat(Object.values(additionalUserMessages)), + locationId, + filterArgs ?? {} + ), + [additionalUserMessages, userMessages] + ); - const addUserMessages: AddUserMessages = (messages) => { - const newMessageMap = { - ...additionalUserMessages, - }; + const addUserMessages: AddUserMessages = useCallback( + (messages) => { + const newMessageMap = { + ...additionalUserMessages, + }; - const addedMessageIds: string[] = []; - messages.forEach((message) => { - if (!newMessageMap[message.uniqueId]) { - addedMessageIds.push(message.uniqueId); - newMessageMap[message.uniqueId] = message; + const addedMessageIds: string[] = []; + messages.forEach((message) => { + if (!newMessageMap[message.uniqueId]) { + addedMessageIds.push(message.uniqueId); + newMessageMap[message.uniqueId] = message; + } + }); + + if (addedMessageIds.length) { + setAdditionalUserMessages(newMessageMap); } - }); - if (addedMessageIds.length) { - setAdditionalUserMessages(newMessageMap); - } + return () => { + const withMessagesRemoved = { + ...additionalUserMessages, + }; - return () => { - const withMessagesRemoved = { - ...additionalUserMessages, - }; - - addedMessageIds.forEach((id) => delete withMessagesRemoved[id]); + addedMessageIds.forEach((id) => delete withMessagesRemoved[id]); - setAdditionalUserMessages(withMessagesRemoved); - }; - }; + setAdditionalUserMessages(withMessagesRemoved); + }; + }, + [additionalUserMessages] + ); return { getUserMessages, addUserMessages }; }; diff --git a/x-pack/platform/plugins/shared/lens/public/app_plugin/lens_top_nav.tsx b/x-pack/platform/plugins/shared/lens/public/app_plugin/lens_top_nav.tsx index 387b808cec7f8..abf420d780064 100644 --- a/x-pack/platform/plugins/shared/lens/public/app_plugin/lens_top_nav.tsx +++ b/x-pack/platform/plugins/shared/lens/public/app_plugin/lens_top_nav.tsx @@ -345,7 +345,8 @@ export const LensTopNavMenu = ({ ); const [indexPatterns, setIndexPatterns] = useState([]); const [currentIndexPattern, setCurrentIndexPattern] = useState(); - const [isOnTextBasedMode, setIsOnTextBasedMode] = useState(false); + const isOnTextBasedMode = + query != null && typeof query === 'object' && isOfAggregateQueryType(query); const [rejectedIndexPatterns, setRejectedIndexPatterns] = useState([]); const dispatchChangeIndexPattern = React.useCallback( @@ -416,9 +417,10 @@ export const LensTopNavMenu = ({ indexPatterns.length + rejectedIndexPatterns.length !== indexPatternIds.size || [...indexPatternIds].some( (id) => - ![...indexPatterns.map((ip) => ip.id), ...rejectedIndexPatterns].find( - (loadedId) => loadedId === id - ) + !indexPatterns + .map((ip) => ip.id) + .concat(rejectedIndexPatterns) + .some((loadedId) => loadedId === id) ); // Update the cached index patterns if the user made a change to any of them @@ -463,12 +465,6 @@ export const LensTopNavMenu = ({ isOnTextBasedMode, ]); - useEffect(() => { - if (typeof query === 'object' && query !== null && isOfAggregateQueryType(query)) { - setIsOnTextBasedMode(true); - } - }, [query]); - useEffect(() => { return () => { // Make sure to close the editors when unmounting @@ -886,7 +882,6 @@ export const LensTopNavMenu = ({ dispatchSetState({ query: newQuery as Query }); // check if query is text-based (esql etc) and switchAndCleanDatasource if (isOfAggregateQueryType(newQuery) && !isOnTextBasedMode) { - setIsOnTextBasedMode(true); dispatch( switchAndCleanDatasource({ newDatasourceId: 'textBased', @@ -1086,7 +1081,6 @@ export const LensTopNavMenu = ({ currentIndexPatternId: newIndexPatternId, }) ); - setIsOnTextBasedMode(false); } }, onEditDataView: async (updatedDataViewStub) => { diff --git a/x-pack/platform/plugins/shared/lens/public/app_plugin/shared/edit_on_the_fly/use_current_attributes.ts b/x-pack/platform/plugins/shared/lens/public/app_plugin/shared/edit_on_the_fly/use_current_attributes.ts index 60c4ca2520f67..e703e548b70a9 100644 --- a/x-pack/platform/plugins/shared/lens/public/app_plugin/shared/edit_on_the_fly/use_current_attributes.ts +++ b/x-pack/platform/plugins/shared/lens/public/app_plugin/shared/edit_on_the_fly/use_current_attributes.ts @@ -4,8 +4,7 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import { useEffect, useState } from 'react'; -import { isEqual } from 'lodash'; +import { useMemo } from 'react'; import { createEmptyLensState } from '../../../react_embeddable/helper'; import type { TypedLensSerializedState } from '../../../react_embeddable/types'; import { useLensSelector } from '../../../state_management'; @@ -27,17 +26,13 @@ export const useCurrentAttributes = ({ (state) => state.lens ); - const [currentAttributes, setCurrentAttributes] = useState< - TypedLensSerializedState['attributes'] | undefined - >(initialAttributes); - // use the latest activeId, but fallback to attributes const visualizationType = visualization.activeId ?? initialAttributes?.visualizationType; const activeVisualization = visualizationType ? visualizationMap[visualizationType] : undefined; - useEffect(() => { + const currentAttributes = useMemo(() => { if (!activeVisualization) { - return; + return initialAttributes; } const dsStates = Object.fromEntries( Object.entries(datasourceStates).map(([id, ds]) => { @@ -73,18 +68,14 @@ export const useCurrentAttributes = ({ references, visualizationType: activeVisualization.id, }; - if (!isEqual(attrs, currentAttributes)) { - setCurrentAttributes(attrs); - } + return attrs; }, [ activeDatasourceId, activeVisualization, - initialAttributes, datasourceMap, datasourceStates, - currentAttributes, + initialAttributes, textBasedMode, - visualization.activeId, visualization.state, ]); diff --git a/x-pack/platform/plugins/shared/lens/public/datasources/form_based/dimension_panel/dimension_editor.tsx b/x-pack/platform/plugins/shared/lens/public/datasources/form_based/dimension_panel/dimension_editor.tsx index ee34cdefc439b..600716b3cf9b9 100644 --- a/x-pack/platform/plugins/shared/lens/public/datasources/form_based/dimension_panel/dimension_editor.tsx +++ b/x-pack/platform/plugins/shared/lens/public/datasources/form_based/dimension_panel/dimension_editor.tsx @@ -480,7 +480,7 @@ export function DimensionEditor(props: DimensionEditorProps) { } else if (!compatibleWithCurrentField) { label = ( - + - + {label} {shouldDisplayDots && ( @@ -625,7 +625,7 @@ export function DimensionEditor(props: DimensionEditorProps) { indexPattern: currentIndexPattern, columnId, op: operationType, - field: currentIndexPattern.getFieldByName(possibleFields.values().next().value), + field: currentIndexPattern.getFieldByName(possibleFields.values().next().value!), visualizationGroups: dimensionGroups, targetGroup: props.groupId, }); diff --git a/x-pack/platform/plugins/shared/lens/public/datasources/form_based/utils.test.tsx b/x-pack/platform/plugins/shared/lens/public/datasources/form_based/utils.test.tsx index 11673c7f6ddd3..40d38cf208608 100644 --- a/x-pack/platform/plugins/shared/lens/public/datasources/form_based/utils.test.tsx +++ b/x-pack/platform/plugins/shared/lens/public/datasources/form_based/utils.test.tsx @@ -144,7 +144,6 @@ describe('indexpattern_datasource utils', () => { ); render({getLongMessage(warningMessages[0])}); - screen.debug(); // Make sure the message is there before checking the absence of the link/button expect(screen.getByText(/might be an approximation./)).toBeInTheDocument(); expect(screen.queryByText('Enable accuracy mode')).toBe(null); diff --git a/x-pack/platform/plugins/shared/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx b/x-pack/platform/plugins/shared/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx index fc2fa2cac18a9..8594aac3d435c 100644 --- a/x-pack/platform/plugins/shared/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx +++ b/x-pack/platform/plugins/shared/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx @@ -242,12 +242,17 @@ export function LayerPanels( [dispatchLens, props.framePublicAPI.dataViews.indexPatterns, props.indexPatternService] ); - const addLayer: AddLayerFunction = (layerType, extraArg, ignoreInitialValues, seriesType) => { - const layerId = generateId(); - dispatchLens(addLayerAction({ layerId, layerType, extraArg, ignoreInitialValues, seriesType })); + const addLayer: AddLayerFunction = useCallback( + (layerType, extraArg, ignoreInitialValues, seriesType) => { + const layerId = generateId(); + dispatchLens( + addLayerAction({ layerId, layerType, extraArg, ignoreInitialValues, seriesType }) + ); - setNextFocusedLayerId(layerId); - }; + setNextFocusedLayerId(layerId); + }, + [dispatchLens, setNextFocusedLayerId] + ); const registerLibraryAnnotationGroupFunction = useCallback< LayerPanelProps['registerLibraryAnnotationGroup'] @@ -255,6 +260,106 @@ export function LayerPanels( const hideAddLayerButton = query && isOfAggregateQueryType(query); + const LayerPanelComponents = useMemo(() => { + return layerIds.map((layerId, layerIndex) => { + const { hidden, groups } = activeVisualization.getConfiguration({ + layerId, + frame: props.framePublicAPI, + state: visualization.state, + }); + + if (hidden) { + return null; + } + + return ( + { + onChangeIndexPattern(args); + const layersToRemove = + activeVisualization.getLayersToRemoveOnIndexPatternChange?.(visualization.state) ?? + []; + layersToRemove.forEach((id) => onRemoveLayer(id)); + }} + updateAll={updateAll} + addLayer={addLayer} + isOnlyLayer={ + getRemoveOperation( + activeVisualization, + visualization.state, + layerId, + layerIds.length + ) === 'clear' + } + onEmptyDimensionAdd={(columnId, { groupId }) => { + // avoid state update if the datasource does not support initializeDimension + if ( + activeDatasourceId != null && + datasourceMap[activeDatasourceId]?.initializeDimension + ) { + dispatchLens( + setLayerDefaultDimension({ + layerId, + columnId, + groupId, + }) + ); + } + }} + onCloneLayer={() => { + dispatchLens( + cloneLayer({ + layerId, + }) + ); + }} + onRemoveLayer={onRemoveLayer} + onRemoveDimension={(dimensionProps) => { + const datasourcePublicAPI = props.framePublicAPI.datasourceLayers?.[layerId]; + const datasourceId = datasourcePublicAPI?.datasourceId; + dispatchLens(removeDimension({ ...dimensionProps, datasourceId })); + }} + toggleFullscreen={toggleFullscreen} + indexPatternService={indexPatternService} + /> + ); + }); + }, [ + activeDatasourceId, + activeVisualization, + addLayer, + datasourceMap, + dispatchLens, + handleDimensionDrop, + indexPatternService, + layerIds, + onChangeIndexPattern, + onRemoveLayer, + props, + registerLibraryAnnotationGroupFunction, + registerNewLayerRef, + setVisualizationState, + toggleFullscreen, + updateAll, + updateDatasource, + updateDatasourceAsync, + visualization.state, + ]); + return ( - {layerIds.map((layerId, layerIndex) => { - const { hidden, groups } = activeVisualization.getConfiguration({ - layerId, - frame: props.framePublicAPI, - state: visualization.state, - }); - - return ( - !hidden && ( - { - onChangeIndexPattern(args); - const layersToRemove = - activeVisualization.getLayersToRemoveOnIndexPatternChange?.( - visualization.state - ) ?? []; - layersToRemove.forEach((id) => onRemoveLayer(id)); - }} - updateAll={updateAll} - addLayer={addLayer} - isOnlyLayer={ - getRemoveOperation( - activeVisualization, - visualization.state, - layerId, - layerIds.length - ) === 'clear' - } - onEmptyDimensionAdd={(columnId, { groupId }) => { - // avoid state update if the datasource does not support initializeDimension - if ( - activeDatasourceId != null && - datasourceMap[activeDatasourceId]?.initializeDimension - ) { - dispatchLens( - setLayerDefaultDimension({ - layerId, - columnId, - groupId, - }) - ); - } - }} - onCloneLayer={() => { - dispatchLens( - cloneLayer({ - layerId, - }) - ); - }} - onRemoveLayer={onRemoveLayer} - onRemoveDimension={(dimensionProps) => { - const datasourcePublicAPI = props.framePublicAPI.datasourceLayers?.[layerId]; - const datasourceId = datasourcePublicAPI?.datasourceId; - dispatchLens(removeDimension({ ...dimensionProps, datasourceId })); - }} - toggleFullscreen={toggleFullscreen} - indexPatternService={indexPatternService} - panelId={props.panelId} - parentApi={props.parentApi} - closeFlyout={props.closeFlyout} - /> - ) - ); - })} + {LayerPanelComponents} {!hideAddLayerButton && activeVisualization?.getAddLayerButtonComponent?.({ state: visualization.state, diff --git a/x-pack/platform/plugins/shared/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx b/x-pack/platform/plugins/shared/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx index ae22fa6778948..6ed2d03fdaa82 100644 --- a/x-pack/platform/plugins/shared/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx +++ b/x-pack/platform/plugins/shared/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx @@ -229,7 +229,7 @@ describe('editor_frame', () => { ); }); - expect(mockVisualization.getConfiguration).toHaveBeenCalledTimes(4); + expect(mockVisualization.getConfiguration).toHaveBeenCalledTimes(2); expect(mockVisualization.getConfiguration).toHaveBeenLastCalledWith( expect.objectContaining({ state: updatedState, diff --git a/x-pack/platform/plugins/shared/lens/public/state_management/lens_slice.ts b/x-pack/platform/plugins/shared/lens/public/state_management/lens_slice.ts index 85ef2e7fd544a..46780fd19e715 100644 --- a/x-pack/platform/plugins/shared/lens/public/state_management/lens_slice.ts +++ b/x-pack/platform/plugins/shared/lens/public/state_management/lens_slice.ts @@ -897,14 +897,19 @@ export const makeLensReducer = (storeDeps: LensStoreDeps) => { typeof updater === 'function' ? updater(current(state.visualization.state)) : updater; } + const datasourceByLayerId = new Map( + Object.entries(datasourceMap).flatMap(([datasourceId, datasource]) => { + if (!state.datasourceStates[datasourceId]) { + return []; + } + return datasource + .getLayers(state.datasourceStates[datasourceId].state) + .map((layerId) => [layerId, datasourceId]); + }) ?? [] + ); + layerIds.forEach((layerId) => { - const [layerDatasourceId] = - Object.entries(datasourceMap).find(([datasourceId, datasource]) => { - return ( - state.datasourceStates[datasourceId] && - datasource.getLayers(state.datasourceStates[datasourceId].state).includes(layerId) - ); - }) ?? []; + const layerDatasourceId = datasourceByLayerId.get(layerId); if (layerDatasourceId) { const { newState } = datasourceMap[layerDatasourceId].removeLayer( current(state).datasourceStates[layerDatasourceId].state,