Skip to content
Merged
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
43 changes: 43 additions & 0 deletions patches/victory-native/details.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# victory-native patches

## 001+fix-rotated-label-bounds-check

- **Patch:** [victory-native+41.20.2+001+fix-rotated-label-bounds-check.patch](victory-native+41.20.2+001+fix-rotated-label-bounds-check.patch)
- **Issue:** https://github.com/Expensify/App/issues/80970
- **PR:** https://github.com/Expensify/App/pull/80967

**Problem:** Victory Native's XAxis component calculates label bounds using the unrotated text width, even when `labelRotate` is specified. This causes labels near the chart edges to be incorrectly hidden when rotated.

For example, at 90° rotation:
- Actual horizontal extent = font height (~14px)
- Victory's bounds check uses = text width (could be 50-100px+)

This results in labels being hidden even though they would visually fit.

**Fix:** Calculate the actual horizontal extent of rotated labels using the formula:
```
rotatedWidth = textWidth * |cos(angle)| + fontSize * |sin(angle)|
```

Use this `rotatedLabelWidth` for the bounds check (`canFitLabelContent`) while preserving the original `labelWidth` for positioning and rotation origin calculations.

## 002+add-label-overflow-prop

- **Patch:** [victory-native+41.20.2+002+add-label-overflow-prop.patch](victory-native+41.20.2+002+add-label-overflow-prop.patch)

**Problem:** Victory Native's XAxis component applies a `canFitLabelContent` bounds check that hides labels near chart edges. When consumers already control label visibility via `formatXLabel` (returning `''` for skipped labels), this creates double-filtering — labels are hidden both by the consumer's skip logic and by Victory's bounds check. This causes non-uniform gaps and missing end labels.

**Fix:** Add a `labelOverflow` prop to `XAxisInputProps`:
- `"hidden"` (default) — current behavior, bounds check active
- `"visible"` — skip the `canFitLabelContent` check, render all labels with non-empty text

When `labelOverflow` is `"visible"`, the rendering condition changes from:
```typescript
font && labelWidth && canFitLabelContent
```
to:
```typescript
font && labelWidth && (labelOverflow === "visible" || canFitLabelContent)
```

Labels with empty text (`formatXLabel` returning `''`) still get hidden naturally because `labelWidth` evaluates to `0` (falsy). This means the consumer's skip logic remains the sole visibility filter, eliminating double-filtering.
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
diff --git a/node_modules/victory-native/src/cartesian/components/XAxis.tsx b/node_modules/victory-native/src/cartesian/components/XAxis.tsx
index 6d83472..a6e2ed0 100644
--- a/node_modules/victory-native/src/cartesian/components/XAxis.tsx
+++ b/node_modules/victory-native/src/cartesian/components/XAxis.tsx
@@ -63,13 +63,22 @@ export const XAxis = <
font
?.getGlyphWidths?.(font.getGlyphIDs(contentX))
.reduce((sum, value) => sum + value, 0) ?? 0;
+
+ // Calculate actual horizontal extent accounting for rotation for bounds checking
+ // For a rotated rectangle: width * |cos(angle)| + height * |sin(angle)|
+ const rotateRad = (Math.PI / 180) * (labelRotate ?? 0);
+ const cosAngle = Math.abs(Math.cos(rotateRad));
+ const sinAngle = Math.abs(Math.sin(rotateRad));
+ const rotatedLabelWidth = labelWidth * cosAngle + fontSize * sinAngle;
+
const labelX = xScale(tick) - (labelWidth ?? 0) / 2;
+ const rotatedLabelX = xScale(tick) - rotatedLabelWidth / 2;
const canFitLabelContent =
xScale(tick) >= chartBounds.left &&
xScale(tick) <= chartBounds.right &&
(yAxisSide === "left"
- ? labelX + labelWidth < chartBounds.right
- : chartBounds.left < labelX);
+ ? rotatedLabelX + rotatedLabelWidth < chartBounds.right
+ : chartBounds.left < rotatedLabelX);

