Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🕵🏾‍♀️ visual changes to review in the Visual Change Report

vr-tests-react-components/Menu Converged - submenuIndicator slotted content 1 screenshots
Image Name Diff(in Pixels) Image Type
vr-tests-react-components/Menu Converged - submenuIndicator slotted content.default - RTL.submenus open.chromium.png 404 Changed
vr-tests-react-components/Positioning 2 screenshots
Image Name Diff(in Pixels) Image Type
vr-tests-react-components/Positioning.Positioning end.updated 2 times.chromium.png 26 Changed
vr-tests-react-components/Positioning.Positioning end.chromium.png 627 Changed
vr-tests-react-components/ProgressBar converged 1 screenshots
Image Name Diff(in Pixels) Image Type
vr-tests-react-components/ProgressBar converged.Indeterminate + thickness.default.chromium.png 128 Changed
vr-tests-react-components/TagPicker 1 screenshots
Image Name Diff(in Pixels) Image Type
vr-tests-react-components/TagPicker.disabled.chromium.png 677 Changed

There were 1 duplicate changes discarded. Check the build logs for more information.

"type": "patch",
"comment": "fix: improve LineChart and GaugeChart callout behavior and formatting",
"packageName": "@fluentui/react-charts",
"email": "atisjai@microsoft.com",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
Expand Up @@ -897,14 +897,22 @@ export const GaugeChart: React_2.FunctionComponent<GaugeChartProps>;

// @public
export interface GaugeChartCalloutData {
chartTitle?: string;
chartValue: number;
chartValueLabel: string;
legend: string;
maxValue: number;
minValue: number;
segments: GaugeChartCalloutSegment[];
segmentValues: YValueHover[];
}

// @public
export interface GaugeChartCalloutSegment extends GaugeChartSegment {
end: number;
start: number;
}

// @public
export interface GaugeChartProps {
calloutProps?: Partial<ChartPopoverProps>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,12 @@ export const CartesianChart: React.FunctionComponent<ModifiedCartesianChartProps
props.onChartMouseLeave && props.onChartMouseLeave();
}

function _onChartBlur(event: React.FocusEvent<HTMLDivElement>): void {
if (!event.relatedTarget || !event.currentTarget.contains(event.relatedTarget as Node)) {
_onChartLeave();
}
}

