[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: #6671 * Changeset file for PR #6674 created/updated --------- (cherry picked from commit 83317a2) Signed-off-by: Anan Zhuang <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
opensearch-project · Jul 24, 2024 · 98caa31 · 98caa31
1 parent 0b7b75f
commit 98caa31
Show file tree

Hide file tree

Showing 8 changed files with 123 additions and 81 deletions.
diff --git a/changelogs/fragments/6674.yml 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))
diff --git a/...lugins/vis_type_metric/public/components/__snapshots__/metric_vis_component.test.tsx.snap b/...lugins/vis_type_metric/public/components/__snapshots__/metric_vis_component.test.tsx.snap
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
@@ -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<MetricVisComponentProps> {
   }
 
   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 (
+      <VisualizationContainer className="mtrVis" showNoResult={metrics.length === 0}>
+        {metrics.length > 0 ? metrics.map(this.renderMetric) : null}
+      </VisualizationContainer>
+    );
   }
 }
 

diff --git 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<MetricVisRenderValue> = () => ({
   name: 'metric_vis',
@@ -47,14 +47,12 @@ export const metricVisRenderer: () => ExpressionRenderDefinition<MetricVisRender
     });
 
     render(
-      <VisualizationContainer className="mtrVis" showNoResult={!visData.rows?.length}>
-        <MetricVisComponent
-          visData={visData}
-          visParams={visConfig}
-          renderComplete={handlers.done}
-          fireEvent={handlers.event}
-        />
-      </VisualizationContainer>,
+      <MetricVisComponent
+        visData={visData}
+        visParams={visConfig}
+        renderComplete={handlers.done}
+        fireEvent={handlers.event}
+      />,
       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
@@ -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(
       <TableVisApp
         services={serviceMock}
@@ -63,10 +72,15 @@ describe('TableVisApp', () => {
   });
 
   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(
       <TableVisApp
         services={serviceMock}
@@ -75,15 +89,20 @@ describe('TableVisApp', () => {
         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(
       <TableVisApp
         services={serviceMock}
@@ -92,7 +111,23 @@ describe('TableVisApp', () => {
         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(
+      <TableVisApp
+        services={serviceMock}
+        visData={visDataMock}
+        visConfig={visConfigMock}
+        handlers={handlersMock}
+      />
+    );
+    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
@@ -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 (
     <I18nProvider>
       <OpenSearchDashboardsContextProvider services={services}>
-        <EuiFlexGroup
-          className="visTable"
-          data-test-subj="visTable"
-          direction={direction === 'column' ? 'row' : 'column'}
-          alignItems={direction === 'column' ? 'flexStart' : 'stretch'}
-        >
-          {table ? (
-            <TableVisComponent
-              table={table}
-              visConfig={visConfig}
-              event={handlers.event}
-              uiState={tableUiState}
-            />
-          ) : (
-            <TableVisComponentGroup
-              tableGroups={tableGroups}
-              visConfig={visConfig}
-              event={handlers.event}
-              uiState={tableUiState}
-            />
-          )}
-        </EuiFlexGroup>
+        <VisualizationContainer className="tableVis" showNoResult={showNoResult}>
+          <EuiFlexGroup
+            className="visTable"
+            data-test-subj="visTable"
+            direction={direction === 'column' ? 'row' : 'column'}
+            alignItems={direction === 'column' ? 'flexStart' : 'stretch'}
+          >
+            {table ? (
+              <TableVisComponent
+                table={table}
+                visConfig={visConfig}
+                event={handlers.event}
+                uiState={tableUiState}
+              />
+            ) : (
+              <TableVisComponentGroup
+                tableGroups={tableGroups}
+                visConfig={visConfig}
+                event={handlers.event}
+                uiState={tableUiState}
+              />
+            )}
+          </EuiFlexGroup>
+        </VisualizationContainer>
       </OpenSearchDashboardsContextProvider>
     </I18nProvider>
   );

diff --git 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(
-      <VisualizationContainer className="tableVis" showNoResult={showNoResult}>
-        <TableVisApp services={core} visData={visData} visConfig={visConfig} handlers={handlers} />
-      </VisualizationContainer>,
+      <TableVisApp services={core} visData={visData} visConfig={visConfig} handlers={handlers} />,
       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
@@ -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);
Original file line number	Diff line number	Diff line change
		@@ -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))