const labelY = (() => {
// bottom, outset
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
diff --git a/node_modules/victory-native/dist/types.d.ts b/node_modules/victory-native/dist/types.d.ts
index efb5207..df7bbba 100644
--- a/node_modules/victory-native/dist/types.d.ts
+++ b/node_modules/victory-native/dist/types.d.ts
@@ -177,8 +177,9 @@ export type XAxisInputProps<RawData extends Record<string, unknown>, XK extends
yAxisSide?: YAxisSide;
linePathEffect?: DashPathEffectComponent;
enableRescaling?: boolean;
+ labelOverflow?: "hidden" | "visible";
};
-export type XAxisPropsWithDefaults<RawData extends Record<string, unknown>, XK extends keyof InputFields<RawData>> = Required<Omit<XAxisInputProps<RawData, XK>, "font" | "tickValues" | "linePathEffect" | "enableRescaling" | "labelRotate">> & Partial<Pick<XAxisInputProps<RawData, XK>, "font" | "tickValues" | "linePathEffect" | "enableRescaling" | "labelRotate">>;
+export type XAxisPropsWithDefaults<RawData extends Record<string, unknown>, XK extends keyof InputFields<RawData>> = Required<Omit<XAxisInputProps<RawData, XK>, "font" | "tickValues" | "linePathEffect" | "enableRescaling" | "labelRotate" | "labelOverflow">> & Partial<Pick<XAxisInputProps<RawData, XK>, "font" | "tickValues" | "linePathEffect" | "enableRescaling" | "labelRotate" | "labelOverflow">>;
export type XAxisProps<RawData extends Record<string, unknown>, XK extends keyof InputFields<RawData>> = XAxisPropsWithDefaults<RawData, XK> & {
xScale: Scale;
yScale: Scale;
diff --git a/node_modules/victory-native/src/cartesian/components/XAxis.tsx b/node_modules/victory-native/src/cartesian/components/XAxis.tsx
index a6e2ed0..628abab 100644
--- a/node_modules/victory-native/src/cartesian/components/XAxis.tsx
+++ b/node_modules/victory-native/src/cartesian/components/XAxis.tsx
@@ -41,6 +41,7 @@ export const XAxis = <
linePathEffect,
chartBounds,
enableRescaling,
+ labelOverflow,
zoom,
}: XAxisProps<RawData, XK>) => {
const xScale = zoom ? zoom.rescaleX(xScaleProp) : xScaleProp;
@@ -146,7 +147,7 @@ export const XAxis = <
</Line>
</Group>
) : null}
- {font && labelWidth && canFitLabelContent ? (
+ {font && labelWidth && (labelOverflow === "visible" || canFitLabelContent) ? (
<Group transform={[{ translateY: rotateOffset }]}>
<Text
transform={[
diff --git a/node_modules/victory-native/src/types.ts b/node_modules/victory-native/src/types.ts
index a3a4c81..73f5f48 100644
--- a/node_modules/victory-native/src/types.ts
+++ b/node_modules/victory-native/src/types.ts
@@ -201,6 +201,7 @@ export type XAxisInputProps<
yAxisSide?: YAxisSide;
linePathEffect?: DashPathEffectComponent;
enableRescaling?: boolean;
+ labelOverflow?: "hidden" | "visible";
};

export type XAxisPropsWithDefaults<
@@ -209,7 +210,7 @@ export type XAxisPropsWithDefaults<
> = Required<
Omit<
XAxisInputProps<RawData, XK>,
- "font" | "tickValues" | "linePathEffect" | "enableRescaling" | "labelRotate"
+ "font" | "tickValues" | "linePathEffect" | "enableRescaling" | "labelRotate" | "labelOverflow"
>
> &
Partial<
@@ -220,6 +221,7 @@ export type XAxisPropsWithDefaults<
| "linePathEffect"
| "enableRescaling"
| "labelRotate"
+ | "labelOverflow"
>
>;

4 changes: 4 additions & 0 deletions src/components/Charts/BarChart/BarChartContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ function BarChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUni
const {shouldUseNarrowLayout} = useResponsiveLayout();
const font = useFont(fontSource, variables.iconSizeExtraSmall);
const [chartWidth, setChartWidth] = useState(0);
const [barAreaWidth, setBarAreaWidth] = useState(0);
const [containerHeight, setContainerHeight] = useState(0);

const defaultBarColor = CHART_COLORS.at(DEFAULT_SINGLE_BAR_COLOR_INDEX);
Expand Down Expand Up @@ -94,6 +95,7 @@ function BarChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUni
data,
font,
chartWidth,
barAreaWidth,
containerHeight,
});

Expand Down Expand Up @@ -126,6 +128,7 @@ function BarChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUni
barWidth: calculatedBarWidth,
chartBottom: bounds.bottom,
});
setBarAreaWidth(domainWidth);
},
[data.length, barGeometry],
);
Expand Down Expand Up @@ -259,6 +262,7 @@ function BarChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUni
lineWidth: X_AXIS_LINE_WIDTH,
formatXLabel: formatXAxisLabel,
labelRotate: labelRotation,
labelOverflow: 'visible',
}}
yAxis={[
{
Expand Down
20 changes: 12 additions & 8 deletions src/components/Charts/hooks/useChartLabelLayout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ type LabelLayoutConfig = {
data: ChartDataPoint[];
font: SkFont | null;
chartWidth: number;
barAreaWidth: number;
containerHeight: number;
};

Expand All @@ -32,7 +33,7 @@ function measureTextWidth(text: string, font: SkFont): number {
return glyphWidths.reduce((sum, w) => sum + w, 0);
}

function useChartLabelLayout({data, font, chartWidth, containerHeight}: LabelLayoutConfig) {
function useChartLabelLayout({data, font, chartWidth, barAreaWidth, containerHeight}: LabelLayoutConfig) {
return useMemo(() => {
if (!font || chartWidth === 0 || containerHeight === 0 || data.length === 0) {
return {labelRotation: 0, labelSkipInterval: 1, truncatedLabels: data.map((p) => p.label)};
Expand Down Expand Up @@ -120,13 +121,16 @@ function useChartLabelLayout({data, font, chartWidth, containerHeight}: LabelLay
}

// Calculate skip interval using spec formula:
// maxVisibleLabels = floor(chartWidth / (effectiveWidth + MIN_LABEL_GAP))
// maxVisibleLabels = floor(barAreaWidth / (effectiveWidth + MIN_LABEL_GAP))
// skipInterval = ceil(barCount / maxVisibleLabels)
let skipInterval = 1;
const maxVisibleLabels = Math.floor(chartWidth / (effectiveWidth + LABEL_PADDING));
if (maxVisibleLabels > 0 && maxVisibleLabels < data.length) {
skipInterval = Math.ceil(data.length / maxVisibleLabels);
}
// Use barAreaWidth (actual plotting area from chartBounds) rather than chartWidth
// (full container) so Y-axis labels and padding don't inflate the count.
const labelAreaWidth = barAreaWidth || chartWidth;
const maxVisibleLabels = Math.floor(labelAreaWidth / (effectiveWidth + LABEL_PADDING));
// When maxVisibleLabels is 0 (area too narrow for even one label) or less than
// data.length, compute the interval. data.length is the safe upper bound — show
// at most the first label.
const skipInterval = maxVisibleLabels >= data.length ? 1 : Math.ceil(data.length / Math.max(1, maxVisibleLabels));

// Convert rotation to negative degrees for Victory chart
let rotationValue = 0;
Expand All @@ -137,7 +141,7 @@ function useChartLabelLayout({data, font, chartWidth, containerHeight}: LabelLay
}

return {labelRotation: rotationValue, labelSkipInterval: skipInterval, truncatedLabels: finalLabels, maxLabelLength};
}, [font, chartWidth, containerHeight, data]);
}, [font, chartWidth, barAreaWidth, containerHeight, data]);
}

export {useChartLabelLayout};
Expand Down
Loading