From 7d4e3dadc089754b6791415fb237a38f5a819810 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Fri, 4 Oct 2024 18:55:39 +0200 Subject: [PATCH] Add a new isLoading internal state that is set to true when a ad hoc data view changes --- .../components/top_nav/discover_topnav.tsx | 34 ++--------- .../application/main/discover_main_route.tsx | 57 ++++++++++++------- .../discover_internal_state_container.ts | 7 +++ .../main/state_management/discover_state.ts | 2 + 4 files changed, 48 insertions(+), 52 deletions(-) diff --git a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx index 45e8755ca3156..86c3a85d705bb 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx @@ -8,7 +8,7 @@ */ import React, { useCallback, useEffect, useMemo, useRef } from 'react'; -import { type DataView, DataViewType } from '@kbn/data-views-plugin/public'; +import { DataViewType } from '@kbn/data-views-plugin/public'; import { DataViewPickerProps } from '@kbn/unified-search-plugin/public'; import { ENABLE_ESQL } from '@kbn/esql-utils'; import { TextBasedLanguages } from '@kbn/esql-utils'; @@ -20,7 +20,6 @@ import { useDiscoverServices } from '../../../../hooks/use_discover_services'; import type { DiscoverStateContainer } from '../../state_management/discover_state'; import { onSaveSearch } from './on_save_search'; import { useDiscoverCustomization } from '../../../../customizations'; -import { addLog } from '../../../../utils/add_log'; import { useAppStateSelector } from '../../state_management/discover_app_state_container'; import { useDiscoverTopNav } from './use_discover_topnav'; import { useIsEsqlMode } from '../../hooks/use_is_esql_mode'; @@ -46,15 +45,8 @@ export const DiscoverTopNav = ({ onCancelClick, }: DiscoverTopNavProps) => { const services = useDiscoverServices(); - const { - dataViewEditor, - navigation, - dataViewFieldEditor, - data, - uiSettings, - dataViews, - setHeaderActionMenu, - } = services; + const { dataViewEditor, navigation, dataViewFieldEditor, data, uiSettings, setHeaderActionMenu } = + services; const query = useAppStateSelector((state) => state.query); const adHocDataViews = useInternalStateSelector((state) => state.adHocDataViews); const dataView = useInternalStateSelector((state) => state.dataView!); @@ -122,23 +114,6 @@ export const DiscoverTopNav = ({ }); }, [dataViewEditor, stateContainer]); - const onEditDataView = useCallback( - async (editedDataView: DataView) => { - if (editedDataView.isPersisted()) { - // Clear the current data view from the cache and create a new instance - // of it, ensuring we have a new object reference to trigger a re-render - dataViews.clearInstanceCache(editedDataView.id); - stateContainer.actions.setDataView(await dataViews.create(editedDataView.toSpec(), true)); - } else { - await stateContainer.actions.updateAdHocDataViewId(); - } - stateContainer.actions.loadDataViewList(); - addLog('[DiscoverTopNav] onEditDataView triggers data fetching'); - stateContainer.dataState.fetch(); - }, - [dataViews, stateContainer.actions, stateContainer.dataState] - ); - const updateSavedQueryId = (newSavedQueryId: string | undefined) => { const { appState } = stateContainer; if (newSavedQueryId) { @@ -221,14 +196,13 @@ export const DiscoverTopNav = ({ textBasedLanguages: supportedTextBasedLanguages, adHocDataViews, savedDataViews, - onEditDataView, + onEditDataView: stateContainer.actions.onDataViewEdited, }; }, [ adHocDataViews, addField, createNewDataView, dataView, - onEditDataView, savedDataViews, stateContainer, uiSettings, diff --git a/src/plugins/discover/public/application/main/discover_main_route.tsx b/src/plugins/discover/public/application/main/discover_main_route.tsx index e763a272e45d0..57173717de2a2 100644 --- a/src/plugins/discover/public/application/main/discover_main_route.tsx +++ b/src/plugins/discover/public/application/main/discover_main_route.tsx @@ -22,6 +22,7 @@ import { reportPerformanceMetricEvent } from '@kbn/ebt-tools'; import { withSuspense } from '@kbn/shared-ux-utility'; import { getInitialESQLQuery } from '@kbn/esql-utils'; import { ESQL_TYPE } from '@kbn/data-view-utils'; +import { useInternalStateSelector } from './state_management/discover_internal_state_container'; import { useUrl } from './hooks/use_url'; import { useDiscoverStateContainer } from './hooks/use_discover_state_container'; import { MainHistoryLocationState } from '../../../common'; @@ -85,7 +86,6 @@ export function DiscoverMainRoute({ stateContainer, }); const [error, setError] = useState(); - const [loading, setLoading] = useState(true); const [hasESData, setHasESData] = useState(false); const [hasUserDataView, setHasUserDataView] = useState(false); const [showNoDataPage, setShowNoDataPage] = useState(false); @@ -156,9 +156,9 @@ export function DiscoverMainRoute({ initialAppState, }: { nextDataView?: DataView; initialAppState?: LoadParams['initialAppState'] } = {}) => { const loadSavedSearchStartTime = window.performance.now(); - setLoading(true); + stateContainer.internalState.transitions.setIsLoading(true); if (!nextDataView && !(await checkData())) { - setLoading(false); + stateContainer.internalState.transitions.setIsLoading(false); return; } try { @@ -181,7 +181,7 @@ export function DiscoverMainRoute({ setBreadcrumbs({ services, titleBreadcrumbText: currentSavedSearch?.title ?? undefined }); } - setLoading(false); + stateContainer.internalState.transitions.setIsLoading(false); if (services.analytics) { const loadSavedSearchDuration = window.performance.now() - loadSavedSearchStartTime; reportPerformanceMetricEvent(services.analytics, { @@ -215,7 +215,7 @@ export function DiscoverMainRoute({ }, [ checkData, - stateContainer.actions, + stateContainer, savedSearchId, historyLocationState?.dataViewSpec, customizationContext.displayMode, @@ -231,8 +231,7 @@ export function DiscoverMainRoute({ useEffect(() => { if (!isCustomizationServiceInitialized) return; - - setLoading(true); + stateContainer.internalState.transitions.setIsLoading(true); setHasESData(false); setHasUserDataView(false); setShowNoDataPage(false); @@ -258,13 +257,13 @@ export function DiscoverMainRoute({ const onDataViewCreated = useCallback( async (nextDataView: unknown) => { if (nextDataView) { - setLoading(true); + stateContainer.internalState.transitions.setIsLoading(true); setShowNoDataPage(false); setError(undefined); await loadSavedSearch({ nextDataView: nextDataView as DataView }); } }, - [loadSavedSearch] + [loadSavedSearch, stateContainer] ); const onESQLNavigationComplete = useCallback(async () => { @@ -325,14 +324,8 @@ export function DiscoverMainRoute({ ); } - if (loading) { - return loadingIndicator; - } - return ; }, [ - loading, - loadingIndicator, noDataDependencies, onDataViewCreated, onESQLNavigationComplete, @@ -354,13 +347,11 @@ export function DiscoverMainRoute({ return ( - <> - - {mainContent} - + ); @@ -368,6 +359,28 @@ export function DiscoverMainRoute({ // eslint-disable-next-line import/no-default-export export default DiscoverMainRoute; +export function DiscoverMainLoading({ + stateContainer, + showNoDataPage, + mainContent, +}: { + stateContainer: DiscoverStateContainer; + showNoDataPage: boolean; + mainContent: JSX.Element; +}) { + const loading = useInternalStateSelector((state) => state.isLoading); + if (loading && !showNoDataPage) { + return ; + } + + return ( + <> + + {mainContent} + + ); +} + function getLoadParamsForNewSearch(stateContainer: DiscoverStateContainer): { nextDataView: LoadParams['dataView']; initialAppState: LoadParams['initialAppState']; diff --git a/src/plugins/discover/public/application/main/state_management/discover_internal_state_container.ts b/src/plugins/discover/public/application/main/state_management/discover_internal_state_container.ts index d5f7deecf980a..19a9bf1438c96 100644 --- a/src/plugins/discover/public/application/main/state_management/discover_internal_state_container.ts +++ b/src/plugins/discover/public/application/main/state_management/discover_internal_state_container.ts @@ -19,6 +19,7 @@ import type { UnifiedHistogramVisContext } from '@kbn/unified-histogram-plugin/p export interface InternalState { dataView: DataView | undefined; + isLoading: boolean; isDataViewLoading: boolean; savedDataViews: DataViewListItem[]; adHocDataViews: DataView[]; @@ -32,6 +33,7 @@ export interface InternalState { export interface InternalStateTransitions { setDataView: (state: InternalState) => (dataView: DataView) => InternalState; setIsDataViewLoading: (state: InternalState) => (isLoading: boolean) => InternalState; + setIsLoading: (state: InternalState) => (isLoading: boolean) => InternalState; setSavedDataViews: (state: InternalState) => (dataView: DataViewListItem[]) => InternalState; setAdHocDataViews: (state: InternalState) => (dataViews: DataView[]) => InternalState; appendAdHocDataViews: ( @@ -71,6 +73,7 @@ export function getInternalStateContainer() { return createStateContainer( { dataView: undefined, + isLoading: true, isDataViewLoading: false, adHocDataViews: [], savedDataViews: [], @@ -90,6 +93,10 @@ export function getInternalStateContainer() { ...prevState, isDataViewLoading: loading, }), + setIsLoading: (prevState: InternalState) => (isLoading: boolean) => ({ + ...prevState, + isLoading, + }), setIsESQLToDataViewTransitionModalVisible: (prevState: InternalState) => (isVisible: boolean) => ({ ...prevState, diff --git a/src/plugins/discover/public/application/main/state_management/discover_state.ts b/src/plugins/discover/public/application/main/state_management/discover_state.ts index f014f48bdf163..542c861e80e6c 100644 --- a/src/plugins/discover/public/application/main/state_management/discover_state.ts +++ b/src/plugins/discover/public/application/main/state_management/discover_state.ts @@ -408,6 +408,7 @@ export function getDiscoverStateContainer({ }; const onDataViewEdited = async (editedDataView: DataView) => { + internalStateContainer.transitions.setIsLoading(true); const edited = editedDataView.id; if (editedDataView.isPersisted()) { // Clear the current data view from the cache and create a new instance @@ -419,6 +420,7 @@ export function getDiscoverStateContainer({ } await loadDataViewList(); addLog('[getDiscoverStateContainer] onDataViewEdited triggers data fetching'); + internalStateContainer.transitions.setIsLoading(false); fetchData(); };