Add a new isLoading internal state that is set to true when a ad hoc …

…data view changes
elastic · Oct 4, 2024 · 7d4e3da · 7d4e3da
1 parent 036538a
commit 7d4e3da
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 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
@@ -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
@@ -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<Error>();
-  const [loading, setLoading] = useState(true);
   const [hasESData, setHasESData] = useState(false);
   const [hasUserDataView, setHasUserDataView] = useState(false);
   const [showNoDataPage, setShowNoDataPage] = useState<boolean>(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 <DiscoverMainAppMemoized stateContainer={stateContainer} />;
   }, [
-    loading,
-    loadingIndicator,
     noDataDependencies,
     onDataViewCreated,
     onESQLNavigationComplete,
@@ -354,20 +347,40 @@ export function DiscoverMainRoute({
   return (
     <DiscoverCustomizationProvider value={customizationService}>
       <DiscoverMainProvider value={stateContainer}>
-        <>
-          <DiscoverTopNavInline
-            stateContainer={stateContainer}
-            hideNavMenuItems={loading || showNoDataPage}
-          />
-          {mainContent}
-        </>
+        <DiscoverMainLoading
+          mainContent={mainContent}
+          showNoDataPage={showNoDataPage}
+          stateContainer={stateContainer}
+        />
       </DiscoverMainProvider>
     </DiscoverCustomizationProvider>
   );
 }
 // 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 <LoadingIndicator />;
+  }
+
+  return (
+    <>
+      <DiscoverTopNavInline stateContainer={stateContainer} hideNavMenuItems={showNoDataPage} />
+      {mainContent}
+    </>
+  );
+}
+
 function getLoadParamsForNewSearch(stateContainer: DiscoverStateContainer): {
   nextDataView: LoadParams['dataView'];
   initialAppState: LoadParams['initialAppState'];

diff --git a/...ns/discover/public/application/main/state_management/discover_internal_state_container.ts b/...ns/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<InternalState, InternalStateTransitions, {}>(
     {
       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
@@ -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();
   };