Skip to content

Commit

Permalink
[VisBuilder][BUG] Flat render structure in Metric and Table Vis
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
ananzh committed Apr 29, 2024
1 parent 460428c commit c4368c2
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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>
);
}
}

Expand Down
15 changes: 6 additions & 9 deletions src/plugins/vis_type_metric/public/metric_vis_renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
);
},
Expand Down
48 changes: 26 additions & 22 deletions src/plugins/vis_type_table/public/components/table_vis_app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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>
);
Expand Down
8 changes: 1 addition & 7 deletions src/plugins/vis_type_table/public/table_vis_renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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
);
},
Expand Down

0 comments on commit c4368c2

Please sign in to comment.