Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for test app crashes for 12 schema data #33542

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+70 −32
Open

Fixes for test app crashes for 12 schema data #33542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4de140a
Fixes for test app crashes for 12 schema data
srmukher Jan 2, 2025
3314b1a
Adding change file
srmukher Jan 2, 2025
60aaf97
Default behaviour in case of unsopprted schema type
srmukher Jan 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions change/@fluentui-react-charting-f8f33f53-e3ef-4918-acb0-1b22073e4a82.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Fixes for test app crashes for 12 schema data",
"packageName": "@fluentui/react-charting",
"email": "[email protected]",
"dependentChangeType": "patch"
}
82 changes: 53 additions & 29 deletions packages/charts/react-charting/src/components/DeclarativeChart/DeclarativeChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export const DeclarativeChart: React.FunctionComponent<DeclarativeChartProps> =
const { data, layout } = plotlySchema;
let { selectedLegends } = plotlySchema;
const xValues = data[0].x;
const yValues = data[0].y;
const isXDate = isDateArray(xValues);
const isXNumber = isNumberArray(xValues);
const isXMonth = isMonthArray(xValues);
Expand Down Expand Up @@ -148,6 +149,40 @@ export const DeclarativeChart: React.FunctionComponent<DeclarativeChartProps> =
selectedLegends: activeLegends,
};

const checkAndRenderChart = (renderChart: any, isAreaChart: boolean = false) => {
if (isXDate || isXNumber) {
const chartProps = {
...transformPlotlyJsonToScatterChartProps({ data, layout }, isAreaChart, colorMap, isDarkTheme),
legendProps,
componentRef: chartRef,
calloutProps: { layerProps: { eventBubblingEnabled: true } },
};
return renderChart(chartProps);
} else if (isXMonth) {
const updatedData = data.map((dataPoint: any) => ({
...dataPoint,
x: updateXValues(dataPoint.x),
}));
const chartProps = {
...transformPlotlyJsonToScatterChartProps({ data: updatedData, layout }, isAreaChart, colorMap, isDarkTheme),
legendProps,
componentRef: chartRef,
calloutProps: { layerProps: { eventBubblingEnabled: true } },
};
return renderChart(chartProps);
}
// Unsupported schema, render as VerticalStackedBarChart
data[0].type = 'Unsupported Schema';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsupported

how can unsupported schema be rendered as vertical bar chart?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we discussed that only right, plotly falls back to line chart in case x and y axis are defined which in turn falls back to VSBC.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. See the other comment

return (
<VerticalStackedBarChart
{...transformPlotlyJsonToVSBCProps(plotlySchema, colorMap, isDarkTheme)}
legendProps={multiSelectLegendProps}
componentRef={chartRef}
calloutProps={{ layerProps: { eventBubblingEnabled: true } }}
/>
);
};

switch (data[0].type) {
case 'pie':
return (
Expand Down Expand Up @@ -214,35 +249,7 @@ export const DeclarativeChart: React.FunctionComponent<DeclarativeChartProps> =
/>
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can refactor to move this code also to the check and render function

};
if (isXDate || isXNumber) {
const chartProps = {
...transformPlotlyJsonToScatterChartProps({ data, layout }, isAreaChart, colorMap, isDarkTheme),
legendProps,
componentRef: chartRef,
calloutProps: { layerProps: { eventBubblingEnabled: true } },
};
return renderChart(chartProps);
} else if (isXMonth) {
const updatedData = data.map((dataPoint: any) => ({
...dataPoint,
x: updateXValues(dataPoint.x),
}));
const chartProps = {
...transformPlotlyJsonToScatterChartProps({ data: updatedData, layout }, isAreaChart, colorMap, isDarkTheme),
legendProps,
componentRef: chartRef,
calloutProps: { layerProps: { eventBubblingEnabled: true } },
};
return renderChart(chartProps);
}
return (
<VerticalStackedBarChart
{...transformPlotlyJsonToVSBCProps(plotlySchema, colorMap, isDarkTheme)}
legendProps={multiSelectLegendProps}
componentRef={chartRef}
calloutProps={{ layerProps: { eventBubblingEnabled: true } }}
/>
);
return checkAndRenderChart(renderChart, isAreaChart);
case 'heatmap':
return (
<HeatMapChart
Expand Down Expand Up @@ -281,6 +288,23 @@ export const DeclarativeChart: React.FunctionComponent<DeclarativeChartProps> =
/>
);
default:
if (xValues.length > 0 && yValues.length > 0) {
const renderLineChart = (chartProps: any) => {
return (
<LineChart
{...{
...chartProps,
legendProps: {
onChange: onActiveLegendsChange,
canSelectMultipleLegends: true,
selectedLegends: activeLegends,
},
}}
/>
);
};
return checkAndRenderChart(renderLineChart);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkAndRenderChart

which chart will handle in case x and y values are not defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in case x and y are not defined, it will throw new Error('Unsupported chart schema');

}
throw new Error('Unsupported chart schema');
}
});
Expand Down
13 changes: 10 additions & 3 deletions packages/charts/react-charting/src/components/DeclarativeChart/PlotlySchemaAdapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export const updateXValues = (xValues: any[]): any[] => {
});
return xValues;
};

export const getColor = (
legendLabel: string,
colorMap: React.MutableRefObject<Map<string, string>>,
Expand Down Expand Up @@ -143,7 +144,7 @@ export const transformPlotlyJsonToVSBCProps = (
mapXToDataPoints[x] = { xAxisPoint: x, chartData: [], lineData: [] };
}
const legend: string = series.name || `Series ${index1 + 1}`;
if (series.type === 'bar' || series.type === 'scatter') {
if (series.type === 'bar' || series.type === 'scatter' || series.type === 'Unsupported Schema') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsupported Schema

which sample schema has this value as type

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This I am setting in Declarative chart for any type that is not supported for uniformity.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is confusing. call it fallback-vsbc

const color = getColor(legend, colorMap, isDarkTheme);
mapXToDataPoints[x].chartData.push({
legend,
Expand Down Expand Up @@ -414,9 +415,15 @@ export const transformPlotlyJsonToHeatmapProps = (jsonObj: any): IHeatMapChartPr

// Convert normalized values to actual values
const domainValuesForColorScale: number[] = firstData.colorscale
? firstData.colorscale.map((arr: any) => arr[0] * (zMax - zMin) + zMin)
? isArrayOrTypedArray(firstData.colorscale)
? firstData.colorscale.map((arr: any) => arr[0] * (zMax - zMin) + zMin)
: []
: [];
const rangeValuesForColorScale: string[] = firstData.colorscale
? isArrayOrTypedArray(firstData.colorscale)
? firstData.colorscale.map((arr: any) => arr[1])
: []
: [];
const rangeValuesForColorScale: string[] = firstData.colorscale ? firstData.colorscale.map((arr: any) => arr[1]) : [];

return {
data: [heatmapData],
Expand Down
Loading