[BUG] Resolve display issue and improve performance

Auto-resized issue is caused by we have a central discover context created useSearch. When it is updated, both panel and canvas which use useDiscoverContext will render. The update on panel will trigger useSearch and cause canvas render before update is done. * use reload to resolve table auto adjust issue * clean out panel to have single useDiscoverContext Issue Resolved: #5440 Signed-off-by: ananzh <[email protected]>
opensearch-project · Jan 29, 2024 · c3936e2 · c3936e2
1 parent f8ee03a
commit c3936e2
Show file tree

Hide file tree

Showing 10 changed files with 164 additions and 178 deletions.
diff --git a/src/plugins/discover/public/application/components/chart/chart.tsx b/src/plugins/discover/public/application/components/chart/chart.tsx
@@ -17,19 +17,20 @@ import { TimechartHeader, TimechartHeaderBucketInterval } from './timechart_head
 import { DiscoverHistogram } from './histogram/histogram';
 import { DiscoverServices } from '../../../build_services';
 import { Chart } from './utils';
-import { useDiscoverContext } from '../../view_components/context';
 import { setInterval, useDispatch, useSelector } from '../../utils/state_management';
+import { RefetchSubject } from '../../view_components/utils/use_search';
 
 interface DiscoverChartProps {
-  bucketInterval?: TimechartHeaderBucketInterval;
-  chartData?: Chart;
+  services: DiscoverServices;
   config: IUiSettingsClient;
   data: DataPublicPluginStart;
   hits: number;
   resetQuery: () => void;
+  bucketInterval?: TimechartHeaderBucketInterval;
+  chartData?: Chart;
   showResetButton?: boolean;
   isTimeBased?: boolean;
-  services: DiscoverServices;
+  refetch?: RefetchSubject;
 }
 
 export const DiscoverChart = ({
@@ -42,8 +43,8 @@ export const DiscoverChart = ({
   isTimeBased,
   services,
   showResetButton = false,
+  refetch,
 }: DiscoverChartProps) => {
-  const { refetch$ } = useDiscoverContext();
   const { from, to } = data.query.timefilter.timefilter.getTime();
   const timeRange = {
     from: dateMath.parse(from)?.format('YYYY-MM-DDTHH:mm:ss.SSSZ') || '',
@@ -53,7 +54,7 @@ export const DiscoverChart = ({
   const dispatch = useDispatch();
   const onChangeInterval = (newInterval: string) => {
     dispatch(setInterval(newInterval));
-    refetch$.next();
+    refetch?.next();
   };
   const timefilterUpdateHandler = useCallback(
     (ranges: { from: number; to: number }) => {

diff --git a/src/plugins/discover/public/application/view_components/canvas/discover_canvas.scss b/src/plugins/discover/public/application/view_components/canvas/discover_canvas.scss
@@ -34,3 +34,7 @@
     }
   }
 }
+
+.trigger-reflow {
+  padding-right: 1px; /* Or any other property that affects layout */
+}
diff --git a/src/plugins/discover/public/application/view_components/canvas/discover_chart_container.tsx b/src/plugins/discover/public/application/view_components/canvas/discover_chart_container.tsx
@@ -5,16 +5,34 @@
 
 import './discover_chart_container.scss';
 import React, { useMemo } from 'react';
-import { DiscoverViewServices } from '../../../build_services';
-import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_react/public';
-import { useDiscoverContext } from '../context';
-import { SearchData } from '../utils/use_search';
 import { DiscoverChart } from '../../components/chart/chart';
+import { DiscoverViewServices } from '../../../build_services';
+import { IndexPattern } from '../../../opensearch_dashboards_services';
+import { SavedSearch } from '../../../saved_searches';
+import { TimechartHeaderBucketInterval } from '../../components/chart/timechart_header';
+import { Chart } from '../../components/chart/utils';
+import { RefetchSubject } from '../utils/use_search';
+
+interface DiscoverChartContainerProps {
+  services: DiscoverViewServices;
+  hits?: number;
+  bucketInterval?: TimechartHeaderBucketInterval | {};
+  chartData?: Chart;
+  indexPattern?: IndexPattern;
+  savedSearch?: SavedSearch;
+  refetch?: RefetchSubject;
+}
 
-export const DiscoverChartContainer = ({ hits, bucketInterval, chartData }: SearchData) => {
-  const { services } = useOpenSearchDashboards<DiscoverViewServices>();
+export const DiscoverChartContainer = ({
+  hits,
+  bucketInterval,
+  chartData,
+  services,
+  indexPattern,
+  savedSearch,
+  refetch,
+}: DiscoverChartContainerProps) => {
   const { uiSettings, data, core } = services;
-  const { indexPattern, savedSearch } = useDiscoverContext();
 
   const isTimeBased = useMemo(() => (indexPattern ? indexPattern.isTimeBased() : false), [
     indexPattern,
@@ -35,6 +53,7 @@ export const DiscoverChartContainer = ({ hits, bucketInterval, chartData }: Sear
       services={services}
       showResetButton={!!savedSearch && !!savedSearch.id}
       isTimeBased={isTimeBased}
+      refetch={refetch}
     />
   );
 };
diff --git a/src/plugins/discover/public/application/view_components/canvas/discover_table.tsx b/src/plugins/discover/public/application/view_components/canvas/discover_table.tsx
@@ -5,9 +5,7 @@
 
 import React, { useCallback, useMemo } from 'react';
 import { DiscoverViewServices } from '../../../build_services';
-import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_react/public';
 import { DataGridTable } from '../../components/data_grid/data_grid_table';
-import { useDiscoverContext } from '../context';
 import {
   addColumn,
   removeColumn,
@@ -22,13 +20,27 @@ import { SortOrder } from '../../../saved_searches/types';
 import { DOC_HIDE_TIME_COLUMN_SETTING } from '../../../../common';
 import { OpenSearchSearchHit } from '../../doc_views/doc_views_types';
 import { popularizeField } from '../../helpers/popularize_field';
+import { IndexPattern } from '../../../opensearch_dashboards_services';
+import { SavedSearch } from '../../../saved_searches';
+import { RefetchSubject } from '../../view_components/utils/use_search';
 
-interface Props {
+interface DiscoverTableProps {
+  services: DiscoverViewServices;
+  refetch: RefetchSubject;
+  setShouldFetchOnColumnChange: React.Dispatch<React.SetStateAction<boolean>>;
+  indexPattern: IndexPattern;
   rows?: OpenSearchSearchHit[];
+  savedSearch?: SavedSearch;
 }
 
-export const DiscoverTable = ({ rows }: Props) => {
-  const { services } = useOpenSearchDashboards<DiscoverViewServices>();
+export const DiscoverTable = ({
+  services,
+  refetch,
+  setShouldFetchOnColumnChange,
+  indexPattern,
+  rows,
+  savedSearch,
+}: DiscoverTableProps) => {
   const {
     uiSettings,
     data: {
@@ -38,27 +50,29 @@ export const DiscoverTable = ({ rows }: Props) => {
     indexPatterns,
   } = services;
 
-  const { refetch$, indexPattern, savedSearch } = useDiscoverContext();
   const { columns, sort } = useSelector((state) => state.discover);
   const dispatch = useDispatch();
   const onAddColumn = (col: string) => {
     if (indexPattern && capabilities.discover?.save) {
       popularizeField(indexPattern, col, indexPatterns);
     }
-
+    setShouldFetchOnColumnChange(false);
     dispatch(addColumn({ column: col }));
   };
   const onRemoveColumn = (col: string) => {
     if (indexPattern && capabilities.discover?.save) {
       popularizeField(indexPattern, col, indexPatterns);
     }
-
+    setShouldFetchOnColumnChange(false);
     dispatch(removeColumn(col));
   };
-  const onSetColumns = (cols: string[]) => dispatch(setColumns({ columns: cols }));
+  const onSetColumns = (cols: string[]) => {
+    setShouldFetchOnColumnChange(false);
+    dispatch(setColumns({ columns: cols }));
+  };
   const onSetSort = (s: SortOrder[]) => {
     dispatch(setSort(s));
-    refetch$.next();
+    refetch.next();
   };
   const onAddFilter = useCallback(
     (field: string | IndexPatternField, values: string, operation: '+' | '-') => {
@@ -80,11 +94,6 @@ export const DiscoverTable = ({ rows }: Props) => {
     [indexPattern, uiSettings]
   );
 
-  if (indexPattern === undefined) {
-    // TODO: handle better
-    return null;
-  }
-
   if (!rows || rows.length === 0) {
     // TODO: handle better
     return <div>{'loading...'}</div>;

diff --git a/src/plugins/discover/public/application/view_components/canvas/index.tsx b/src/plugins/discover/public/application/view_components/canvas/index.tsx
@@ -19,15 +19,21 @@ import { DiscoverViewServices } from '../../../build_services';
 import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_react/public';
 import { filterColumns } from '../utils/filter_columns';
 import { DEFAULT_COLUMNS_SETTING, MODIFY_COLUMNS_ON_SWITCH } from '../../../../common';
-import { OpenSearchSearchHit } from '../../../application/doc_views/doc_views_types';
 import './discover_canvas.scss';
 
 // eslint-disable-next-line import/no-default-export
 export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewProps) {
-  const { data$, refetch$, indexPattern } = useDiscoverContext();
   const {
-    services: { uiSettings },
-  } = useOpenSearchDashboards<DiscoverViewServices>();
+    data$,
+    refetch$,
+    indexPattern,
+    savedSearch,
+    inspectorAdapters,
+    shouldFetchOnColumnChange,
+    setShouldFetchOnColumnChange,
+  } = useDiscoverContext();
+  const { services } = useOpenSearchDashboards<DiscoverViewServices>();
+  const { uiSettings } = services;
   const { columns } = useSelector((state) => state.discover);
   const filteredColumns = filterColumns(
     columns,
@@ -52,7 +58,6 @@ export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewPro
     },
     [refetch$]
   );
-  const [rows, setRows] = useState<OpenSearchSearchHit[] | undefined>(undefined);
 
   useEffect(() => {
     const subscription = data$.subscribe((next) => {
@@ -67,19 +72,19 @@ export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewPro
       if (next.chartData && next.chartData !== fetchState.chartData) shouldUpdateState = true;
       if (next.rows && next.rows !== fetchState.rows) {
         shouldUpdateState = true;
-        setRows(next.rows);
       }
 
       // Update the state if any condition is met.
       if (shouldUpdateState) {
+        setShouldFetchOnColumnChange(true);
         setFetchState({ ...fetchState, ...next });
       }
     });
 
     return () => {
       subscription.unsubscribe();
     };
-  }, [data$, fetchState]);
+  }, [data$, fetchState, setShouldFetchOnColumnChange]);
 
   useEffect(() => {
     if (indexPattern !== prevIndexPattern.current) {
@@ -90,6 +95,23 @@ export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewPro
 
   const timeField = indexPattern?.timeFieldName ? indexPattern.timeFieldName : undefined;
 
+  useEffect(() => {
+    if (fetchState.status === ResultStatus.READY) {
+      const element = document.querySelector('.dscCanvas') as HTMLElement;
+      if (element) {
+        element.classList.add('trigger-reflow');
+        setTimeout(() => {
+          element.classList.remove('trigger-reflow');
+        }, 10);
+      }
+    }
+  }, [fetchState.status]);
+
+  if (indexPattern === undefined || shouldFetchOnColumnChange === false) {
+    // TODO: handle better
+    return null;
+  }
+
   return (
     <EuiPanel
       hasBorder={false}
@@ -98,11 +120,15 @@ export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewPro
       paddingSize="none"
       className="dscCanvas"
     >
-      <TopNav
+      <MemorizedTopNav
         opts={{
           setHeaderActionMenu,
           onQuerySubmit,
         }}
+        services={services}
+        inspectorAdapters={inspectorAdapters}
+        savedSearch={savedSearch}
+        indexPattern={indexPattern}
       />
       {fetchState.status === ResultStatus.NO_RESULTS && (
         <DiscoverNoResults timeFieldName={timeField} queryLanguage={''} />
@@ -115,15 +141,31 @@ export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewPro
         <>
           <EuiPanel hasBorder={false} hasShadow={false} color="transparent" paddingSize="s">
             <EuiPanel>
-              <MemoizedDiscoverChartContainer {...fetchState} />
+              <MemoizedDiscoverChartContainer
+                services={services}
+                hits={fetchState.hits}
+                bucketInterval={fetchState.bucketInterval}
+                chartData={fetchState.chartData}
+                indexPattern={indexPattern}
+                savedSearch={savedSearch}
+                refetch={refetch$}
+              />
             </EuiPanel>
           </EuiPanel>
-          <MemoizedDiscoverTable rows={rows} />
+          <MemoizedDiscoverTable
+            services={services}
+            rows={fetchState.rows}
+            refetch={refetch$}
+            indexPattern={indexPattern}
+            savedSearch={savedSearch}
+            setShouldFetchOnColumnChange={setShouldFetchOnColumnChange}
+          />
         </>
       )}
     </EuiPanel>
   );
 }
 
+const MemorizedTopNav = React.memo(TopNav);
 const MemoizedDiscoverTable = React.memo(DiscoverTable);
 const MemoizedDiscoverChartContainer = React.memo(DiscoverChartContainer);
diff --git a/src/plugins/discover/public/application/view_components/canvas/top_nav.tsx b/src/plugins/discover/public/application/view_components/canvas/top_nav.tsx
@@ -7,24 +7,32 @@ import React, { useEffect, useMemo, useState } from 'react';
 import { TimeRange, Query } from 'src/plugins/data/common';
 import { AppMountParameters } from '../../../../../../core/public';
 import { PLUGIN_ID } from '../../../../common';
-import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_react/public';
 import { DiscoverViewServices } from '../../../build_services';
 import { IndexPattern } from '../../../opensearch_dashboards_services';
 import { getTopNavLinks } from '../../components/top_nav/get_top_nav_links';
-import { useDiscoverContext } from '../context';
 import { getRootBreadcrumbs } from '../../helpers/breadcrumbs';
 import { opensearchFilters, connectStorageToQueryState } from '../../../../../data/public';
+import { SavedSearch } from '../../../saved_searches';
+import { InspectorAdapters } from '../utils/use_search';
 
 export interface TopNavProps {
   opts: {
     setHeaderActionMenu: AppMountParameters['setHeaderActionMenu'];
     onQuerySubmit: (payload: { dateRange: TimeRange; query?: Query }, isUpdate?: boolean) => void;
   };
+  services: DiscoverViewServices;
+  inspectorAdapters: InspectorAdapters;
+  indexPattern?: IndexPattern;
+  savedSearch?: SavedSearch;
 }
 
-export const TopNav = ({ opts }: TopNavProps) => {
-  const { services } = useOpenSearchDashboards<DiscoverViewServices>();
-  const { inspectorAdapters, savedSearch, indexPattern } = useDiscoverContext();
+export const TopNav = ({
+  opts,
+  services,
+  inspectorAdapters,
+  savedSearch,
+  indexPattern,
+}: TopNavProps) => {
   const [indexPatterns, setIndexPatterns] = useState<IndexPattern[] | undefined>(undefined);
 
   const {

diff --git a/src/plugins/discover/public/application/view_components/panel/index.tsx b/src/plugins/discover/public/application/view_components/panel/index.tsx
@@ -32,7 +32,7 @@ export default function DiscoverPanel(props: ViewProps) {
     capabilities,
     indexPatterns,
   } = services;
-  const { data$, indexPattern } = useDiscoverContext();
+  const { data$, indexPattern, setShouldFetchOnColumnChange } = useDiscoverContext();
   const [fetchState, setFetchState] = useState<SearchData>(data$.getValue());
 
   const { columns } = useSelector((state) => ({
@@ -95,7 +95,7 @@ export default function DiscoverPanel(props: ViewProps) {
         if (indexPattern && capabilities.discover?.save) {
           popularizeField(indexPattern, fieldName, indexPatterns);
         }
-
+        setShouldFetchOnColumnChange(false);
         dispatch(
           addColumn({
             column: fieldName,
@@ -107,7 +107,7 @@ export default function DiscoverPanel(props: ViewProps) {
         if (indexPattern && capabilities.discover?.save) {
           popularizeField(indexPattern, fieldName, indexPatterns);
         }
-
+        setShouldFetchOnColumnChange(false);
         dispatch(removeColumn(fieldName));
       }}
       onReorderFields={(source, destination) => {
-Original file line number
+Diff line change
@@ Expand Up / @@ -34,3 +34,7 @@ @@
         }
       }
     }
+    .trigger-reflow {
+      padding-right: 1px; /* Or any other property that affects layout */
+    }