From 6d704d2f21df1af7c20227ed46c477d53d99e56b Mon Sep 17 00:00:00 2001 From: Anush Gupta <74965306+Anush2303@users.noreply.github.com> Date: Wed, 18 Dec 2024 12:12:05 +0000 Subject: [PATCH 1/5] Ensure type safety of dependent fields --- .../DeclarativeChart/PlotlySchemaAdapter.ts | 58 +++++++++---------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/packages/charts/react-charting/src/components/DeclarativeChart/PlotlySchemaAdapter.ts b/packages/charts/react-charting/src/components/DeclarativeChart/PlotlySchemaAdapter.ts index 70b9aa4b8dc952..81c7afda1b23cb 100644 --- a/packages/charts/react-charting/src/components/DeclarativeChart/PlotlySchemaAdapter.ts +++ b/packages/charts/react-charting/src/components/DeclarativeChart/PlotlySchemaAdapter.ts @@ -65,11 +65,11 @@ export const transformPlotlyJsonToDonutProps = ( }; }); - const width: number = layout?.width || 440; - const height: number = layout?.height || 220; - const hideLabels = firstData.textinfo ? !['value', 'percent'].includes(firstData.textinfo) : false; - const donutMarginHorizontal = hideLabels ? 0 : 80; - const donutMarginVertical = 40 + (hideLabels ? 0 : 40); + const width: number = typeof layout?.width === 'number' ? layout?.width : 440; + const height: number = typeof layout?.height === 'number' ? layout?.height : 220; + const hideLabels: boolean = firstData.textinfo ? !['value', 'percent'].includes(firstData.textinfo) : false; + const donutMarginHorizontal: number = hideLabels ? 0 : 80; + const donutMarginVertical: number = 40 + (hideLabels ? 0 : 40); const innerRadius: number = firstData.hole ? firstData.hole * (Math.min(width - donutMarginHorizontal, height - donutMarginVertical) / 2) : 0; @@ -77,7 +77,7 @@ export const transformPlotlyJsonToDonutProps = ( const styles: IDonutChartProps['styles'] = { root: { '[class^="arcLabel"]': { - fontSize: firstData.textfont?.size, + fontSize: typeof firstData.textfont?.size === 'number' ? firstData.textfont?.size : 0, }, }, }; @@ -222,8 +222,8 @@ export const transformPlotlyJsonToVBCProps = ( const totalDataPoints = d3Merge(buckets).length; buckets.forEach(bucket => { - const legend = series.name || `Series ${index + 1}`; - const color = getColor(legend, colorMap, isDarkTheme); + const legend: string = series.name || `Series ${index + 1}`; + const color: string = getColor(legend, colorMap, isDarkTheme); let y = bucket.length; if (series.histnorm === 'percent') { @@ -256,7 +256,7 @@ export const transformPlotlyJsonToVBCProps = ( return { data: vbcData, - chartTitle: layout?.title, + chartTitle: typeof layout?.title === 'string' ? layout?.title : '', // width: layout?.width, // height: layout?.height, hideLegend: true, @@ -278,7 +278,7 @@ export const transformPlotlyJsonToScatterChartProps = ( const isString = typeof xValues[0] === 'string'; const isXDate = isDateArray(xValues); const isXNumber = isNumberArray(xValues); - const legend = series.name || `Series ${index + 1}`; + const legend: string = series.name || `Series ${index + 1}`; const lineColor = getColor(legend, colorMap, isDarkTheme); return { @@ -292,7 +292,7 @@ export const transformPlotlyJsonToScatterChartProps = ( }); const chartProps: IChartProps = { - chartTitle: layout.title || '', + chartTitle: typeof layout.title === 'string' ? layout.title : '', lineChartData: chartData, }; @@ -330,24 +330,24 @@ export const transformPlotlyJsonToHorizontalBarWithAxisProps = ( }) .flat(); - const chartHeight = layout.height || 450; - const margin = layout.margin?.l || 0; - const padding = layout.margin?.pad || 0; - const availableHeight = chartHeight - margin - padding; - const numberOfBars = data[0].y.length; + const chartHeight: number = typeof layout.height === 'number' ? layout.height : 450; + const margin: number = typeof layout.margin?.l === 'number' ? layout.margin?.l : 0; + const padding: number = typeof layout.margin?.pad === 'number' ? layout.margin?.pad : 0; + const availableHeight: number = chartHeight - margin - padding; + const numberOfBars: number = typeof data[0].y.length === 'number' ? data[0].y.length : 0; const scalingFactor = 0.01; const gapFactor = 1 / (1 + scalingFactor * numberOfBars); const barHeight = availableHeight / (numberOfBars * (1 + gapFactor)); return { data: chartData, - chartTitle: layout.title || '', + chartTitle: typeof layout.title === 'string' ? layout.title : '', barHeight, showYAxisLables: true, styles: { root: { height: chartHeight, - width: layout.width || 600, + width: typeof layout.width === 'number' ? layout.width : 600, }, }, }; @@ -375,7 +375,7 @@ export const transformPlotlyJsonToHeatmapProps = (jsonObj: any): IHeatMapChartPr }); }); const heatmapData: IHeatMapChartData = { - legend: firstData.name || '', + legend: typeof firstData.name === 'string' ? firstData.name : '', data: heatmapDataPoints, value: 0, }; @@ -429,17 +429,17 @@ export const transformPlotlyJsonToSankeyProps = ( }), }; - const width: number = layout?.width || 440; - const height: number = layout?.height || 220; + const width: number = typeof layout?.width === 'number' ? layout?.width : 440; + const height: number = typeof layout?.height === 'number' ? layout?.height : 220; const styles: ISankeyChartProps['styles'] = { root: { - fontSize: layout.font?.size, + fontSize: typeof layout.font?.size === 'number' ? layout.font?.size : 0, }, }; const shouldResize: number = width + height; return { data: { - chartTitle: layout?.title, + chartTitle: typeof layout?.title === 'string' ? layout?.title : '', SankeyChartData: sankeyChartData, }, width, @@ -491,15 +491,15 @@ export const transformPlotlyJsonToGaugeProps = ( return { segments, - chartValue: firstData.value, - chartTitle: firstData.title?.text, + chartValue: typeof firstData.value === 'number' ? firstData.value : 0, + chartTitle: typeof firstData.title?.text === 'string' ? firstData.title?.text : '', sublabel, // range values can be null - minValue: firstData.gauge?.axis?.range?.[0] ?? undefined, - maxValue: firstData.gauge?.axis?.range?.[1] ?? undefined, + minValue: typeof firstData.gauge?.axis?.range?.[0] === 'number' ? firstData.gauge?.axis?.range?.[0] : undefined, + maxValue: typeof firstData.gauge?.axis?.range?.[1] === 'number' ? firstData.gauge?.axis?.range?.[1] : undefined, chartValueFormat: () => firstData.value, - width: layout?.width, - height: layout?.height, + width: typeof layout?.width === 'number' ? layout?.width : 0, + height: typeof layout?.height === 'number' ? layout?.height : 0, hideLegend: true, styles, }; From b90ec2a480defe044e899798b2208dddf904b587 Mon Sep 17 00:00:00 2001 From: Anush Gupta <74965306+Anush2303@users.noreply.github.com> Date: Wed, 18 Dec 2024 12:15:24 +0000 Subject: [PATCH 2/5] Add change file --- ...eact-charting-f56dc096-9891-4d5c-ae30-4d8a6ac5b481.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@fluentui-react-charting-f56dc096-9891-4d5c-ae30-4d8a6ac5b481.json diff --git a/change/@fluentui-react-charting-f56dc096-9891-4d5c-ae30-4d8a6ac5b481.json b/change/@fluentui-react-charting-f56dc096-9891-4d5c-ae30-4d8a6ac5b481.json new file mode 100644 index 00000000000000..5a9fd5dcb32329 --- /dev/null +++ b/change/@fluentui-react-charting-f56dc096-9891-4d5c-ae30-4d8a6ac5b481.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Ensure type safety of dependent fields", + "packageName": "@fluentui/react-charting", + "email": "74965306+Anush2303@users.noreply.github.com", + "dependentChangeType": "patch" +} From 76d5ba3e96a54485313306d97411ca1f1e833fd2 Mon Sep 17 00:00:00 2001 From: Anush Gupta <74965306+Anush2303@users.noreply.github.com> Date: Wed, 18 Dec 2024 12:32:50 +0000 Subject: [PATCH 3/5] controlled legend multi select fix --- .../src/components/DeclarativeChart/DeclarativeChart.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/charts/react-charting/src/components/DeclarativeChart/DeclarativeChart.tsx b/packages/charts/react-charting/src/components/DeclarativeChart/DeclarativeChart.tsx index c8f199b72fb03e..c75490160da75b 100644 --- a/packages/charts/react-charting/src/components/DeclarativeChart/DeclarativeChart.tsx +++ b/packages/charts/react-charting/src/components/DeclarativeChart/DeclarativeChart.tsx @@ -141,7 +141,7 @@ export const DeclarativeChart: React.FunctionComponent = Date: Wed, 18 Dec 2024 14:52:13 +0000 Subject: [PATCH 4/5] resolve comments --- .../src/components/DeclarativeChart/PlotlySchemaAdapter.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/charts/react-charting/src/components/DeclarativeChart/PlotlySchemaAdapter.ts b/packages/charts/react-charting/src/components/DeclarativeChart/PlotlySchemaAdapter.ts index 81c7afda1b23cb..5dfec2532bd009 100644 --- a/packages/charts/react-charting/src/components/DeclarativeChart/PlotlySchemaAdapter.ts +++ b/packages/charts/react-charting/src/components/DeclarativeChart/PlotlySchemaAdapter.ts @@ -77,7 +77,7 @@ export const transformPlotlyJsonToDonutProps = ( const styles: IDonutChartProps['styles'] = { root: { '[class^="arcLabel"]': { - fontSize: typeof firstData.textfont?.size === 'number' ? firstData.textfont?.size : 0, + fontSize: typeof firstData.textfont?.size === 'number' ? firstData.textfont?.size : undefined, }, }, }; @@ -334,7 +334,7 @@ export const transformPlotlyJsonToHorizontalBarWithAxisProps = ( const margin: number = typeof layout.margin?.l === 'number' ? layout.margin?.l : 0; const padding: number = typeof layout.margin?.pad === 'number' ? layout.margin?.pad : 0; const availableHeight: number = chartHeight - margin - padding; - const numberOfBars: number = typeof data[0].y.length === 'number' ? data[0].y.length : 0; + const numberOfBars = data[0].y.length; const scalingFactor = 0.01; const gapFactor = 1 / (1 + scalingFactor * numberOfBars); const barHeight = availableHeight / (numberOfBars * (1 + gapFactor)); @@ -433,7 +433,7 @@ export const transformPlotlyJsonToSankeyProps = ( const height: number = typeof layout?.height === 'number' ? layout?.height : 220; const styles: ISankeyChartProps['styles'] = { root: { - fontSize: typeof layout.font?.size === 'number' ? layout.font?.size : 0, + fontSize: typeof layout.font?.size === 'number' ? layout.font?.size : undefined, }, }; const shouldResize: number = width + height; From 14b236e5fd18660aea356bad0bbb42b488297120 Mon Sep 17 00:00:00 2001 From: Anush Gupta <74965306+Anush2303@users.noreply.github.com> Date: Wed, 18 Dec 2024 15:59:11 +0000 Subject: [PATCH 5/5] resolve comments --- .../src/components/DeclarativeChart/PlotlySchemaAdapter.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/charts/react-charting/src/components/DeclarativeChart/PlotlySchemaAdapter.ts b/packages/charts/react-charting/src/components/DeclarativeChart/PlotlySchemaAdapter.ts index 5dfec2532bd009..368fbda214df2f 100644 --- a/packages/charts/react-charting/src/components/DeclarativeChart/PlotlySchemaAdapter.ts +++ b/packages/charts/react-charting/src/components/DeclarativeChart/PlotlySchemaAdapter.ts @@ -77,7 +77,7 @@ export const transformPlotlyJsonToDonutProps = ( const styles: IDonutChartProps['styles'] = { root: { '[class^="arcLabel"]': { - fontSize: typeof firstData.textfont?.size === 'number' ? firstData.textfont?.size : undefined, + ...(typeof firstData.textfont?.size === 'number' ? { fontSize: firstData.textfont.size } : {}), }, }, }; @@ -433,7 +433,7 @@ export const transformPlotlyJsonToSankeyProps = ( const height: number = typeof layout?.height === 'number' ? layout?.height : 220; const styles: ISankeyChartProps['styles'] = { root: { - fontSize: typeof layout.font?.size === 'number' ? layout.font?.size : undefined, + ...(typeof layout.font?.size === 'number' ? { fontSize: layout.font?.size } : {}), }, }; const shouldResize: number = width + height;