function _calculateChartMinWidth(): number {
// Adding 10px for padding on both sides
const labelWidth = _calcMaxLabelWidthWithTransform(_tickLabels) + 10;
Expand Down Expand Up @@ -746,6 +752,7 @@ export const CartesianChart: React.FunctionComponent<ModifiedCartesianChartProps
ref={(rootElem: HTMLDivElement) => {
chartContainer.current = rootElem;
}}
onBlur={_onChartBlur}
onMouseLeave={_onChartLeave}
>
<div className={classes.chartWrapper} {...focusAttributes} {...arrowAttributes}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -392,19 +392,26 @@ describe('GaugeChart custom callout', () => {
{
segments,
chartValue: 30,
chartTitle: 'Server tick time',
minValue: 10,
maxValue: 110,
onRenderCallout: (calloutData?: GaugeChartCalloutData) => (
<div data-testid="custom-gauge-callout">
{calloutData?.legend}: {calloutData?.chartValue} blocks ({calloutData?.minValue}-{calloutData?.maxValue})
</div>
),
onRenderCallout: (calloutData?: GaugeChartCalloutData) => {
const lastSegment = calloutData?.segments.at(-1);
return (
<div data-testid="custom-gauge-callout">
{calloutData?.chartTitle}: {calloutData?.legend}: {calloutData?.chartValue} blocks ({calloutData?.minValue}-
{calloutData?.maxValue}); last range {lastSegment?.start}-{lastSegment?.end}
</div>
);
},
},
() => {
const chartSegments = screen.getAllByText((content, element) => element!.tagName.toLowerCase() === 'path');
fireEvent.mouseOver(chartSegments[0]);

expect(screen.getByTestId('custom-gauge-callout')).toHaveTextContent('Low Risk: 30 blocks (10-110)');
expect(screen.getByTestId('custom-gauge-callout')).toHaveTextContent(
'Server tick time: Low Risk: 30 blocks (10-110); last range 77-110',
);
},
);

Expand All @@ -414,13 +421,14 @@ describe('GaugeChart custom callout', () => {
{
segments,
chartValue: 30,
chartValueFormat: ([value]) => `${value}ms`,
onRenderCallout: renderDefaultCallout,
},
() => {
const chartSegments = screen.getAllByText((content, element) => element!.tagName.toLowerCase() === 'path');
fireEvent.mouseOver(chartSegments[0]);

expect(screen.getByTestId('wrapped-gauge-callout')).toHaveTextContent('Current value is 30/100');
expect(screen.getByTestId('wrapped-gauge-callout')).toHaveTextContent('Current value is 30ms');
expect(screen.getByTestId('wrapped-gauge-callout')).toHaveTextContent('Low Risk');
},
);
Expand Down Expand Up @@ -504,30 +512,33 @@ describe('GaugeChart rendering and behavior tests', () => {

it('should render the chart value correctly', () => {
const customChartValue = 'Custom chart value';
const formatMilliseconds = ([value]: [number, number]) => (value === 0 ? 'offline' : `${value}ms`);

expect(getChartValueLabel(25, 0, 100)).toBe('25%');
expect(getChartValueLabel(25, 0, 100, undefined, true)).toBe('25/100');
expect(getChartValueLabel(25, 0, 100, undefined, true)).toBe('25%');

expect(getChartValueLabel(25, 0, 100, GaugeValueFormat.Percentage)).toBe('25%');
expect(getChartValueLabel(25, 0, 100, GaugeValueFormat.Percentage, true)).toBe('25/100');
expect(getChartValueLabel(25, 0, 100, GaugeValueFormat.Percentage, true)).toBe('25%');

expect(getChartValueLabel(25, 0, 100, GaugeValueFormat.Fraction)).toBe('25/100');
expect(getChartValueLabel(25, 0, 100, GaugeValueFormat.Fraction, true)).toBe('25%');
expect(getChartValueLabel(25, 0, 100, GaugeValueFormat.Fraction, true)).toBe('25/100');

expect(getChartValueLabel(25, 0, 100, () => customChartValue)).toBe(customChartValue);
expect(getChartValueLabel(25, 0, 100, () => customChartValue, true)).toBe('25/100');
expect(getChartValueLabel(25, 0, 100, () => customChartValue, true)).toBe(customChartValue);
expect(getChartValueLabel(50, 0, 200, formatMilliseconds, true)).toBe('50ms');
expect(getChartValueLabel(0, 0, 200, formatMilliseconds, true)).toBe('offline');

expect(getChartValueLabel(125, 100, 200)).toBe('125');
expect(getChartValueLabel(125, 100, 200, undefined, true)).toBe('125');

expect(getChartValueLabel(125, 100, 200, GaugeValueFormat.Percentage)).toBe('125');
expect(getChartValueLabel(125, 100, 200, GaugeValueFormat.Percentage, true)).toBe('125');
expect(getChartValueLabel(125, 100, 200, GaugeValueFormat.Percentage)).toBe('25%');
expect(getChartValueLabel(125, 100, 200, GaugeValueFormat.Percentage, true)).toBe('25%');

expect(getChartValueLabel(125, 100, 200, GaugeValueFormat.Fraction)).toBe('125');
expect(getChartValueLabel(125, 100, 200, GaugeValueFormat.Fraction, true)).toBe('125');

expect(getChartValueLabel(125, 100, 200, () => customChartValue)).toBe(customChartValue);
expect(getChartValueLabel(125, 100, 200, () => customChartValue, true)).toBe('125');
expect(getChartValueLabel(125, 100, 200, () => customChartValue, true)).toBe(customChartValue);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,25 +83,38 @@ export const getChartValueLabel = (
chartValueFormat?: GaugeValueFormat | ((sweepFraction: [number, number]) => string),
forCallout: boolean = false,
): string => {
if (forCallout) {
// When displaying the chart value as a percentage, use fractions in the callout, and vice versa.
// This helps clarify the actual value and avoid repetition.
return minValue !== 0
? chartValue.toString()
: chartValueFormat === 'fraction'
? `${((chartValue / maxValue) * 100).toFixed()}%`
: `${chartValue}/${maxValue}`;
if (typeof chartValueFormat === 'function') {
return chartValueFormat([chartValue - minValue, maxValue - minValue]);
}

return typeof chartValueFormat === 'function'
? chartValueFormat([chartValue - minValue, maxValue - minValue])
: minValue !== 0
if (chartValueFormat === 'percentage') {
return `${(((chartValue - minValue) / (maxValue - minValue)) * 100).toFixed()}%`;
}

return minValue !== 0
? chartValue.toString()
: chartValueFormat === 'fraction'
? `${chartValue}/${maxValue}`
: `${((chartValue / maxValue) * 100).toFixed()}%`;
};

const getCalloutSegmentLabel = (
segment: ExtendedSegment,
minValue: number,
maxValue: number,
variant: GaugeChartVariant | undefined,
chartValueFormat: GaugeChartProps['chartValueFormat'],
): string => {
if (chartValueFormat === 'percentage' || (!chartValueFormat && minValue === 0)) {
const range = maxValue - minValue;
const startPercentage = (((segment.start - minValue) / range) * 100).toFixed();
const endPercentage = (((segment.end - minValue) / range) * 100).toFixed();
return `${startPercentage}% - ${endPercentage}%`;
}

return getSegmentLabel(segment, minValue, maxValue, variant);
};

interface YValue extends Omit<YValueHover, 'y'> {
y?: string | number;
}
Expand Down Expand Up @@ -396,7 +409,7 @@ export const GaugeChart: React.FunctionComponent<GaugeChartProps> = React.forwar
.map(segment => {
const yValue: YValue = {
legend: segment.legend,
y: getSegmentLabel(segment, _minValue, _maxValue, props.variant),
y: getCalloutSegmentLabel(segment, _minValue, _maxValue, props.variant, props.chartValueFormat),
color: segment.color,
};
return yValue;
Expand Down Expand Up @@ -726,6 +739,7 @@ export const GaugeChart: React.FunctionComponent<GaugeChartProps> = React.forwar
(() => {
const calloutData: GaugeChartCalloutData = {
legend: calloutLegend,
chartTitle: props.chartTitle,
chartValue: props.chartValue,
minValue: _minValue,
maxValue: _maxValue,
Expand All @@ -736,6 +750,7 @@ export const GaugeChart: React.FunctionComponent<GaugeChartProps> = React.forwar
props.chartValueFormat,
true,
),
segments: _segments,
segmentValues: hoverYValues,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,22 @@ export type GaugeValueFormat = 'percentage' | 'fraction';
*/
export type GaugeChartVariant = 'single-segment' | 'multiple-segments';

/**
* GaugeChart segment data with its calculated range.
* {@docCategory GaugeChart}
*/
export interface GaugeChartCalloutSegment extends GaugeChartSegment {
/**
* Start of the segment range.
*/
start: number;

/**
* End of the segment range.
*/
end: number;
}

/**
* Data provided to a custom GaugeChart callout renderer.
* {@docCategory GaugeChart}
Expand All @@ -56,6 +72,11 @@ export interface GaugeChartCalloutData {
*/
legend: string;

/**
* Title of the gauge.
*/
chartTitle?: string;

/**
* Current value of the gauge.
*/
Expand All @@ -76,6 +97,11 @@ export interface GaugeChartCalloutData {
*/
chartValueLabel: string;

/**
* Gauge segments with their calculated ranges.
*/
segments: GaugeChartCalloutSegment[];

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[ReviewAgent] Bug: anyone who builds this object in a test fixture or a wrapper stops compiling on upgrade, because the new field is required while chartTitle beside it at :78 is optional. GaugeChartCalloutData is published in api.md. make it optional.


/**
* Segment values displayed in the default callout.
*/
Expand Down
Loading
Loading