Skip to content
Open
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 @@
{
"type": "patch",
"comment": "fix double padding issue in scatter chart",
"packageName": "@fluentui/react-charts",
"email": "anushgupta@microsoft.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ export interface CartesianChartProps {
xMinValue?: number;

/**
* maximum data value point in x-axis (for numeric x-axis)
* maximum data value point in x-axis
*/
xMaxValue?: number;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,8 @@ export const LineChart: React.FunctionComponent<LineChartProps> = React.forwardR
isRTL,
props.xScaleType,
_hasMarkersMode,
props.xMinValue,
props.xMaxValue,
);
} else if (xAxisType === XAxisTypes.DateAxis) {
domainNRangeValue = domainRangeOfDateForAreaLineScatterVerticalBarCharts(
Expand Down Expand Up @@ -550,6 +552,10 @@ export const LineChart: React.FunctionComponent<LineChartProps> = React.forwardR
xScaleType: props.xScaleType,
yScaleType: props.yScaleType,
secondaryYScaleType: props.secondaryYScaleType,
xMinValue: props.xMinValue,
xMaxValue: props.xMaxValue,
yMinValue: props.yMinValue,
yMaxValue: props.yMaxValue,
})
: 0;
if (_points[i].data.length === 1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,8 @@ export const ScatterChart: React.FunctionComponent<ScatterChartProps> = React.fo
isRTL,
props.xScaleType,
true,
props.xMinValue,
props.xMaxValue,
);
} else if (xAxisType === XAxisTypes.DateAxis) {
domainNRangeValue = domainRangeOfDateForAreaLineScatterVerticalBarCharts(
Expand Down Expand Up @@ -386,6 +388,10 @@ export const ScatterChart: React.FunctionComponent<ScatterChartProps> = React.fo
yScalePrimary: _yAxisScale,
xScaleType: props.xScaleType,
yScaleType: props.yScaleType,
xMinValue: props.xMinValue,
xMaxValue: props.xMaxValue,
yMinValue: props.yMinValue,
yMaxValue: props.yMaxValue,
})
: 0;

Expand Down
33 changes: 29 additions & 4 deletions packages/charts/react-charts/library/src/utilities/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1366,12 +1366,14 @@ export function domainRangeOfNumericForAreaLineScatterCharts(
isRTL: boolean,
scaleType?: AxisScaleType,
hasMarkersMode?: boolean,
xMinVal?: number,
xMaxVal?: number,
): IDomainNRange {
const isScatterPolar = isScatterPolarSeries(points);
let [xMin, xMax] = getScatterXDomainExtent(points, scaleType) as [number, number];

if (hasMarkersMode) {
const xPadding = getDomainPaddingForMarkers(xMin, xMax, scaleType);
const xPadding = getDomainPaddingForMarkers(xMin, xMax, scaleType, xMinVal, xMaxVal);
xMin = xMin - xPadding.start;
xMax = xMax + xPadding.end;
}
Expand Down Expand Up @@ -2212,6 +2214,8 @@ export const getDomainPaddingForMarkers = (
minVal: number,
maxVal: number,
scaleType?: AxisScaleType,
userMinVal?: number,
userMaxVal?: number,
): { start: number; end: number } => {
if (scaleType === 'log') {
return {
Expand All @@ -2220,7 +2224,20 @@ export const getDomainPaddingForMarkers = (
};
}

const defaultPadding = (maxVal - minVal) * 0.1;
/* if user explicitly sets userMinVal or userMaxVal, we will check that to avoid excessive padding on either side.
If the difference between minVal and userMinVal is more than 10% of the data range, we set padding to 0 on that side.
this is to avoid cases where userMinVal is significantly smaller than minVal or userMaxVal is significantly larger than
maxVal, which would lead to excessive padding. In other cases, we apply the default 10% padding on both sides.
*/
const rangePadding = (maxVal - minVal) * 0.1;

// If explicit bounds are set and they're far from the data range, don't add extra padding
const hasExcessivePaddingAtMin =
Copy link
Contributor

Choose a reason for hiding this comment

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

call this padding already satisfied

userMinVal !== undefined && rangePadding > Math.abs(minVal - Math.min(minVal, userMinVal));
const hasExcessivePaddingAtMax =
userMaxVal !== undefined && rangePadding > Math.abs(maxVal - Math.max(maxVal, userMaxVal));

const defaultPadding = hasExcessivePaddingAtMin && hasExcessivePaddingAtMax ? 0 : rangePadding;
Copy link
Contributor

Choose a reason for hiding this comment

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

defaultPadding = hasExcessivePaddingAtMin && hasExcessivePaddingAtMax ? 0 : rangePadding;

The min and max padding should be considered independently while deciding the defaultPadding

return {
start: defaultPadding,
end: defaultPadding,
Expand Down Expand Up @@ -2270,6 +2287,10 @@ export const getRangeForScatterMarkerSize = ({
xScaleType,
yScaleType: primaryYScaleType,
secondaryYScaleType,
xMinValue,
xMaxValue,
yMinValue,
yMaxValue,
}: {
data: LineChartPoints[] | ScatterChartPoints[];
xScale: ScaleContinuousNumeric<number, number> | ScaleTime<number, number>;
Expand All @@ -2279,6 +2300,10 @@ export const getRangeForScatterMarkerSize = ({
xScaleType?: AxisScaleType;
yScaleType?: AxisScaleType;
secondaryYScaleType?: AxisScaleType;
xMinValue?: number;
xMaxValue?: number;
yMinValue?: number;
yMaxValue?: number;
}): number => {
// Note: This function is executed after the scale is created, so the actual padding can be
// obtained by calculating the difference between the respective minimums or maximums of the
Expand All @@ -2290,14 +2315,14 @@ export const getRangeForScatterMarkerSize = ({
// it the other way around (i.e., adjusting the scale domain first with padding and then scaling
// the markers to fit inside the plot area).
const [xMin, xMax] = getScatterXDomainExtent(data, xScaleType);
const xPadding = getDomainPaddingForMarkers(+xMin, +xMax, xScaleType);
const xPadding = getDomainPaddingForMarkers(+xMin, +xMax, xScaleType, xMinValue, xMaxValue);
const scaleXMin = xMin instanceof Date ? new Date(+xMin - xPadding.start) : xMin - xPadding.start;
const scaleXMax = xMax instanceof Date ? new Date(+xMax + xPadding.end) : xMax + xPadding.end;
const extraXPixels = Math.min(Math.abs(xScale(xMin) - xScale(scaleXMin)), Math.abs(xScale(scaleXMax) - xScale(xMax)));

const yScaleType = useSecondaryYScale ? secondaryYScaleType : primaryYScaleType;
const { startValue: yMin, endValue: yMax } = findNumericMinMaxOfY(data, undefined, useSecondaryYScale, yScaleType);
const yPadding = getDomainPaddingForMarkers(yMin, yMax, yScaleType);
const yPadding = getDomainPaddingForMarkers(yMin, yMax, yScaleType, yMinValue, yMaxValue);
const scaleYMin = yMin - yPadding.start;
const scaleYMax = yMax + yPadding.end;
const yScale = (useSecondaryYScale ? yScaleSecondary : yScalePrimary)!;
Expand Down