[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: opensearch-project#5440 Signed-off-by: ananzh <[email protected]>
ananzh · Jan 29, 2024 · f228a85 · f228a85
1 parent f8ee03a
commit f228a85
Show file tree

Hide file tree

Showing 11 changed files with 166 additions and 178 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -51,6 +51,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [Discover] Fix missing index pattern field from breaking Discover [#5626](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5626)
 - [BUG] Remove duplicate sample data as id 90943e30-9a47-11e8-b64d-95841ca0b247 ([5668](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5668))
 - [BUG][Multiple Datasource] Fix datasource testing connection unexpectedly passed with wrong endpoint [#5663](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5663)
+- [BUG] Resolve display issue and improve performance in Discover ([#5514](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5514))
 
 ### 🚞 Infrastructure
 

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,24 @@ export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewPro
 
   const timeField = indexPattern?.timeFieldName ? indexPattern.timeFieldName : undefined;
 
+  // Add a reflow to ensure that updated content is properly laid out on the page.
+  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 +121,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 +142,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) => {