From 582d90f5b9211f40c851146bed7a58ae20945f21 Mon Sep 17 00:00:00 2001 From: Anan Zhuang Date: Wed, 24 Jul 2024 11:14:27 -0700 Subject: [PATCH] [VisBuilder][BUG] Flat render structure in Metric and Table Vis (#6674) * [VisBuilder][BUG] Flat render structure in Metric and Table Vis This issue is caused by the callback behavior in ReactExpressionRenderer. The callback to ReactExpressionRenderer to update isloading state is lost if we wrap the render vis with VisualizationContainer. This PR moved VisualizationContainer directly in MetricVisComponent. When put VisualizationContainer directly in the MetricVisComponent, all the lifecycle methods and hooks within MetricVisComponent directly influence the rendering of VisualizationContainer. This means that calls to renderComplete and other lifecycle integrations are more directly managed. Issue Resolved: https://github.com/opensearch-project/OpenSearch-Dashboards/issues/6671 Signed-off-by: Anan Zhuang * Changeset file for PR #6674 created/updated --------- Signed-off-by: Anan Zhuang Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> --- changelogs/fragments/6674.yml | 2 + .../metric_vis_component.test.tsx.snap | 42 +++++++----- .../components/metric_vis_component.tsx | 13 ++-- .../public/metric_vis_renderer.tsx | 20 +++--- .../public/components/table_vis_app.test.tsx | 67 ++++++++++++++----- .../public/components/table_vis_app.tsx | 48 +++++++------ .../public/table_vis_renderer.tsx | 8 +-- .../public/utils/get_table_ui_state.ts | 4 +- 8 files changed, 123 insertions(+), 81 deletions(-) create mode 100644 changelogs/fragments/6674.yml diff --git a/changelogs/fragments/6674.yml b/changelogs/fragments/6674.yml new file mode 100644 index 00000000000..7a559d02fa5 --- /dev/null +++ b/changelogs/fragments/6674.yml @@ -0,0 +1,2 @@ +fix: +- [VisBuilder][BUG] Flat render structure in Metric and Table Vis ([#6674](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6674)) \ No newline at end of file diff --git a/src/plugins/vis_type_metric/public/components/__snapshots__/metric_vis_component.test.tsx.snap b/src/plugins/vis_type_metric/public/components/__snapshots__/metric_vis_component.test.tsx.snap index f07fdfa682d..27c9507a549 100644 --- a/src/plugins/vis_type_metric/public/components/__snapshots__/metric_vis_component.test.tsx.snap +++ b/src/plugins/vis_type_metric/public/components/__snapshots__/metric_vis_component.test.tsx.snap @@ -1,7 +1,10 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`MetricVisComponent should render correct structure for multi-value metrics 1`] = ` -Array [ + , + /> , -] + /> + `; exports[`MetricVisComponent should render correct structure for single metric 1`] = ` - + + showLabel={true} + /> + `; diff --git a/src/plugins/vis_type_metric/public/components/metric_vis_component.tsx b/src/plugins/vis_type_metric/public/components/metric_vis_component.tsx index 8162eb7d197..42e46054b9d 100644 --- a/src/plugins/vis_type_metric/public/components/metric_vis_component.tsx +++ b/src/plugins/vis_type_metric/public/components/metric_vis_component.tsx @@ -40,6 +40,7 @@ import { VisParams, MetricVisMetric } from '../types'; import { getFormatService } from '../services'; import { SchemaConfig } from '../../../visualizations/public'; import { Range } from '../../../expressions/public'; +import { VisualizationContainer } from '../../../visualizations/public'; import './metric_vis.scss'; @@ -214,12 +215,12 @@ class MetricVisComponent extends Component { } render() { - let metricsHtml; - if (this.props.visData) { - const metrics = this.processTableGroups(this.props.visData); - metricsHtml = metrics.map(this.renderMetric); - } - return metricsHtml; + const metrics = this.props.visData.rows ? this.processTableGroups(this.props.visData) : []; + return ( + + {metrics.length > 0 ? metrics.map(this.renderMetric) : null} + + ); } } diff --git a/src/plugins/vis_type_metric/public/metric_vis_renderer.tsx b/src/plugins/vis_type_metric/public/metric_vis_renderer.tsx index dcd3fcd0a72..14b0c28f2c5 100644 --- a/src/plugins/vis_type_metric/public/metric_vis_renderer.tsx +++ b/src/plugins/vis_type_metric/public/metric_vis_renderer.tsx @@ -28,14 +28,14 @@ * under the License. */ -import React, { lazy } from 'react'; +import React from 'react'; import { render, unmountComponentAtNode } from 'react-dom'; -import { VisualizationContainer } from '../../visualizations/public'; import { ExpressionRenderDefinition } from '../../expressions/common/expression_renderers'; import { MetricVisRenderValue } from './metric_vis_fn'; +import MetricVisComponent from './components/metric_vis_component'; + // @ts-ignore -const MetricVisComponent = lazy(() => import('./components/metric_vis_component')); export const metricVisRenderer: () => ExpressionRenderDefinition = () => ({ name: 'metric_vis', @@ -47,14 +47,12 @@ export const metricVisRenderer: () => ExpressionRenderDefinition - - , + , domNode ); }, diff --git a/src/plugins/vis_type_table/public/components/table_vis_app.test.tsx b/src/plugins/vis_type_table/public/components/table_vis_app.test.tsx index f2a347dcd0a..58e38a34636 100644 --- a/src/plugins/vis_type_table/public/components/table_vis_app.test.tsx +++ b/src/plugins/vis_type_table/public/components/table_vis_app.test.tsx @@ -9,7 +9,7 @@ import { coreMock } from '../../../../core/public/mocks'; import { IInterpreterRenderHandlers } from 'src/plugins/expressions'; import { TableVisApp } from './table_vis_app'; import { TableVisConfig } from '../types'; -import { TableVisData } from '../table_vis_response_handler'; +import { TableVisData, FormattedTableContext } from '../table_vis_response_handler'; jest.mock('./table_vis_component_group', () => ({ TableVisComponentGroup: () => ( @@ -42,15 +42,24 @@ describe('TableVisApp', () => { } as unknown) as IInterpreterRenderHandlers; const visConfigMock = ({} as unknown) as TableVisConfig; - it('should render TableVisComponent if no split table', () => { - const visDataMock = { - table: { - columns: [], - rows: [], - formattedColumns: [], + const createMockFormattedTableContext = (rowCount: number): FormattedTableContext => ({ + columns: [{ id: 'col1', name: 'Column 1' }], + rows: Array(rowCount).fill({ col1: 'value' }), + formattedColumns: [ + { + id: 'col1', + title: 'Column 1', + formatter: {} as any, + filterable: true, }, + ], + }); + + it('should render TableVisComponent if no split table and has rows', () => { + const visDataMock: TableVisData = { + table: createMockFormattedTableContext(1), tableGroups: [], - } as TableVisData; + }; const { getByTestId } = render( { }); it('should render TableVisComponentGroup component if split direction is column', () => { - const visDataMock = { - tableGroups: [], + const visDataMock: TableVisData = { + tableGroups: [ + { + table: createMockFormattedTableContext(1), + title: 'Group 1', + }, + ], direction: 'column', - } as TableVisData; + }; const { container, getByTestId } = render( { handlers={handlersMock} /> ); - expect(container.outerHTML.includes('visTable')).toBe(true); + expect(container.querySelector('.visTable')).not.toBeNull(); expect(getByTestId('TableVisComponentGroup')).toBeInTheDocument(); }); it('should render TableVisComponentGroup component if split direction is row', () => { - const visDataMock = { - tableGroups: [], + const visDataMock: TableVisData = { + tableGroups: [ + { + table: createMockFormattedTableContext(1), + title: 'Group 1', + }, + ], direction: 'row', - } as TableVisData; + }; const { container, getByTestId } = render( { handlers={handlersMock} /> ); - expect(container.outerHTML.includes('visTable')).toBe(true); + expect(container.querySelector('.visTable')).not.toBeNull(); expect(getByTestId('TableVisComponentGroup')).toBeInTheDocument(); }); + + it('should render "No results found" when there are no rows', () => { + const visDataMock: TableVisData = { + table: createMockFormattedTableContext(0), + tableGroups: [], + }; + const { getByText } = render( + + ); + expect(getByText('No results found')).toBeInTheDocument(); + }); }); diff --git a/src/plugins/vis_type_table/public/components/table_vis_app.tsx b/src/plugins/vis_type_table/public/components/table_vis_app.tsx index cbde913452d..ccdab8cdaf8 100644 --- a/src/plugins/vis_type_table/public/components/table_vis_app.tsx +++ b/src/plugins/vis_type_table/public/components/table_vis_app.tsx @@ -16,6 +16,7 @@ import { TableVisConfig } from '../types'; import { TableVisComponent } from './table_vis_component'; import { TableVisComponentGroup } from './table_vis_component_group'; import { getTableUIState, TableUiState } from '../utils'; +import { VisualizationContainer } from '../../../visualizations/public'; interface TableVisAppProps { services: CoreStart; @@ -36,32 +37,35 @@ export const TableVisApp = ({ }, [handlers]); const tableUiState: TableUiState = getTableUIState(handlers.uiState as PersistedState); + const showNoResult = table ? table.rows.length === 0 : tableGroups?.length === 0; return ( - - {table ? ( - - ) : ( - - )} - + + + {table ? ( + + ) : ( + + )} + + ); diff --git a/src/plugins/vis_type_table/public/table_vis_renderer.tsx b/src/plugins/vis_type_table/public/table_vis_renderer.tsx index 8e467112528..01b995e64fb 100644 --- a/src/plugins/vis_type_table/public/table_vis_renderer.tsx +++ b/src/plugins/vis_type_table/public/table_vis_renderer.tsx @@ -7,7 +7,6 @@ import React from 'react'; import { render, unmountComponentAtNode } from 'react-dom'; import { CoreStart } from 'opensearch-dashboards/public'; -import { VisualizationContainer } from '../../visualizations/public'; import { ExpressionRenderDefinition } from '../../expressions/common/expression_renderers'; import { TableVisRenderValue } from './table_vis_fn'; import { TableVisApp } from './components/table_vis_app'; @@ -23,13 +22,8 @@ export const getTableVisRenderer: ( unmountComponentAtNode(domNode); }); - const showNoResult = visData.table - ? visData.table.rows.length === 0 - : visData.tableGroups?.length === 0; render( - - - , + , domNode ); }, diff --git a/src/plugins/vis_type_table/public/utils/get_table_ui_state.ts b/src/plugins/vis_type_table/public/utils/get_table_ui_state.ts index 58fc6b472a4..67c1a1864f4 100644 --- a/src/plugins/vis_type_table/public/utils/get_table_ui_state.ts +++ b/src/plugins/vis_type_table/public/utils/get_table_ui_state.ts @@ -14,8 +14,8 @@ export interface TableUiState { } export function getTableUIState(uiState: PersistedState): TableUiState { - const sort: ColumnSort = uiState.get('vis.sortColumn') || {}; - const colWidth: ColumnWidth[] = uiState.get('vis.columnsWidth') || []; + const sort: ColumnSort = uiState?.get('vis.sortColumn') || {}; + const colWidth: ColumnWidth[] = uiState?.get('vis.columnsWidth') || []; const setSort = (newSort: ColumnSort) => { uiState.set('vis.sortColumn', newSort);