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

fix(react-charting): declarative chart bug fixes #33567

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
+335 −141
Open

fix(react-charting): declarative chart bug fixes #33567

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5922aeb
declarative chart bug fixes
Anush2303 Jan 7, 2025
b7e7adb
Add change file
Anush2303 Jan 7, 2025
ff00fcf
Merge branch 'master' into usr/agupta/declarativeCharts/bugFixes
Anush2303 Jan 7, 2025
63b90a3
update snapshots
Anush2303 Jan 7, 2025
97f2630
Merge branch 'usr/agupta/declarativeCharts/bugFixes' of https://githu…
Anush2303 Jan 7, 2025
a71a1ea
Merge branch 'master' into usr/agupta/declarativeCharts/bugFixes
Anush2303 Jan 7, 2025
41912f3
Gauge indicator color fix
Anush2303 Jan 8, 2025
2c37657
Merge branch 'usr/agupta/declarativeCharts/bugFixes' of https://githu…
Anush2303 Jan 8, 2025
d958c9c
resolve comments
Anush2303 Jan 9, 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-4aabb452-e7e3-47e3-9964-7a7c4814e759.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
Anush2303 marked this conversation as resolved.
Show resolved Hide resolved
"type": "patch",
"comment": "Declarative chart bug fixes",
"packageName": "@fluentui/react-charting",
"email": "[email protected]",
"dependentChangeType": "patch"
}
2 changes: 1 addition & 1 deletion packages/charts/react-charting/src/components/AreaChart/AreaChart.base.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ export class AreaChartBase extends React.Component<IAreaChartProps, IAreaChartSt
});
stackedData.push(currentStack);
});
this._isMultiStackChart = stackedData && stackedData.length > 1 ? true : false;
this._isMultiStackChart = stackedData && stackedData.length >= 1 ? true : false;
Copy link
Contributor

Choose a reason for hiding this comment

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

if stackedData length == 1, then is it multi-stacked?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Logically not. But _isMultiStackChart reference is used only to set the opacity of the chart.
image
To grey out chart in case of single legend also, it has to fail the first if condition.

Copy link
Contributor

Choose a reason for hiding this comment

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

in what case will this value be 0?

Copy link
Contributor Author

@Anush2303 Anush2303 Jan 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I agree, it won't be zero. Then what should we do, either we can ommit this property altogether? Because without changing this, the chart will not grey out. And this is used only to set the opacity.

return {
stackedData,
maxOfYVal,
Expand Down
34 changes: 25 additions & 9 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 @@ -24,20 +24,33 @@ import { IHorizontalBarChartWithAxisProps } from '../HorizontalBarChartWithAxis/
import { ILineChartProps } from '../LineChart/index';
import { IAreaChartProps } from '../AreaChart/index';
import { IHeatMapChartProps } from '../HeatMapChart/index';
import { DataVizPalette, getNextColor } from '../../utilities/colors';
import { DataVizPalette, getColorFromToken, getNextColor } from '../../utilities/colors';
import { GaugeChartVariant, IGaugeChartProps, IGaugeChartSegment } from '../GaugeChart/index';
import { IGroupedVerticalBarChartProps } from '../GroupedVerticalBarChart/index';
import { IVerticalBarChartProps } from '../VerticalBarChart/index';

const isDate = (value: any): boolean => !isNaN(Date.parse(value));
import { timeParse } from 'd3-time-format';

const isDate = (value: any): boolean => {
Anush2303 marked this conversation as resolved.
Show resolved Hide resolved
const yearInString = /\b\d{4}\b/.test(value);
Anush2303 marked this conversation as resolved.
Show resolved Hide resolved
const parsedDate = new Date(Date.parse(value));
const parsedYear = parsedDate.getFullYear();
Anush2303 marked this conversation as resolved.
Show resolved Hide resolved
if (isNaN(parsedDate.getTime())) {
Anush2303 marked this conversation as resolved.
Show resolved Hide resolved
return false;
}
if (!yearInString && (parsedYear === 2000 || parsedYear === 2001)) {
Copy link
Contributor

@srmukher srmukher Jan 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Found that parsed year can have values other than 2000 and 2001 also. And in those cases, isDate() is also returning true:

Value : Year Returned by parsedYear(Value) : isDate(Value)
-1 to -12 : 2001 : true
-13 to -31 : no year : false
-32 to -49 : 2032 to 2049 : true
-50 to -99 : 1950 to 1999 : true
0 : 2000 : true
1 to 12 : 2001 : true
13 to 31 : no year : false
32 to 49 : 2032 - 2049 : true
50 to 99 : 1950 - 1999 : true

Are these expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is expected.

return false;
}
return true;
};
const isNumber = (value: any): boolean => !isNaN(parseFloat(value)) && isFinite(value);
export const isDateArray = (array: any[]): boolean => isArrayOrTypedArray(array) && array.every(isDate);
export const isNumberArray = (array: any[]): boolean => isArrayOrTypedArray(array) && array.every(isNumber);
export const isMonthArray = (array: any[]): boolean => {
if (array && array.length > 0) {
const presentYear = new Date().getFullYear();
const parseFullMonth = timeParse('%B');
const parseShortMonth = timeParse('%b');
return array.every(possiblyMonthValue => {
return isDate(`${possiblyMonthValue} 01, ${presentYear}`);
return parseFullMonth(possiblyMonthValue) !== null || parseShortMonth(possiblyMonthValue) !== null;
});
}
return false;
Expand Down Expand Up @@ -147,7 +160,9 @@ export const transformPlotlyJsonToDonutProps = (
height,
innerRadius,
hideLabels,
showLabelsInPercent: firstData.textinfo ? firstData.textinfo === 'percent' : true,
showLabelsInPercent: firstData.textinfo
? firstData.textinfo === 'percent' || firstData.textinfo === 'label+percent'
Anush2303 marked this conversation as resolved.
Show resolved Hide resolved
: true,
styles,
};
};
Expand Down Expand Up @@ -401,7 +416,8 @@ export const transformPlotlyJsonToHorizontalBarWithAxisProps = (
};
});
})
.flat();
.flat()
.reverse();

Anush2303 marked this conversation as resolved.
Show resolved Hide resolved
const chartHeight: number = typeof layout.height === 'number' ? layout.height : 450;
const margin: number = typeof layout.margin?.l === 'number' ? layout.margin?.l : 0;
Expand Down Expand Up @@ -576,11 +592,11 @@ export const transformPlotlyJsonToGaugeProps = (
const diff = firstData.value - firstData.delta.reference;
if (diff >= 0) {
sublabel = `\u25B2 ${diff}`;
const color = getColor(firstData.delta.increasing?.color || '', colorMap, isDarkTheme);
const color = getColorFromToken(DataVizPalette.success, isDarkTheme);
sublabelColor = color;
} else {
sublabel = `\u25BC ${Math.abs(diff)}`;
Anush2303 marked this conversation as resolved.
Show resolved Hide resolved
const color = getColor(firstData.delta.decreasing?.color || '', colorMap, isDarkTheme);
const color = getColorFromToken(DataVizPalette.error, isDarkTheme);
sublabelColor = color;
}
}
Expand Down
Loading
Loading