[VisBuilder][BUG] Flat render structure in Metric and Table Vis (open…

…search-project#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: opensearch-project#6671 Signed-off-by: Anan Zhuang <ananzh@amazon.com> * Changeset file for PR opensearch-project#6674 created/updated --------- Signed-off-by: Anan Zhuang <ananzh@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
RyanL1997 · Jul 24, 2024 · 83317a2 · 83317a2
1 parent f0a1307
commit 83317a2
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);