[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 <[email protected]>
ananzh · Jul 23, 2024 · 9d8bca7 · 9d8bca7
1 parent c8496f8
commit 9d8bca7
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 65 deletions.
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.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);