From 4a71389570c730f99a9bfdff6cb2ea8692bdaef4 Mon Sep 17 00:00:00 2001 From: Anan Zhuang Date: Tue, 3 Oct 2023 16:33:10 -0700 Subject: [PATCH] [BUG][Data Explorer][Discover] Allow data grid to auto adjust size based on fetched data count (#5191) * This PR adds a new rows state to the DiscoverCanvas component and updated it whenever the data$ observable emitted new row data. * The DiscoverTable component was then refactored to accept rows as a prop, making it dependent on the parent component to provide the correct set of data. This ensures that the table renders correctly based on the current data and doesn't rely on its internal state, which could be outdated. Issue Resolve https://github.com/opensearch-project/OpenSearch-Dashboards/issues/5181 Signed-off-by: ananzh (cherry picked from commit 677fdf53052c6e116e5daf0cad98d73345116517) Signed-off-by: Miki --- .../components/data_grid/data_grid_table.tsx | 1 - .../view_components/canvas/discover_table.tsx | 31 +++---------- .../view_components/canvas/index.tsx | 43 +++++++++++++------ 3 files changed, 35 insertions(+), 40 deletions(-) diff --git a/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx b/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx index 29b787726d5b..fcb786b23e86 100644 --- a/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx +++ b/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx @@ -29,7 +29,6 @@ export interface DataGridTableProps { onSetColumns: (columns: string[]) => void; sort: SortOrder[]; displayTimeColumn: boolean; - services: DiscoverServices; title?: string; description?: string; isToolbarVisible?: boolean; 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 index e57a0b7c7668..a7db2ce3034f 100644 --- a/src/plugins/discover/public/application/view_components/canvas/discover_table.tsx +++ b/src/plugins/discover/public/application/view_components/canvas/discover_table.tsx @@ -3,8 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React, { useState, useEffect, useCallback, useMemo } from 'react'; -import { History } from 'history'; +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'; @@ -17,17 +16,18 @@ import { useDispatch, useSelector, } from '../../utils/state_management'; -import { ResultStatus, SearchData, useSearch } from '../utils/use_search'; +import { useSearch } from '../utils/use_search'; import { IndexPatternField, opensearchFilters } from '../../../../../data/public'; import { DocViewFilterFn } from '../../doc_views/doc_views_types'; import { SortOrder } from '../../../saved_searches/types'; import { DOC_HIDE_TIME_COLUMN_SETTING } from '../../../../common'; +import { OpenSearchSearchHit } from '../../doc_views/doc_views_types'; interface Props { - history: History; + rows: OpenSearchSearchHit[]; } -export const DiscoverTable = ({ history }: Props) => { +export const DiscoverTable = ({ rows }: Props) => { const { services } = useOpenSearchDashboards(); const { uiSettings, @@ -35,12 +35,8 @@ export const DiscoverTable = ({ history }: Props) => { query: { filterManager }, }, } = services; - const { data$, refetch$, indexPattern, savedSearch } = useDiscoverContext(); - const [fetchState, setFetchState] = useState({ - status: data$.getValue().status, - rows: [], - }); + const { refetch$, indexPattern, savedSearch } = useDiscoverContext(); const { columns, sort } = useSelector((state) => state.discover); const dispatch = useDispatch(); const onAddColumn = (col: string) => dispatch(addColumn({ column: col })); @@ -70,20 +66,6 @@ export const DiscoverTable = ({ history }: Props) => { [indexPattern, uiSettings] ); - const { rows } = fetchState || {}; - - useEffect(() => { - const subscription = data$.subscribe((next) => { - if (next.status === ResultStatus.LOADING) return; - if (next.status !== fetchState.status || (next.rows && next.rows !== fetchState.rows)) { - setFetchState({ ...fetchState, ...next }); - } - }); - return () => { - subscription.unsubscribe(); - }; - }, [data$, fetchState]); - if (indexPattern === undefined) { // TODO: handle better return null; @@ -106,7 +88,6 @@ export const DiscoverTable = ({ history }: Props) => { sort={sort} rows={rows} displayTimeColumn={displayTimeColumn} - services={services} title={savedSearch?.id ? savedSearch.title : ''} description={savedSearch?.id ? savedSearch.description : ''} /> diff --git a/src/plugins/discover/public/application/view_components/canvas/index.tsx b/src/plugins/discover/public/application/view_components/canvas/index.tsx index 7be8cc8585c0..1d994c92a2ea 100644 --- a/src/plugins/discover/public/application/view_components/canvas/index.tsx +++ b/src/plugins/discover/public/application/view_components/canvas/index.tsx @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React, { useEffect, useState, useRef, useCallback } from 'react'; +import React, { useEffect, useState, useRef, useCallback, useMemo } from 'react'; import { EuiPanel } from '@elastic/eui'; import { TopNav } from './top_nav'; import { ViewProps } from '../../../../../data_explorer/public'; @@ -19,6 +19,7 @@ import { DiscoverViewServices } from '../../../build_services'; import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_react/public'; import { filterColumns } from '../utils/filter_columns'; import { DEFAULT_COLUMNS_SETTING } from '../../../../common'; +import { OpenSearchSearchHit } from '../../../application/doc_views/doc_views_types'; import './discover_canvas.scss'; // eslint-disable-next-line import/no-default-export @@ -42,7 +43,6 @@ export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewPro bucketInterval: {}, }); - const { status } = fetchState; const onQuerySubmit = useCallback( (payload, isUpdate) => { if (isUpdate === false) { @@ -51,18 +51,30 @@ export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewPro }, [refetch$] ); + const [rows, setRows] = useState(undefined); useEffect(() => { const subscription = data$.subscribe((next) => { - if ( - next.status !== fetchState.status || - (next.hits && next.hits !== fetchState.hits) || - (next.bucketInterval && next.bucketInterval !== fetchState.bucketInterval) || - (next.chartData && next.chartData !== fetchState.chartData) - ) { + if (next.status === ResultStatus.LOADING) return; + + let shouldUpdateState = false; + + if (next.status !== fetchState.status) shouldUpdateState = true; + if (next.hits && next.hits !== fetchState.hits) shouldUpdateState = true; + if (next.bucketInterval && next.bucketInterval !== fetchState.bucketInterval) + shouldUpdateState = true; + 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) { setFetchState({ ...fetchState, ...next }); } }); + return () => { subscription.unsubscribe(); }; @@ -77,6 +89,9 @@ export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewPro const timeField = indexPattern?.timeFieldName ? indexPattern.timeFieldName : undefined; + const MemoizedDiscoverTable = React.memo(DiscoverTable); + const MemoizedDiscoverChartContainer = React.memo(DiscoverChartContainer); + return ( - {status === ResultStatus.NO_RESULTS && ( + {fetchState.status === ResultStatus.NO_RESULTS && ( )} - {status === ResultStatus.UNINITIALIZED && ( + {fetchState.status === ResultStatus.UNINITIALIZED && ( refetch$.next()} /> )} - {status === ResultStatus.LOADING && } - {status === ResultStatus.READY && ( + {fetchState.status === ResultStatus.LOADING && } + {fetchState.status === ResultStatus.READY && ( <> - + - + )}