From 677fdf53052c6e116e5daf0cad98d73345116517 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 --- CHANGELOG.md | 1 + .../components/data_grid/data_grid_table.tsx | 1 - .../view_components/canvas/discover_table.tsx | 31 +++---------- .../view_components/canvas/index.tsx | 43 +++++++++++++------ 4 files changed, 36 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9210c679f17..58aa2e13207b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [BUG][Discover] Fix misc navigation issues ([#5168](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5168)) - [BUG][Discover] Fix mobile view ([#5168](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5168)) - [BUG][Data Explorer][Discover] Automatically load solo added default index pattern ([#5171](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5171)) +- [BUG][Data Explorer][Discover] Allow data grid to auto adjust size based on fetched data count ([#5191](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5191)) ### 🚞 Infrastructure 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 298643e2a2b7..b285b4739361 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 && ( <> - + - + )}