From 98caa31d070679a61c7aba32ce3db80e8435480a Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Wed, 24 Jul 2024 13:58:56 -0700 Subject: [PATCH] [VisBuilder][BUG] Flat render structure in Metric and Table Vis (#6674) (#7447) * [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 * Changeset file for PR #6674 created/updated --------- (cherry picked from commit 83317a231212d6770da9adadc20e40f65b1136dc) Signed-off-by: Anan Zhuang Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] 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 000000000000..7a559d02fa55 --- /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 f07fdfa682d8..27c9507a5493 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 8162eb7d1978..42e46054b9d8 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 dcd3fcd0a727..14b0c28f2c56 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 f2a347dcd0a2..58e38a346365 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 cbde913452d1..ccdab8cdaf83 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 8e467112528d..01b995e64fbc 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 58fc6b472a40..67c1a1864f4b 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);