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
Open

Fixes for test app crashes for 12 schema data #33542

Show file tree
Hide file tree
Changes from 2 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"
}
44 changes: 40 additions & 4 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 @@ -31,7 +31,8 @@ import { IVerticalBarChartProps } from '../VerticalBarChart/index';

const isDate = (value: any): boolean => !isNaN(Date.parse(value));
const isNumber = (value: any): boolean => !isNaN(parseFloat(value)) && isFinite(value);
export const isDateArray = (array: any[]): boolean => isArrayOrTypedArray(array) && array.every(isDate);
export const isDateArray = (array: any[]): boolean =>
isArrayOrTypedArray(array) && (array.every(isDate) || array.every(parseDateFromString));
export const isNumberArray = (array: any[]): boolean => isArrayOrTypedArray(array) && array.every(isNumber);
export const isMonthArray = (array: any[]): boolean => {
if (array && array.length > 0) {
Expand Down Expand Up @@ -65,6 +66,35 @@ export const updateXValues = (xValues: any[]): any[] => {
});
return xValues;
};

function parseDateFromString(dateString: string): Date | null {
Copy link
Contributor

Choose a reason for hiding this comment

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

parseDateFromString

does Date.parse not take care of these cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no this is for the cases if the dateString is of any format like "2021 Q1". If it contains date string, parseDateFromString will then parse it and return the date.

Copy link
Contributor

Choose a reason for hiding this comment

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

what is the output for 2021 Q1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

new Date(2021)

Copy link
Contributor

Choose a reason for hiding this comment

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

and for 2121 Q2?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not getting what is 2020 Q2 getting transformed into. What line of code is doing this conversion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Every year if present is getting extracted as it is without the Q2 and created as a Date. For "2020 Q2" it is new Date(2020) as well as for "2020 Q3".

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 not correct. Q1, Q2, Q3, Q4 should have different x values. In such case we can fall back on vertical stacked bar chart and render x value as string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the string can be anything like "2021 Quater1" or something else, the entire string cannot be used to convert to a unique Date string. So, falling back would be best here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added default behaviours in case of unsupported schema types.

// Regular expression for "YYYY-MM-DD" format
const isoDateRegex = /(\d{4})-(\d{2})-(\d{2})/;
const isoDateMatch = dateString.match(isoDateRegex);
if (isoDateMatch) {
const [_, year, month, day] = isoDateMatch;
return new Date(Number(year), Number(month) - 1, Number(day));
}

// Regular expression for "MM/DD/YYYY" format
const usDateRegex = /(\d{2})\/(\d{2})\/(\d{4})/;
const usDateMatch = dateString.match(usDateRegex);
if (usDateMatch) {
const [_, month, day, year] = usDateMatch;
return new Date(Number(year), Number(month) - 1, Number(day));
}

// Regular expression for "YYYY" format
const yearRegex = /(\d{4})/;
const yearMatch = dateString.match(yearRegex);
if (yearMatch) {
const [_, year] = yearMatch;
return new Date(Number(year), 0, 1);
}

return null;
}

export const getColor = (
legendLabel: string,
colorMap: React.MutableRefObject<Map<string, string>>,
Expand Down Expand Up @@ -315,7 +345,7 @@ export const transformPlotlyJsonToScatterChartProps = (
return {
legend,
data: xValues.map((x: string | number, i: number) => ({
x: isString ? (isXDate ? new Date(x) : isXNumber ? parseFloat(x as string) : x) : x,
x: isString ? (isXDate ? parseDateFromString(x.toString()) : isXNumber ? parseFloat(x as string) : x) : x,
y: series.y[i],
})),
color: lineColor,
Expand Down Expand Up @@ -414,9 +444,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