From c4368c24e29a4ea5c291209b408624e20a959d43 Mon Sep 17 00:00:00 2001
From: Anan Zhuang <ananzh@amazon.com>
Date: Mon, 29 Apr 2024 16:12:47 +0000
Subject: [PATCH] [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 <ananzh@amazon.com>
---
 .../components/metric_vis_component.tsx       | 13 ++---
 .../public/metric_vis_renderer.tsx            | 15 +++---
 .../public/components/table_vis_app.tsx       | 48 ++++++++++---------
 .../public/table_vis_renderer.tsx             |  8 +---
 4 files changed, 40 insertions(+), 44 deletions(-)

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<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
index dcd3fcd0a727..eabcd5fe2f6c 100644
--- a/src/plugins/vis_type_metric/public/metric_vis_renderer.tsx
+++ b/src/plugins/vis_type_metric/public/metric_vis_renderer.tsx
@@ -31,7 +31,6 @@
 import React, { lazy } 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';
 // @ts-ignore
@@ -47,14 +46,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
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 (
     <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
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(
-      <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
     );
   },