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.

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

fix(react-charting): resolve bugs in declarative chart #33564

Merged
Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
krkshitij marked this conversation as resolved.
Show resolved Hide resolved
"type": "patch",
"comment": "fix: resolve bugs in declarative chart",
"packageName": "@fluentui/react-charting",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
transformPlotlyJsonToGaugeProps,
transformPlotlyJsonToGVBCProps,
transformPlotlyJsonToVBCProps,
isLineData,
} from './PlotlySchemaAdapter';
import { LineChart, ILineChartProps } from '../LineChart/index';
import { HorizontalBarChartWithAxis } from '../HorizontalBarChartWithAxis/index';
Expand Down Expand Up @@ -178,6 +179,7 @@ export const DeclarativeChart: React.FunctionComponent<DeclarativeChartProps> =
(opts?: IImageExportOptions) => {
return toImage(chartRef.current?.chartContainer, {
background: theme.semanticColors.bodyBackground,
scale: 3,
...opts,
});
},
Expand Down Expand Up @@ -205,7 +207,10 @@ export const DeclarativeChart: React.FunctionComponent<DeclarativeChartProps> =
/>
);
} else {
if (['group', 'overlay'].includes(plotlySchema?.layout?.barmode)) {
const containsLines = plotlyInput.data.some(
series => series.type === 'scatter' || isLineData(series as Partial<PlotData>),
);
if (['group', 'overlay'].includes(plotlySchema?.layout?.barmode) && !containsLines) {
return (
<GroupedVerticalBarChart
{...transformPlotlyJsonToGVBCProps(plotlySchema, colorMap, isDarkTheme)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ interface ISecondaryYAxisValues {
secondaryYScaleOptions?: { yMinValue?: number; yMaxValue?: number };
}

const SUPPORTED_PLOT_TYPES = ['pie', 'bar', 'scatter', 'heatmap', 'sankey', 'indicator', 'histogram'];
krkshitij marked this conversation as resolved.
Show resolved Hide resolved

const isDate = (value: any): boolean => !isNaN(Date.parse(value));
const isNumber = (value: any): boolean => !isNaN(parseFloat(value)) && isFinite(value);

Expand All @@ -49,24 +51,24 @@ const isMonth = (possiblyMonthValue: any, presentYear: number): boolean => {
};

const isArrayOfType = (
data: Datum[] | Datum[][] | TypedArray,
plotCoordinates: Datum[] | Datum[][] | TypedArray | undefined,
typeCheck: (datum: any, ...args: any[]) => boolean,
...args: any[]
): boolean => {
if (!isArrayOrTypedArray(data)) {
if (!isArrayOrTypedArray(plotCoordinates)) {
return false;
}

if (data.length === 0) {
if (plotCoordinates!.length === 0) {
return false;
}

if (Array.isArray(data[0])) {
if (Array.isArray(plotCoordinates![0])) {
// Handle 2D array
return (data as Datum[][]).every(innerArray => innerArray.every(datum => typeCheck(datum, ...args)));
return (plotCoordinates as Datum[][]).every(innerArray => innerArray.every(datum => typeCheck(datum, ...args)));
} else {
// Handle 1D array
return (data as Datum[]).every(datum => typeCheck(datum, ...args));
return (plotCoordinates as Datum[]).every(datum => typeCheck(datum, ...args));
}
};

Expand All @@ -83,6 +85,16 @@ export const isMonthArray = (data: Datum[] | Datum[][] | TypedArray): boolean =>
return isArrayOfType(data, isMonth, presentYear);
};

export const isLineData = (data: Partial<PlotData>): boolean => {
return (
!SUPPORTED_PLOT_TYPES.includes(`${data.type}`) &&
Array.isArray(data.x) &&
isArrayOfType(data.y, (value: any) => typeof value === 'number') &&
data.x.length > 0 &&
data.x.length === data.y!.length
);
};

const invalidate2Dseries = (series: PlotData, chartType: string): void => {
if (series.x?.length > 0 && Array.isArray(series.x[0])) {
throw new Error(`transform to ${chartType}:: 2D x array not supported`);
Expand Down Expand Up @@ -252,21 +264,21 @@ export const transformPlotlyJsonToVSBCProps = (
}
const legend: string = getLegend(series, index1);
const yVal: number = (series.y?.[index2] as number) ?? 0;
if (series.type === 'bar' || series.type === 'scatter' || !!fallbackVSBC) {
if (series.type === 'bar') {
const color = getColor(legend, colorMap, isDarkTheme);
mapXToDataPoints[x].chartData.push({
legend,
data: yVal,
color,
});
} /* else if (series.type === 'line') { //ToDo - Fix this as series.type cannot be line type
} else if (series.type === 'scatter' || isLineData(series) || !!fallbackVSBC) {
const color = getColor(legend, colorMap, isDarkTheme);
mapXToDataPoints[x].lineData!.push({
legend,
y: yVal,
color,
});
} */
}

yMaxValue = Math.max(yMaxValue, yVal);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4142,7 +4142,7 @@ exports[`DeclarativeChart Should render gaugechart in DeclarativeChart 1`] = `
transform="rotate(151.2, 0, 0)"
>
<path
aria-label="Current value: 420"
aria-label="Current value: 84%"
class=

{
Expand Down Expand Up @@ -4180,7 +4180,7 @@ exports[`DeclarativeChart Should render gaugechart in DeclarativeChart 1`] = `
x="0"
y="0"
>
420
84%
</text>
</g>
<text
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,8 @@ Object {
},
],
"mode": "plotly",
"secondaryYAxistitle": undefined,
"secondaryYScaleOptions": undefined,
"xAxisTitle": "Votes",
"yAxisTitle": "Framework",
}
Expand All @@ -366,7 +368,6 @@ exports[`transform Plotly Json To chart Props transformPlotlyJsonToGaugeProps -
Object {
"chartTitle": "",
"chartValue": 420,
"chartValueFormat": [Function],
"height": 400,
"maxValue": 500,
"minValue": undefined,
Expand Down Expand Up @@ -2800,7 +2801,7 @@ Object {
"y": "y_19",
},
],
"legend": "",
"legend": undefined,
"value": 0,
},
],
Expand Down Expand Up @@ -3324,6 +3325,8 @@ Object {
},
],
},
"secondaryYAxistitle": undefined,
"secondaryYScaleOptions": undefined,
"supportNegativeData": true,
"xAxisTitle": "",
"yAxisTitle": "",
Expand Down Expand Up @@ -4555,6 +4558,8 @@ Object {
},
],
},
"secondaryYAxistitle": undefined,
"secondaryYScaleOptions": undefined,
"supportNegativeData": true,
"xAxisTitle": "",
"yAxisTitle": "",
Expand Down Expand Up @@ -4927,6 +4932,8 @@ Object {
},
],
"mode": "plotly",
"secondaryYAxistitle": undefined,
"secondaryYScaleOptions": undefined,
"xAxisTitle": "",
"yAxisTitle": "",
"yMaxValue": 3400,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2505,7 +2505,7 @@ exports[`Vertical bar chart re-rendering Should re-render the vertical bar chart
fill="currentColor"
y="16"
>
10,000
10000
</text>
</g>
<g
Expand All @@ -2523,7 +2523,7 @@ exports[`Vertical bar chart re-rendering Should re-render the vertical bar chart
fill="currentColor"
y="16"
>
25,000
25000
</text>
</g>
</g>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1254,7 +1254,7 @@ export class VerticalStackedBarChartBase
return !(
this.props.data &&
this.props.data.length > 0 &&
this.props.data.filter(item => item.chartData.length === 0).length === 0
this.props.data.some(item => item.chartData.length > 0 || (item.lineData && item.lineData.length > 0))
);
}

Expand Down
11 changes: 2 additions & 9 deletions packages/charts/react-charting/src/utilities/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -312,19 +312,12 @@ export function createStringXAxis(
.range([domainNRangeValues.rStartValue, domainNRangeValues.rEndValue])
.paddingInner(typeof xAxisInnerPadding !== 'undefined' ? xAxisInnerPadding : xAxisPadding)
.paddingOuter(typeof xAxisOuterPadding !== 'undefined' ? xAxisOuterPadding : xAxisPadding);
const xAxis = d3AxisBottom(xAxisScale)
.tickSize(xAxistickSize)
.tickPadding(tickPadding)
.ticks(xAxisCount)
.tickFormat((x: string, index: number) => {
return convertToLocaleString(dataset[index], culture) as string;
});
const xAxis = d3AxisBottom(xAxisScale).tickSize(xAxistickSize).tickPadding(tickPadding).ticks(xAxisCount);

if (xAxisParams.xAxisElement) {
d3Select(xAxisParams.xAxisElement).call(xAxis).selectAll('text').attr('aria-hidden', 'true');
}
const tickValues = dataset.map(xAxis.tickFormat()!);
return { xScale: xAxisScale, tickValues };
return { xScale: xAxisScale, tickValues: dataset };
}

/**
Expand Down
Loading