From b6ad3f1cd01c2eb01c554f2a0a2582e2f729169b Mon Sep 17 00:00:00 2001 From: Mateusz Rajski Date: Wed, 25 Feb 2026 15:41:07 +0100 Subject: [PATCH 01/13] Fix label clipping for both Cartesian charts --- patches/victory-native/details.md | 43 ----- ...2+001+fix-rotated-label-bounds-check.patch | 29 ---- ...+41.20.2+002+add-label-overflow-prop.patch | 65 -------- .../Charts/BarChart/BarChartContent.tsx | 46 ++++-- .../Charts/LineChart/LineChartContent.tsx | 105 +++--------- .../Charts/components/ChartXAxisLabels.tsx | 75 +++++++++ .../Charts/hooks/useChartLabelLayout.ts | 151 +++++++++++++----- 7 files changed, 245 insertions(+), 269 deletions(-) delete mode 100644 patches/victory-native/details.md delete mode 100644 patches/victory-native/victory-native+41.20.2+001+fix-rotated-label-bounds-check.patch delete mode 100644 patches/victory-native/victory-native+41.20.2+002+add-label-overflow-prop.patch create mode 100644 src/components/Charts/components/ChartXAxisLabels.tsx diff --git a/patches/victory-native/details.md b/patches/victory-native/details.md deleted file mode 100644 index d0fd67d8efb2e..0000000000000 --- a/patches/victory-native/details.md +++ /dev/null @@ -1,43 +0,0 @@ -# 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. diff --git a/patches/victory-native/victory-native+41.20.2+001+fix-rotated-label-bounds-check.patch b/patches/victory-native/victory-native+41.20.2+001+fix-rotated-label-bounds-check.patch deleted file mode 100644 index 35093d2ec3edc..0000000000000 --- a/patches/victory-native/victory-native+41.20.2+001+fix-rotated-label-bounds-check.patch +++ /dev/null @@ -1,29 +0,0 @@ -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 diff --git a/patches/victory-native/victory-native+41.20.2+002+add-label-overflow-prop.patch b/patches/victory-native/victory-native+41.20.2+002+add-label-overflow-prop.patch deleted file mode 100644 index 232ed798110fb..0000000000000 --- a/patches/victory-native/victory-native+41.20.2+002+add-label-overflow-prop.patch +++ /dev/null @@ -1,65 +0,0 @@ -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, XK extends - yAxisSide?: YAxisSide; - linePathEffect?: DashPathEffectComponent; - enableRescaling?: boolean; -+ labelOverflow?: "hidden" | "visible"; - }; --export type XAxisPropsWithDefaults, XK extends keyof InputFields> = Required, "font" | "tickValues" | "linePathEffect" | "enableRescaling" | "labelRotate">> & Partial, "font" | "tickValues" | "linePathEffect" | "enableRescaling" | "labelRotate">>; -+export type XAxisPropsWithDefaults, XK extends keyof InputFields> = Required, "font" | "tickValues" | "linePathEffect" | "enableRescaling" | "labelRotate" | "labelOverflow">> & Partial, "font" | "tickValues" | "linePathEffect" | "enableRescaling" | "labelRotate" | "labelOverflow">>; - export type XAxisProps, XK extends keyof InputFields> = XAxisPropsWithDefaults & { - 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) => { - const xScale = zoom ? zoom.rescaleX(xScaleProp) : xScaleProp; -@@ -146,7 +147,7 @@ export const XAxis = < - - - ) : null} -- {font && labelWidth && canFitLabelContent ? ( -+ {font && labelWidth && (labelOverflow === "visible" || canFitLabelContent) ? ( - - = Required< - Omit< - XAxisInputProps, -- "font" | "tickValues" | "linePathEffect" | "enableRescaling" | "labelRotate" -+ "font" | "tickValues" | "linePathEffect" | "enableRescaling" | "labelRotate" | "labelOverflow" - > - > & - Partial< -@@ -220,6 +221,7 @@ export type XAxisPropsWithDefaults< - | "linePathEffect" - | "enableRescaling" - | "labelRotate" -+ | "labelOverflow" - > - >; - diff --git a/src/components/Charts/BarChart/BarChartContent.tsx b/src/components/Charts/BarChart/BarChartContent.tsx index 15de5c4bf21f8..12ac41ec711b6 100644 --- a/src/components/Charts/BarChart/BarChartContent.tsx +++ b/src/components/Charts/BarChart/BarChartContent.tsx @@ -3,11 +3,12 @@ import React, {useCallback, useMemo, useState} from 'react'; import type {LayoutChangeEvent} from 'react-native'; import {View} from 'react-native'; import {useSharedValue} from 'react-native-reanimated'; -import type {ChartBounds, PointsArray, Scale} from 'victory-native'; +import type {CartesianChartRenderArg, ChartBounds, PointsArray, Scale} from 'victory-native'; import {Bar, CartesianChart} from 'victory-native'; import ActivityIndicator from '@components/ActivityIndicator'; import ChartHeader from '@components/Charts/components/ChartHeader'; import ChartTooltip from '@components/Charts/components/ChartTooltip'; +import ChartXAxisLabels from '@components/Charts/components/ChartXAxisLabels'; import {AXIS_LABEL_GAP, CHART_CONTENT_MIN_HEIGHT, CHART_PADDING, X_AXIS_LINE_WIDTH, Y_AXIS_LINE_WIDTH, Y_AXIS_TICK_COUNT} from '@components/Charts/constants'; import fontSource from '@components/Charts/font'; import type {HitTestArgs} from '@components/Charts/hooks'; @@ -40,6 +41,8 @@ function BarChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUni const font = useFont(fontSource, variables.iconSizeExtraSmall); const [chartWidth, setChartWidth] = useState(0); const [barAreaWidth, setBarAreaWidth] = useState(0); + const [boundsLeft, setBoundsLeft] = useState(0); + const [boundsRight, setBoundsRight] = useState(0); const defaultBarColor = DEFAULT_CHART_COLOR; // prepare data for display @@ -75,6 +78,8 @@ function BarChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUni font, tickSpacing: barAreaWidth > 0 ? barAreaWidth / data.length : 0, labelAreaWidth: barAreaWidth, + firstTickLeftSpace: boundsLeft, + lastTickRightSpace: chartWidth > 0 ? chartWidth - boundsRight : 0, }); const domainPadding = useMemo(() => { @@ -85,14 +90,11 @@ function BarChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUni return {...BASE_DOMAIN_PADDING, left: horizontalPadding, right: horizontalPadding}; }, [chartWidth, data.length]); - const {formatLabel, formatValue} = useChartLabelFormats({ + const {formatValue} = useChartLabelFormats({ data, font, unit: yAxisUnit, unitPosition: yAxisUnitPosition, - labelSkipInterval, - labelRotation, - truncatedLabels, }); // Store bar geometry for hit-testing (only constants, no arrays) @@ -108,6 +110,8 @@ function BarChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUni chartBottom.set(bounds.bottom); yZero.set(0); setBarAreaWidth(domainWidth); + setBoundsLeft(bounds.left); + setBoundsRight(bounds.right); }, [data.length, barWidth, chartBottom, yZero], ); @@ -170,6 +174,27 @@ function BarChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUni [data, useSingleColor, defaultBarColor], ); + const renderCustomXLabels = useCallback( + (args: CartesianChartRenderArg<{x: number; y: number}, 'y'>) => { + if (!font) { + return null; + } + return ( + + ); + }, + [font, truncatedLabels, labelRotation, labelSkipInterval, theme.textSupporting], + ); + // When labels are rotated 90°, add measured label height to container // This keeps bar area at ~250px while giving labels their needed vertical space const dynamicChartStyle = useMemo( @@ -204,24 +229,17 @@ function BarChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUni {chartWidth > 0 && ( { setPlotAreaWidth(bounds.right - bounds.left); + setBoundsLeft(bounds.left); + setBoundsRight(bounds.right); }, []); - // Calculate dynamic domain padding for centered labels // Optimize by reducing wasted space when edge labels are shorter than tick spacing const domainPadding = useMemo(() => { if (chartWidth === 0 || data.length === 0) { @@ -82,12 +86,10 @@ function LineChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUn const geometricPadding = calculateMinDomainPadding(chartWidth, data.length); - // Without font, use geometric padding (safe fallback) if (!font) { return {...BASE_DOMAIN_PADDING, left: geometricPadding, right: geometricPadding}; } - // Measure edge labels to see if we can reduce padding const firstLabelWidth = measureTextWidth(data.at(0)?.label ?? '', font); const lastLabelWidth = measureTextWidth(data.at(-1)?.label ?? '', font); @@ -95,21 +97,19 @@ function LineChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUn const firstLabelNeeds = firstLabelWidth / 2; const lastLabelNeeds = lastLabelWidth / 2; - // How much space is wasted on each side const wastedLeft = geometricPadding - firstLabelNeeds; const wastedRight = geometricPadding - lastLabelNeeds; const reclaimablePadding = Math.min(wastedLeft, wastedRight); // Only reduce if both sides have excess space (labels short enough for 0°) - // If reclaimablePadding <= 0, labels are too long and hook will use rotation/truncation - const shouldUseExtraPadding = reclaimablePadding > 0; - const horizontalPadding = Math.max(shouldUseExtraPadding ? geometricPadding - reclaimablePadding : geometricPadding, MIN_SAFE_PADDING); + if (reclaimablePadding <= 0) { + return {...BASE_DOMAIN_PADDING, left: geometricPadding, right: geometricPadding}; + } - // If shouldUseExtraPadding is true then we have to add the extra padding to the right so the label is not clipped - return {...BASE_DOMAIN_PADDING, left: horizontalPadding, right: horizontalPadding + (shouldUseExtraPadding ? MIN_SAFE_PADDING : 0)}; + const horizontalPadding = Math.max(geometricPadding - reclaimablePadding, MIN_SAFE_PADDING); + return {...BASE_DOMAIN_PADDING, left: horizontalPadding, right: horizontalPadding}; }, [chartWidth, data, font]); - // For centered labels, tick spacing is evenly distributed across the plot area (same as BarChart) const tickSpacing = plotAreaWidth > 0 && data.length > 0 ? plotAreaWidth / data.length : 0; const {labelRotation, labelSkipInterval, truncatedLabels, xAxisLabelHeight} = useChartLabelLayout({ @@ -117,20 +117,11 @@ function LineChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUn font, tickSpacing, labelAreaWidth: plotAreaWidth, + firstTickLeftSpace: boundsLeft, + lastTickRightSpace: chartWidth > 0 ? chartWidth - boundsRight : 0, allowTightDiagonalPacking: true, }); - // Measure label widths for custom positioning in `renderOutside` - const labelWidths = useMemo(() => { - if (!font) { - return [] as number[]; - } - return truncatedLabels.map((label) => measureTextWidth(label, font)); - }, [font, truncatedLabels]); - - // Convert hook's degree rotation to radians for Skia rendering - const angleRad = (Math.abs(labelRotation) * Math.PI) / 180; - const {formatValue} = useChartLabelFormats({ data, font, @@ -153,68 +144,24 @@ function LineChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUn const tooltipData = useTooltipData(activeDataIndex, data, formatValue); - // Custom x-axis labels with hybrid positioning: - // - At 0° (horizontal): center label under the point (like bar chart) - // - At 45° (rotated): right-align so the last character is under the point const renderCustomXLabels = useCallback( (args: CartesianChartRenderArg<{x: number; y: number}, 'y'>) => { if (!font) { return null; } - - const fontMetrics = font.getMetrics(); - const ascent = Math.abs(fontMetrics.ascent); - const descent = Math.abs(fontMetrics.descent); - const labelY = args.chartBounds.bottom + AXIS_LABEL_GAP + rotatedLabelYOffset(ascent, descent, angleRad); - - return truncatedLabels.map((label, i) => { - if (i % labelSkipInterval !== 0) { - return null; - } - - const tickX = args.xScale(i); - const labelWidth = labelWidths.at(i) ?? 0; - - // At 0°: center the label under the point (like bar chart) - // At 45°: right-align so the last character is under the point - if (angleRad === 0) { - return ( - - ); - } - - const textX = tickX - labelWidth; // right-aligned for rotated labels - const origin = vec(tickX, labelY); - - // Rotate around the anchor, then translate to correct for ascent/descent - // asymmetry (ascent > descent shifts the visual center left of the anchor). - const correction = rotatedLabelCenterCorrection(ascent, descent, angleRad); - - return ( - - - - ); - }); + return ( + + ); }, - [font, truncatedLabels, labelSkipInterval, labelWidths, angleRad, theme.textSupporting], + [font, truncatedLabels, labelRotation, labelSkipInterval, theme.textSupporting], ); const dynamicChartStyle = useMemo( diff --git a/src/components/Charts/components/ChartXAxisLabels.tsx b/src/components/Charts/components/ChartXAxisLabels.tsx new file mode 100644 index 0000000000000..56c112f3882ec --- /dev/null +++ b/src/components/Charts/components/ChartXAxisLabels.tsx @@ -0,0 +1,75 @@ +import {Group, Text as SkiaText, vec} from '@shopify/react-native-skia'; +import type {SkFont} from '@shopify/react-native-skia'; +import React, {useMemo} from 'react'; +import {AXIS_LABEL_GAP} from '@components/Charts/constants'; +import {measureTextWidth, rotatedLabelCenterCorrection, rotatedLabelYOffset} from '@components/Charts/utils'; + +type ChartXAxisLabelsProps = { + labels: string[]; + labelRotation: number; + labelSkipInterval: number; + font: SkFont; + labelColor: string; + xScale: (value: number) => number; + chartBoundsBottom: number; + /** When true, rotated labels are centered on the tick. When false, they are right-aligned (end of text at tick). */ + centerRotatedLabels?: boolean; +}; + +function ChartXAxisLabels({labels, labelRotation, labelSkipInterval, font, labelColor, xScale, chartBoundsBottom, centerRotatedLabels = false}: ChartXAxisLabelsProps) { + const angleRad = (Math.abs(labelRotation) * Math.PI) / 180; + + const fontMetrics = font.getMetrics(); + const ascent = Math.abs(fontMetrics.ascent); + const descent = Math.abs(fontMetrics.descent); + const labelY = chartBoundsBottom + AXIS_LABEL_GAP + rotatedLabelYOffset(ascent, descent, angleRad); + const correction = rotatedLabelCenterCorrection(ascent, descent, angleRad); + + const labelWidths = useMemo(() => { + return labels.map((label) => measureTextWidth(label, font)); + }, [labels, font]); + + return labels.map((label, i) => { + if (i % labelSkipInterval !== 0 || label.length === 0) { + return null; + } + + const tickX = xScale(i); + const labelWidth = labelWidths.at(i) ?? 0; + + if (angleRad === 0) { + return ( + + ); + } + + const textX = centerRotatedLabels ? tickX - labelWidth / 2 : tickX - labelWidth; + const origin = vec(tickX, labelY); + + return ( + + + + ); + }); +} + +export default ChartXAxisLabels; +export type {ChartXAxisLabelsProps}; diff --git a/src/components/Charts/hooks/useChartLabelLayout.ts b/src/components/Charts/hooks/useChartLabelLayout.ts index 78564dfc131cc..c89f526a2ee41 100644 --- a/src/components/Charts/hooks/useChartLabelLayout.ts +++ b/src/components/Charts/hooks/useChartLabelLayout.ts @@ -25,7 +25,11 @@ type LabelLayoutConfig = { font: SkFont | null; tickSpacing: number; labelAreaWidth: number; - /** When true, allows tighter label packing at 45° by accounting for vertical offset. Useful for line charts. */ + /** Pixels from first tick to left edge of canvas. Defaults to Infinity (no constraint). */ + firstTickLeftSpace?: number; + /** Pixels from last tick to right edge of canvas. Defaults to Infinity (no constraint). */ + lastTickRightSpace?: number; + /** When true, allows tighter label packing at 45° by accounting for vertical offset between right-aligned labels. */ allowTightDiagonalPacking?: boolean; }; @@ -42,7 +46,7 @@ function truncateLabel(label: string, labelWidth: number, maxWidth: number, elli return label.slice(0, maxChars) + ELLIPSIS; } -/** Horizontal footprint of a label at a given rotation angle. */ +/** Horizontal footprint of a label at a given rotation angle (for inter-tick overlap checks). */ function effectiveWidth(labelWidth: number, lineHeight: number, rotation: number): number { if (rotation === LABEL_ROTATIONS.VERTICAL) { return lineHeight; @@ -70,37 +74,64 @@ function maxVisibleCount(areaWidth: number, itemWidth: number): number { } /** - * Pick the smallest rotation (0 → 45 → 90) where labels don't overlap, - * preferring rotation over skip interval. + * How far a label extends beyond its tick position after rotation. + * Accounts for the rotatedLabelCenterCorrection translateX applied during rendering. */ -function pickRotation( - maxLabelWidth: number, - lineHeight: number, - tickSpacing: number, - labelArea: number, - dataCount: number, - minTruncatedWidth: number, - allowTightDiagonalPacking: boolean, -): number { - // 0°: labels fit horizontally without truncation - const horizontalWidth = effectiveWidth(maxLabelWidth, lineHeight, LABEL_ROTATIONS.HORIZONTAL); - if (horizontalWidth + LABEL_PADDING <= tickSpacing && maxVisibleCount(labelArea, horizontalWidth) >= dataCount) { - return LABEL_ROTATIONS.HORIZONTAL; +function labelOverhang(labelWidth: number, lineHeight: number, rotation: number, rightAligned: boolean): {left: number; right: number} { + if (rotation === LABEL_ROTATIONS.HORIZONTAL) { + return {left: labelWidth / 2, right: labelWidth / 2}; } - - // 45°: viable if MIN_TRUNCATED_CHARS + ellipsis fits between ticks - // With tight packing, labels can overlap horizontally by lineHeight * sin(45°) due to vertical offset - const diagonalOverlap = allowTightDiagonalPacking ? lineHeight * SIN_45 : 0; - const minDiagonalWidth = minTruncatedWidth * SIN_45 - diagonalOverlap; - if (minDiagonalWidth + LABEL_PADDING <= tickSpacing) { - return LABEL_ROTATIONS.DIAGONAL; + if (rotation === LABEL_ROTATIONS.DIAGONAL) { + const halfLH = lineHeight / 2; + if (rightAligned) { + return { + left: (labelWidth + halfLH) * SIN_45, + right: halfLH * SIN_45, + }; + } + const overhang = (labelWidth / 2 + halfLH) * SIN_45; + return {left: overhang, right: overhang}; } + return {left: lineHeight / 2, right: lineHeight / 2}; +} + +/** Check if first and last labels fit within the available canvas edge space. */ +function edgeLabelsFit( + firstLabelWidth: number, + lastLabelWidth: number, + lineHeight: number, + rotation: number, + firstTickLeftSpace: number, + lastTickRightSpace: number, + rightAligned: boolean, +): boolean { + const first = labelOverhang(firstLabelWidth, lineHeight, rotation, rightAligned); + const last = labelOverhang(lastLabelWidth, lineHeight, rotation, rightAligned); + return first.left <= firstTickLeftSpace && last.right <= lastTickRightSpace; +} - // 90°: fallback - return LABEL_ROTATIONS.VERTICAL; +/** + * Maximum label width that fits within the available edge space at a given rotation. + * Returns Infinity when the overhang at that edge doesn't depend on label width. + */ +function edgeMaxLabelWidth(edgeSpace: number, lineHeight: number, rotation: number, rightAligned: boolean, edge: 'first' | 'last'): number { + const halfLH = lineHeight / 2; + if (rotation === LABEL_ROTATIONS.HORIZONTAL) { + return 2 * edgeSpace; + } + if (rotation === LABEL_ROTATIONS.DIAGONAL) { + if (rightAligned) { + // Right-aligned: only first label can overflow left, last label's right overhang is constant + return edge === 'first' ? Math.max(0, edgeSpace / SIN_45 - halfLH) : Infinity; + } + // Centered: symmetric overhang on both edges + return Math.max(0, 2 * (edgeSpace / SIN_45 - halfLH)); + } + // Vertical: overhang is lineHeight/2, doesn't depend on label width + return Infinity; } -function useChartLabelLayout({data, font, tickSpacing, labelAreaWidth, allowTightDiagonalPacking = false}: LabelLayoutConfig) { +function useChartLabelLayout({data, font, tickSpacing, labelAreaWidth, firstTickLeftSpace = Infinity, lastTickRightSpace = Infinity, allowTightDiagonalPacking = false}: LabelLayoutConfig) { return useMemo(() => { if (!font || data.length === 0 || tickSpacing <= 0 || labelAreaWidth <= 0) { return {labelRotation: 0, labelSkipInterval: 1, truncatedLabels: []}; @@ -110,10 +141,12 @@ function useChartLabelLayout({data, font, tickSpacing, labelAreaWidth, allowTigh const lineHeight = Math.abs(fontMetrics.descent) + Math.abs(fontMetrics.ascent); const ellipsisWidth = measureTextWidth(ELLIPSIS, font); const labelWidths = data.map((point) => measureTextWidth(point.label, font)); - const maxLabelLength = Math.max(...labelWidths); + const maxLabelWidth = Math.max(...labelWidths); + const firstLabelWidth = labelWidths.at(0) ?? 0; + const lastLabelWidth = labelWidths.at(-1) ?? 0; + const rightAligned = allowTightDiagonalPacking; // Maximum width of labels after truncation to MIN_TRUNCATED_CHARS characters. - // Labels shorter than the threshold keep their full width (can't be truncated further). const minTruncatedWidth = Math.max( ...data.map((point, index) => { if (point.label.length <= MIN_TRUNCATED_CHARS) { @@ -123,18 +156,58 @@ function useChartLabelLayout({data, font, tickSpacing, labelAreaWidth, allowTigh }), ); - // 1. Pick rotation - const rotation = pickRotation(maxLabelLength, lineHeight, tickSpacing, labelAreaWidth, data.length, minTruncatedWidth, allowTightDiagonalPacking); + // Min truncated widths for edge labels specifically + const firstLabel = data.at(0)?.label ?? ''; + const lastLabel = data.at(-1)?.label ?? ''; + const firstMinTrunc = firstLabel.length <= MIN_TRUNCATED_CHARS ? firstLabelWidth : measureTextWidth(firstLabel.slice(0, MIN_TRUNCATED_CHARS) + ELLIPSIS, font); + const lastMinTrunc = lastLabel.length <= MIN_TRUNCATED_CHARS ? lastLabelWidth : measureTextWidth(lastLabel.slice(0, MIN_TRUNCATED_CHARS) + ELLIPSIS, font); + + // --- 1. Pick rotation (prefer 0° → 45° → 90°) --- + let rotation: number = LABEL_ROTATIONS.VERTICAL; + + // 0°: labels fit horizontally without truncation AND edge labels fit + const hWidth = effectiveWidth(maxLabelWidth, lineHeight, LABEL_ROTATIONS.HORIZONTAL); + const hFitsInTicks = hWidth + LABEL_PADDING <= tickSpacing && maxVisibleCount(labelAreaWidth, hWidth) >= data.length; + const hEdgeFits = edgeLabelsFit(firstLabelWidth, lastLabelWidth, lineHeight, LABEL_ROTATIONS.HORIZONTAL, firstTickLeftSpace, lastTickRightSpace, false); + if (hFitsInTicks && hEdgeFits) { + rotation = LABEL_ROTATIONS.HORIZONTAL; + } else { + // 45°: viable if MIN_TRUNCATED_CHARS + ellipsis fits between ticks + const diagonalOverlap = allowTightDiagonalPacking ? lineHeight * SIN_45 : 0; + const minDiagWidth = minTruncatedWidth * SIN_45 - diagonalOverlap; + const dFitsInTicks = minDiagWidth + LABEL_PADDING <= tickSpacing; + + // Edge check: can first/last labels show at least MIN_TRUNCATED_CHARS at 45°? + const firstEdgeMax = edgeMaxLabelWidth(firstTickLeftSpace, lineHeight, LABEL_ROTATIONS.DIAGONAL, rightAligned, 'first'); + const lastEdgeMax = edgeMaxLabelWidth(lastTickRightSpace, lineHeight, LABEL_ROTATIONS.DIAGONAL, rightAligned, 'last'); + const dEdgeFits = firstEdgeMax >= firstMinTrunc && lastEdgeMax >= lastMinTrunc; + + if (dFitsInTicks && dEdgeFits) { + rotation = LABEL_ROTATIONS.DIAGONAL; + } + } + + // --- 2. Truncate labels --- + const truncDiagonalOverlap = allowTightDiagonalPacking ? lineHeight : 0; + const tickMaxWidth = rotation === LABEL_ROTATIONS.DIAGONAL ? (tickSpacing - LABEL_PADDING) / SIN_45 + truncDiagonalOverlap : Infinity; - // 2. Truncate labels (only at 45°) - // With tight packing, labels can be longer due to allowed horizontal overlap - const diagonalOverlap = allowTightDiagonalPacking ? lineHeight : 0; - const maxLabelWidth = rotation === LABEL_ROTATIONS.DIAGONAL ? (tickSpacing - LABEL_PADDING) / SIN_45 + diagonalOverlap : Infinity; const finalLabels = data.map((point, index) => { - return truncateLabel(point.label, labelWidths.at(index) ?? 0, maxLabelWidth, ellipsisWidth); + let maxWidth = tickMaxWidth; + + // Apply edge constraints for first/last labels + if (index === 0) { + const edgeMax = edgeMaxLabelWidth(firstTickLeftSpace, lineHeight, rotation, rightAligned, 'first'); + maxWidth = Math.min(maxWidth, edgeMax); + } + if (index === data.length - 1) { + const edgeMax = edgeMaxLabelWidth(lastTickRightSpace, lineHeight, rotation, rightAligned, 'last'); + maxWidth = Math.min(maxWidth, edgeMax); + } + + return truncateLabel(point.label, labelWidths.at(index) ?? 0, maxWidth, ellipsisWidth); }); - // 3. Compute skip interval (only at 90°) + // --- 3. Compute skip interval (only at 90°) --- const finalMaxWidth = Math.max(...finalLabels.map((label) => measureTextWidth(label, font))); let skipInterval = 1; if (rotation === LABEL_ROTATIONS.VERTICAL) { @@ -143,7 +216,7 @@ function useChartLabelLayout({data, font, tickSpacing, labelAreaWidth, allowTigh skipInterval = visibleCount >= data.length ? 1 : Math.ceil(data.length / Math.max(1, visibleCount)); } - // 4. Compute vertical space needed for x-axis labels + // --- 4. Compute vertical space needed for x-axis labels --- const xAxisLabelHeight = effectiveHeight(finalMaxWidth, lineHeight, rotation); return { @@ -152,7 +225,7 @@ function useChartLabelLayout({data, font, tickSpacing, labelAreaWidth, allowTigh truncatedLabels: finalLabels, xAxisLabelHeight, }; - }, [font, tickSpacing, labelAreaWidth, data, allowTightDiagonalPacking]); + }, [font, tickSpacing, labelAreaWidth, firstTickLeftSpace, lastTickRightSpace, data, allowTightDiagonalPacking]); } export {LABEL_ROTATIONS, useChartLabelLayout}; From 5916e9b517a98f215be80b0de9ea803050e23707 Mon Sep 17 00:00:00 2001 From: Mateusz Rajski Date: Wed, 25 Feb 2026 15:44:47 +0100 Subject: [PATCH 02/13] Add tests that cover useChartLabelLayout logic --- tests/unit/Charts/useChartLabelLayout.test.ts | 281 ++++++++++++++++++ 1 file changed, 281 insertions(+) create mode 100644 tests/unit/Charts/useChartLabelLayout.test.ts diff --git a/tests/unit/Charts/useChartLabelLayout.test.ts b/tests/unit/Charts/useChartLabelLayout.test.ts new file mode 100644 index 0000000000000..759d20ba7f23c --- /dev/null +++ b/tests/unit/Charts/useChartLabelLayout.test.ts @@ -0,0 +1,281 @@ +import type {SkFont} from '@shopify/react-native-skia'; +import {renderHook} from '@testing-library/react-native'; +import {useChartLabelLayout} from '@components/Charts/hooks/useChartLabelLayout'; +import type {ChartDataPoint} from '@components/Charts/types'; + +/** + * Each glyph = PX_PER_CHAR wide. This gives deterministic widths: + * "AAA" = 21px, "AAAAAA" = 42px, "A".repeat(16) = 112px, "..." = 21px + */ +const PX_PER_CHAR = 7; + +function createMockFont(): SkFont { + return { + getMetrics: () => ({ascent: -12, descent: 4, leading: 0}), + getGlyphIDs: (text: string) => [...text].map((_, i) => i + 1), + getGlyphWidths: (glyphIDs: number[]) => glyphIDs.map(() => PX_PER_CHAR), + getSize: () => 12, + } as unknown as SkFont; +} + +function makeData(...labels: string[]): ChartDataPoint[] { + return labels.map((label, i) => ({label, total: (i + 1) * 100})); +} + +const SIN_45 = Math.sin(Math.PI / 4); +const LINE_HEIGHT = 16; // |ascent(12)| + |descent(4)| + +describe('useChartLabelLayout', () => { + const font = createMockFont(); + + describe('early returns', () => { + const defaults = {labelRotation: 0, labelSkipInterval: 1, truncatedLabels: []}; + + it('returns defaults when font is null', () => { + const {result} = renderHook(() => useChartLabelLayout({data: makeData('A', 'B'), font: null, tickSpacing: 50, labelAreaWidth: 100})); + expect(result.current).toEqual(defaults); + }); + + it('returns defaults when data is empty', () => { + const {result} = renderHook(() => useChartLabelLayout({data: [], font, tickSpacing: 50, labelAreaWidth: 100})); + expect(result.current).toEqual(defaults); + }); + + it('returns defaults when tickSpacing is 0', () => { + const {result} = renderHook(() => useChartLabelLayout({data: makeData('A', 'B'), font, tickSpacing: 0, labelAreaWidth: 100})); + expect(result.current).toEqual(defaults); + }); + + it('returns defaults when labelAreaWidth is 0', () => { + const {result} = renderHook(() => useChartLabelLayout({data: makeData('A', 'B'), font, tickSpacing: 50, labelAreaWidth: 0})); + expect(result.current).toEqual(defaults); + }); + }); + + describe('rotation selection without edge constraints', () => { + it('picks 0° when labels fit horizontally', () => { + // "AAA" = 21px. 21+4=25 ≤ tickSpacing(30). maxVisibleCount(90,21)=3 ≥ 3 + const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAA', 'BBB', 'CCC'), font, tickSpacing: 30, labelAreaWidth: 90})); + expect(result.current.labelRotation).toBe(-0); + expect(result.current.truncatedLabels).toEqual(['AAA', 'BBB', 'CCC']); + }); + + it('picks 45° when labels overflow horizontally but fit diagonally', () => { + // "AAAAAA" = 42px. 42+4=46 > tickSpacing(40) → 0° fails. + // At 45°: 42*SIN_45 ≈ 29.7, 29.7+4 ≤ 40 ✓ + const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 40, labelAreaWidth: 400})); + expect(result.current.labelRotation).toBe(-45); + expect(result.current.truncatedLabels).toEqual(['AAAAAA', 'BBBBBB']); + }); + + it('picks 90° when labels overflow at all rotations', () => { + // tickSpacing=20: 0° fails (46>20), 45° fails (29.7+4=33.7>20) + const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 20, labelAreaWidth: 400})); + expect(result.current.labelRotation).toBe(-90); + expect(result.current.truncatedLabels).toEqual(['AAAAAA', 'BBBBBB']); + }); + }); + + describe('backward compatibility', () => { + it('produces identical result whether edge params are omitted or set to Infinity', () => { + const config = {data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 40, labelAreaWidth: 400}; + const {result: withoutEdge} = renderHook(() => useChartLabelLayout(config)); + const {result: withEdge} = renderHook(() => useChartLabelLayout({...config, firstTickLeftSpace: Infinity, lastTickRightSpace: Infinity})); + expect(withoutEdge.current).toEqual(withEdge.current); + }); + + it('Infinity edge space never constrains rotation', () => { + const {result} = renderHook(() => + useChartLabelLayout({ + data: makeData('AAAAAA', 'BBBBBB', 'CCCCCC'), + font, + tickSpacing: 50, + labelAreaWidth: 150, + firstTickLeftSpace: Infinity, + lastTickRightSpace: Infinity, + }), + ); + expect(result.current.labelRotation).toBe(-0); + }); + }); + + describe('edge-constrained rotation', () => { + it('same data picks 0° without edge constraint but 45° with edge constraint', () => { + // "A".repeat(16) = 112px. At 0°: overhang = 56px. + const config = {data: makeData('A'.repeat(16), 'BB', 'CC'), font, tickSpacing: 120, labelAreaWidth: 360}; + + const {result: noEdge} = renderHook(() => useChartLabelLayout(config)); + expect(noEdge.current.labelRotation).toBe(-0); + + // firstTickLeftSpace=40 < 56 → 0° edge fails → escalates to 45° + const {result: withEdge} = renderHook(() => useChartLabelLayout({...config, firstTickLeftSpace: 40, lastTickRightSpace: 200})); + expect(withEdge.current.labelRotation).toBe(-45); + }); + + it('escalates to 90° when edge space is too small for both 0° and 45°', () => { + // firstTickLeftSpace=5: at 45° centered edgeMax = max(0, 2*(5/SIN_45-8)) ≈ 0 → fails + const {result} = renderHook(() => + useChartLabelLayout({data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 50, labelAreaWidth: 200, firstTickLeftSpace: 5, lastTickRightSpace: 5}), + ); + expect(result.current.labelRotation).toBe(-90); + }); + + it('allowTightDiagonalPacking enables 45° at tighter tick spacing', () => { + // "AAAAAA" = 42px. tickSpacing=30. + // Without packing: minDiagWidth = 42*SIN_45 ≈ 29.7, 29.7+4=33.7 > 30 → 45° fails + // With packing: diagonalOverlap = 16*SIN_45 ≈ 11.3, minDiagWidth = 29.7-11.3=18.4, 18.4+4=22.4 ≤ 30 ✓ + const base = {data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 30, labelAreaWidth: 400, firstTickLeftSpace: 100, lastTickRightSpace: 100}; + + const {result: noPacking} = renderHook(() => useChartLabelLayout({...base, allowTightDiagonalPacking: false})); + expect(noPacking.current.labelRotation).toBe(-90); + + const {result: withPacking} = renderHook(() => useChartLabelLayout({...base, allowTightDiagonalPacking: true})); + expect(withPacking.current.labelRotation).toBe(-45); + }); + }); + + describe('edge-aware truncation', () => { + it('truncates first label due to edge constraint while middle labels remain full (centered)', () => { + // First label: 16 chars = 112px. tickMaxWidth ≈ 164. edgeMax ≈ 97 (stricter). + // Truncated to 10 chars + "..." + const {result} = renderHook(() => + useChartLabelLayout({ + data: makeData('A'.repeat(16), 'BB', 'CC'), + font, + tickSpacing: 120, + labelAreaWidth: 360, + firstTickLeftSpace: 40, + lastTickRightSpace: 200, + }), + ); + expect(result.current.labelRotation).toBe(-45); + expect(result.current.truncatedLabels?.at(0)).toBe(`${'A'.repeat(10)}...`); + expect(result.current.truncatedLabels?.at(1)).toBe('BB'); + expect(result.current.truncatedLabels?.at(2)).toBe('CC'); + }); + + it('truncates first label due to edge constraint (right-aligned)', () => { + // Right-aligned first label: edgeMax = 72/SIN_45 - 8 ≈ 93.8. tickMax ≈ 95.2. + // Edge is stricter → first label truncated to 10 chars + "..." + const {result} = renderHook(() => + useChartLabelLayout({ + data: makeData('A'.repeat(16), 'BB', 'CC'), + font, + tickSpacing: 60, + labelAreaWidth: 360, + firstTickLeftSpace: 72, + lastTickRightSpace: 200, + allowTightDiagonalPacking: true, + }), + ); + expect(result.current.labelRotation).toBe(-45); + expect(result.current.truncatedLabels?.at(0)).toBe(`${'A'.repeat(10)}...`); + expect(result.current.truncatedLabels?.at(1)).toBe('BB'); + }); + + it('truncates last label when centered due to symmetric overhang', () => { + // Centered: last label right overhang = (W/2+halfLH)*SIN_45 ≈ 45.6 for W=112 + // lastTickRightSpace=40: edgeMax = 2*(40/SIN_45-8) ≈ 97.1 < 112 → truncated + const {result} = renderHook(() => + useChartLabelLayout({ + data: makeData('AA', 'BB', 'A'.repeat(16)), + font, + tickSpacing: 200, + labelAreaWidth: 600, + firstTickLeftSpace: 200, + lastTickRightSpace: 40, + }), + ); + expect(result.current.labelRotation).toBe(-45); + expect(result.current.truncatedLabels?.at(2)).toBe(`${'A'.repeat(10)}...`); + }); + + it('does NOT truncate last label when right-aligned despite tight right edge', () => { + // Right-aligned: last label right overhang = halfLH*SIN_45 ≈ 5.6 (constant, tiny). + // lastTickRightSpace=40 >> 5.6 → edgeMax = Infinity → no edge truncation. + // tickMaxWidth = (200-4)/SIN_45+16 ≈ 293 > 112 → no tick truncation either. + const {result} = renderHook(() => + useChartLabelLayout({ + data: makeData('AA', 'BB', 'A'.repeat(16)), + font, + tickSpacing: 200, + labelAreaWidth: 600, + firstTickLeftSpace: 200, + lastTickRightSpace: 40, + allowTightDiagonalPacking: true, + }), + ); + expect(result.current.labelRotation).toBe(-45); + expect(result.current.truncatedLabels?.at(2)).toBe('A'.repeat(16)); + }); + }); + + describe('skip interval', () => { + it('computes skip interval > 1 at 90° when too many labels for the area', () => { + // 10 labels, forced to 90°. verticalWidth = lineHeight = 16. + // maxVisibleCount(100, 16) = floor(100/20) = 5 < 10 → skip = ceil(10/5) = 2 + const labels = Array.from({length: 10}, (_, i) => `L${String(i).padStart(4, '0')}`); + const {result} = renderHook(() => useChartLabelLayout({data: makeData(...labels), font, tickSpacing: 10, labelAreaWidth: 100})); + expect(result.current.labelRotation).toBe(-90); + expect(result.current.labelSkipInterval).toBe(2); + }); + + it('returns skip interval 1 at 90° when labels fit', () => { + const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 10, labelAreaWidth: 400})); + expect(result.current.labelRotation).toBe(-90); + expect(result.current.labelSkipInterval).toBe(1); + }); + + it('never uses skip interval at 0° or 45°', () => { + const {result: h} = renderHook(() => useChartLabelLayout({data: makeData('AAA', 'BBB'), font, tickSpacing: 50, labelAreaWidth: 200})); + expect(h.current.labelRotation).toBe(-0); + expect(h.current.labelSkipInterval).toBe(1); + + const {result: d} = renderHook(() => useChartLabelLayout({data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 40, labelAreaWidth: 400})); + expect(d.current.labelRotation).toBe(-45); + expect(d.current.labelSkipInterval).toBe(1); + }); + }); + + describe('xAxisLabelHeight', () => { + it('equals lineHeight at 0°', () => { + const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAA', 'BBB'), font, tickSpacing: 50, labelAreaWidth: 200})); + expect(result.current.labelRotation).toBe(-0); + expect(result.current.xAxisLabelHeight).toBe(LINE_HEIGHT); + }); + + it('uses diagonal formula at 45°', () => { + // effectiveHeight = (finalMaxWidth + lineHeight) * SIN_45. finalMaxWidth = 42 (no truncation) + const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 40, labelAreaWidth: 400})); + expect(result.current.labelRotation).toBe(-45); + expect(result.current.xAxisLabelHeight).toBeCloseTo((42 + LINE_HEIGHT) * SIN_45, 5); + }); + + it('equals label width at 90°', () => { + // effectiveHeight at 90° = finalMaxWidth = 42 + const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 10, labelAreaWidth: 400})); + expect(result.current.labelRotation).toBe(-90); + expect(result.current.xAxisLabelHeight).toBe(42); + }); + }); + + describe('edge cases', () => { + it('handles single data point', () => { + const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAA'), font, tickSpacing: 50, labelAreaWidth: 50})); + expect(result.current.truncatedLabels).toEqual(['AAA']); + expect(result.current.labelSkipInterval).toBe(1); + }); + + it('preserves all labels at 0° without truncation', () => { + const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAA', 'BBB'), font, tickSpacing: 50, labelAreaWidth: 200})); + expect(result.current.labelRotation).toBe(-0); + expect(result.current.truncatedLabels).toEqual(['AAA', 'BBB']); + }); + + it('preserves labels at 90° without truncation', () => { + const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 10, labelAreaWidth: 400})); + expect(result.current.labelRotation).toBe(-90); + expect(result.current.truncatedLabels).toEqual(['AAAAAA', 'BBBBBB']); + }); + }); +}); From ca13d99b33ba9ed8d87095d43c675a5cd6321d90 Mon Sep 17 00:00:00 2001 From: Mateusz Rajski Date: Mon, 2 Mar 2026 12:33:16 +0100 Subject: [PATCH 03/13] Fix incorrect calculations when deciding which rotation and truncation we should pick --- .../Charts/BarChart/BarChartContent.tsx | 21 +-- .../Charts/LineChart/LineChartContent.tsx | 10 +- .../Charts/components/ChartXAxisLabels.tsx | 6 +- .../Charts/hooks/useChartLabelLayout.ts | 4 +- src/components/Search/SearchChartView.tsx | 140 +----------------- 5 files changed, 29 insertions(+), 152 deletions(-) diff --git a/src/components/Charts/BarChart/BarChartContent.tsx b/src/components/Charts/BarChart/BarChartContent.tsx index 74725e53d4500..b3625c06c9a21 100644 --- a/src/components/Charts/BarChart/BarChartContent.tsx +++ b/src/components/Charts/BarChart/BarChartContent.tsx @@ -75,15 +75,6 @@ function BarChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUni setChartWidth(event.nativeEvent.layout.width); }, []); - const {labelRotation, labelSkipInterval, truncatedLabels, xAxisLabelHeight} = useChartLabelLayout({ - data, - font, - tickSpacing: barAreaWidth > 0 ? barAreaWidth / data.length : 0, - labelAreaWidth: barAreaWidth, - firstTickLeftSpace: boundsLeft, - lastTickRightSpace: chartWidth > 0 ? chartWidth - boundsRight : 0, - }); - const domainPadding = useMemo(() => { if (chartWidth === 0) { return BASE_DOMAIN_PADDING; @@ -92,6 +83,18 @@ function BarChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUni return {...BASE_DOMAIN_PADDING, left: horizontalPadding, right: horizontalPadding}; }, [chartWidth, data.length]); + const totalDomainPadding = domainPadding.left + domainPadding.right; + const paddingScale = barAreaWidth > 0 ? barAreaWidth / (barAreaWidth + totalDomainPadding) : 0; + + const {labelRotation, labelSkipInterval, truncatedLabels, xAxisLabelHeight} = useChartLabelLayout({ + data, + font, + tickSpacing: barAreaWidth > 0 ? barAreaWidth / data.length : 0, + labelAreaWidth: barAreaWidth, + firstTickLeftSpace: boundsLeft + domainPadding.left * paddingScale, + lastTickRightSpace: chartWidth > 0 ? chartWidth - boundsRight + domainPadding.right * paddingScale : 0, + }); + const {formatValue} = useChartLabelFormats({ data, font, diff --git a/src/components/Charts/LineChart/LineChartContent.tsx b/src/components/Charts/LineChart/LineChartContent.tsx index 27c90c57bb5ec..7a7959a754606 100644 --- a/src/components/Charts/LineChart/LineChartContent.tsx +++ b/src/components/Charts/LineChart/LineChartContent.tsx @@ -113,13 +113,19 @@ function LineChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUn const tickSpacing = plotAreaWidth > 0 && data.length > 0 ? plotAreaWidth / data.length : 0; + // Victory's domainPadding extends the data domain, not the pixel range directly. + // The actual pixel offset for the first tick is smaller than domainPadding when + // the padding is large relative to the plot area. + const totalDomainPadding = domainPadding.left + domainPadding.right; + const paddingScale = plotAreaWidth > 0 ? plotAreaWidth / (plotAreaWidth + totalDomainPadding) : 0; + const {labelRotation, labelSkipInterval, truncatedLabels, xAxisLabelHeight} = useChartLabelLayout({ data, font, tickSpacing, labelAreaWidth: plotAreaWidth, - firstTickLeftSpace: boundsLeft, - lastTickRightSpace: chartWidth > 0 ? chartWidth - boundsRight : 0, + firstTickLeftSpace: boundsLeft + domainPadding.left * paddingScale, + lastTickRightSpace: chartWidth > 0 ? chartWidth - boundsRight + domainPadding.right * paddingScale : 0, allowTightDiagonalPacking: true, }); diff --git a/src/components/Charts/components/ChartXAxisLabels.tsx b/src/components/Charts/components/ChartXAxisLabels.tsx index 56c112f3882ec..f87c0fa28bcbf 100644 --- a/src/components/Charts/components/ChartXAxisLabels.tsx +++ b/src/components/Charts/components/ChartXAxisLabels.tsx @@ -22,13 +22,17 @@ function ChartXAxisLabels({labels, labelRotation, labelSkipInterval, font, label const fontMetrics = font.getMetrics(); const ascent = Math.abs(fontMetrics.ascent); const descent = Math.abs(fontMetrics.descent); - const labelY = chartBoundsBottom + AXIS_LABEL_GAP + rotatedLabelYOffset(ascent, descent, angleRad); const correction = rotatedLabelCenterCorrection(ascent, descent, angleRad); const labelWidths = useMemo(() => { return labels.map((label) => measureTextWidth(label, font)); }, [labels, font]); + // Centered labels extend upward by (maxWidth/2)*sin(angle) from the anchor; + // push the anchor down so the top of the bounding box clears chartBoundsBottom. + const centeredUpwardOffset = centerRotatedLabels && angleRad > 0 ? (Math.max(...labelWidths) / 2) * Math.sin(angleRad) : 0; + const labelY = chartBoundsBottom + AXIS_LABEL_GAP + rotatedLabelYOffset(ascent, descent, angleRad) + centeredUpwardOffset; + return labels.map((label, i) => { if (i % labelSkipInterval !== 0 || label.length === 0) { return null; diff --git a/src/components/Charts/hooks/useChartLabelLayout.ts b/src/components/Charts/hooks/useChartLabelLayout.ts index c89f526a2ee41..cc2c7eb324a9d 100644 --- a/src/components/Charts/hooks/useChartLabelLayout.ts +++ b/src/components/Charts/hooks/useChartLabelLayout.ts @@ -169,6 +169,7 @@ function useChartLabelLayout({data, font, tickSpacing, labelAreaWidth, firstTick const hWidth = effectiveWidth(maxLabelWidth, lineHeight, LABEL_ROTATIONS.HORIZONTAL); const hFitsInTicks = hWidth + LABEL_PADDING <= tickSpacing && maxVisibleCount(labelAreaWidth, hWidth) >= data.length; const hEdgeFits = edgeLabelsFit(firstLabelWidth, lastLabelWidth, lineHeight, LABEL_ROTATIONS.HORIZONTAL, firstTickLeftSpace, lastTickRightSpace, false); + if (hFitsInTicks && hEdgeFits) { rotation = LABEL_ROTATIONS.HORIZONTAL; } else { @@ -208,7 +209,8 @@ function useChartLabelLayout({data, font, tickSpacing, labelAreaWidth, firstTick }); // --- 3. Compute skip interval (only at 90°) --- - const finalMaxWidth = Math.max(...finalLabels.map((label) => measureTextWidth(label, font))); + const finalWidths = finalLabels.map((label) => measureTextWidth(label, font)); + const finalMaxWidth = Math.max(...finalWidths); let skipInterval = 1; if (rotation === LABEL_ROTATIONS.VERTICAL) { const verticalWidth = effectiveWidth(finalMaxWidth, lineHeight, rotation); diff --git a/src/components/Search/SearchChartView.tsx b/src/components/Search/SearchChartView.tsx index b7b913812e99a..598e65d6904bf 100644 --- a/src/components/Search/SearchChartView.tsx +++ b/src/components/Search/SearchChartView.tsx @@ -30,138 +30,6 @@ import SearchLineChart from './SearchLineChart'; import SearchPieChart from './SearchPieChart'; import type {ChartView, GroupedItem, SearchChartProps, SearchGroupBy, SearchQueryJSON} from './types'; -// --- DEBUG MOCK DATA (set MOCK_DATA_INDEX to -1 to disable) --- -const MOCK_DATA_INDEX = -1; - -function mockMonthItems(entries: Array<[string, number]>): GroupedItem[] { - return entries.map(([label, total]) => ({formattedMonth: label, total: total * 100, currency: 'USD', month: 1, year: 2025}) as unknown as GroupedItem); -} - -const MOCK_DATASETS: Array<{label: string; groupBy: SearchGroupBy; data: GroupedItem[]}> = [ - { - label: '0: 3 short labels', - groupBy: CONST.SEARCH.GROUP_BY.MONTH, - data: mockMonthItems([ - ['Jan', 1200], - ['Feb', 3400], - ['Mar', 2100], - ]), - }, - { - label: '1: 12 short labels (full year)', - groupBy: CONST.SEARCH.GROUP_BY.MONTH, - data: mockMonthItems([ - ['Jan', 1200], - ['Feb', 3400], - ['Mar', 2100], - ['Apr', 800], - ['May', 4500], - ['Jun', 3200], - ['Jul', 2900], - ['Aug', 1100], - ['Sep', 5600], - ['Oct', 4100], - ['Nov', 2700], - ['Dec', 3800], - ]), - }, - { - label: '2: 6 long labels (full month names)', - groupBy: CONST.SEARCH.GROUP_BY.MONTH, - data: mockMonthItems([ - ['January', 1200], - ['February', 3400], - ['March', 2100], - ['April', 800], - ['May', 4500], - ['June', 3200], - ]), - }, - { - label: '3: 12 long labels (month + year)', - groupBy: CONST.SEARCH.GROUP_BY.MONTH, - data: mockMonthItems([ - ['January 2024', 1200], - ['February 2024', 3400], - ['March 2024', 2100], - ['April 2024', 800], - ['May 2024', 4500], - ['June 2024', 3200], - ['July 2024', 2900], - ['August 2024', 1100], - ['September 2024', 5600], - ['October 2024', 4100], - ['November 2024', 2700], - ['December 2024', 3800], - ]), - }, - { - label: '4: mixed lengths (long edges)', - groupBy: CONST.SEARCH.GROUP_BY.MONTH, - data: mockMonthItems([ - ['January 2024', 1200], - ['Feb', 3400], - ['Mar', 2100], - ['Apr', 800], - ['December 2024', 3800], - ]), - }, - { - label: '5: very long labels (categories)', - groupBy: CONST.SEARCH.GROUP_BY.MONTH, - data: mockMonthItems([ - ['Office Supplies & Equipment', 4200], - ['Travel & Transportation', 3100], - ['Software Subscriptions', 2800], - ['Professional Services', 1900], - ]), - }, - { - label: '6: 2 data points', - groupBy: CONST.SEARCH.GROUP_BY.MONTH, - data: mockMonthItems([ - ['January 2024', 5000], - ['February 2024', 3200], - ]), - }, - { - label: '7: 24 data points (2 years monthly)', - groupBy: CONST.SEARCH.GROUP_BY.MONTH, - data: mockMonthItems([ - ['Jan 24', 12], - ['Feb 24', 34], - ['Mar 24', 21], - ['Apr 24', 8], - ['May 24', 45], - ['Jun 24', 32], - ['Jul 24', 29], - ['Aug 24', 11], - ['Sep 24', 56], - ['Oct 24', 41], - ['Nov 24', 27], - ['Dec 24', 38], - ['Jan 25', 15], - ['Feb 25', 42], - ['Mar 25', 19], - ['Apr 25', 33], - ['May 25', 51], - ['Jun 25', 22], - ['Jul 25', 37], - ['Aug 25', 14], - ['Sep 25', 48], - ['Oct 25', 30], - ['Nov 25', 25], - ['Dec 25', 44], - ]), - }, - { - label: '8: single data point', - groupBy: CONST.SEARCH.GROUP_BY.MONTH, - data: mockMonthItems([['September 2024', 4200]]), - }, -]; -// --- END DEBUG MOCK DATA --- - type ChartGroupByConfig = { titleIconName: 'Users' | 'CreditCard' | 'Send' | 'Folder' | 'Basket' | 'Tag' | 'Calendar'; getLabel: (item: GroupedItem) => string; @@ -278,18 +146,12 @@ const CHART_VIEW_TO_COMPONENT: Record= 0 --- - const mockDataset = MOCK_DATA_INDEX >= 0 ? MOCK_DATASETS.at(MOCK_DATA_INDEX) : undefined; - const data = mockDataset?.data ?? dataProp; - const groupBy = mockDataset?.groupBy ?? groupByProp; - // --- END DEBUG --- - const {titleIconName, getLabel, getFilterQuery} = CHART_GROUP_BY_CONFIG[groupBy]; const titleIcon = icons[titleIconName]; const ChartComponent = CHART_VIEW_TO_COMPONENT[view]; From ba83eceb577c4da6a95922208b6291f9a6a2a3e7 Mon Sep 17 00:00:00 2001 From: Mateusz Rajski Date: Mon, 2 Mar 2026 13:32:26 +0100 Subject: [PATCH 04/13] Add more tests --- .../Charts/hooks/useChartLabelFormats.ts | 2 +- .../Charts/hooks/useChartLabelLayout.ts | 132 ++---------------- src/components/Charts/utils.ts | 124 ++++++++++++++++ tests/unit/Charts/useChartLabelLayout.test.ts | 8 ++ 4 files changed, 148 insertions(+), 118 deletions(-) diff --git a/src/components/Charts/hooks/useChartLabelFormats.ts b/src/components/Charts/hooks/useChartLabelFormats.ts index 3c6ab6f71cca7..4948d90f0eff4 100644 --- a/src/components/Charts/hooks/useChartLabelFormats.ts +++ b/src/components/Charts/hooks/useChartLabelFormats.ts @@ -1,7 +1,7 @@ import type {SkFont} from '@shopify/react-native-skia'; import type {ChartDataPoint, UnitPosition, UnitWithFallback} from '@components/Charts/types'; +import {LABEL_ROTATIONS} from '@components/Charts/utils'; import useLocalize from '@hooks/useLocalize'; -import {LABEL_ROTATIONS} from './useChartLabelLayout'; type UseChartLabelFormatsProps = { data: ChartDataPoint[]; diff --git a/src/components/Charts/hooks/useChartLabelLayout.ts b/src/components/Charts/hooks/useChartLabelLayout.ts index cc2c7eb324a9d..a979a746a961e 100644 --- a/src/components/Charts/hooks/useChartLabelLayout.ts +++ b/src/components/Charts/hooks/useChartLabelLayout.ts @@ -1,24 +1,20 @@ import type {SkFont} from '@shopify/react-native-skia'; import {useMemo} from 'react'; import type {ChartDataPoint} from '@components/Charts/types'; -import {measureTextWidth} from '@components/Charts/utils'; - -/** Supported label rotation angles in degrees */ -const LABEL_ROTATIONS = { - HORIZONTAL: 0, - DIAGONAL: 45, - VERTICAL: 90, -} as const; - -const SIN_45 = Math.sin(Math.PI / 4); - -/** Minimum gap between adjacent labels (px) */ -const LABEL_PADDING = 4; - -const ELLIPSIS = '...'; - -/** Minimum visible characters (excluding ellipsis) for truncation to be worthwhile */ -const MIN_TRUNCATED_CHARS = 10; +import { + edgeLabelsFit, + edgeMaxLabelWidth, + effectiveHeight, + effectiveWidth, + ELLIPSIS, + LABEL_PADDING, + LABEL_ROTATIONS, + maxVisibleCount, + measureTextWidth, + MIN_TRUNCATED_CHARS, + SIN_45, + truncateLabel, +} from '@components/Charts/utils'; type LabelLayoutConfig = { data: ChartDataPoint[]; @@ -33,104 +29,6 @@ type LabelLayoutConfig = { allowTightDiagonalPacking?: boolean; }; -/** Truncate `label` so its pixel width fits within `maxWidth`, adding ellipsis. */ -function truncateLabel(label: string, labelWidth: number, maxWidth: number, ellipsisWidth: number): string { - if (labelWidth <= maxWidth) { - return label; - } - const available = maxWidth - ellipsisWidth; - if (available <= 0) { - return ELLIPSIS; - } - const maxChars = Math.max(1, Math.floor(label.length * (available / labelWidth))); - return label.slice(0, maxChars) + ELLIPSIS; -} - -/** Horizontal footprint of a label at a given rotation angle (for inter-tick overlap checks). */ -function effectiveWidth(labelWidth: number, lineHeight: number, rotation: number): number { - if (rotation === LABEL_ROTATIONS.VERTICAL) { - return lineHeight; - } - if (rotation === LABEL_ROTATIONS.DIAGONAL) { - return labelWidth * SIN_45; - } - return labelWidth; -} - -/** Vertical footprint of a label at a given rotation angle. */ -function effectiveHeight(labelWidth: number, lineHeight: number, rotation: number): number { - if (rotation === LABEL_ROTATIONS.VERTICAL) { - return labelWidth; - } - if (rotation === LABEL_ROTATIONS.DIAGONAL) { - return labelWidth * SIN_45 + lineHeight * SIN_45; - } - return lineHeight; -} - -/** How many labels fit side-by-side in `areaWidth` given each takes `itemWidth`. */ -function maxVisibleCount(areaWidth: number, itemWidth: number): number { - return Math.floor(areaWidth / (itemWidth + LABEL_PADDING)); -} - -/** - * How far a label extends beyond its tick position after rotation. - * Accounts for the rotatedLabelCenterCorrection translateX applied during rendering. - */ -function labelOverhang(labelWidth: number, lineHeight: number, rotation: number, rightAligned: boolean): {left: number; right: number} { - if (rotation === LABEL_ROTATIONS.HORIZONTAL) { - return {left: labelWidth / 2, right: labelWidth / 2}; - } - if (rotation === LABEL_ROTATIONS.DIAGONAL) { - const halfLH = lineHeight / 2; - if (rightAligned) { - return { - left: (labelWidth + halfLH) * SIN_45, - right: halfLH * SIN_45, - }; - } - const overhang = (labelWidth / 2 + halfLH) * SIN_45; - return {left: overhang, right: overhang}; - } - return {left: lineHeight / 2, right: lineHeight / 2}; -} - -/** Check if first and last labels fit within the available canvas edge space. */ -function edgeLabelsFit( - firstLabelWidth: number, - lastLabelWidth: number, - lineHeight: number, - rotation: number, - firstTickLeftSpace: number, - lastTickRightSpace: number, - rightAligned: boolean, -): boolean { - const first = labelOverhang(firstLabelWidth, lineHeight, rotation, rightAligned); - const last = labelOverhang(lastLabelWidth, lineHeight, rotation, rightAligned); - return first.left <= firstTickLeftSpace && last.right <= lastTickRightSpace; -} - -/** - * Maximum label width that fits within the available edge space at a given rotation. - * Returns Infinity when the overhang at that edge doesn't depend on label width. - */ -function edgeMaxLabelWidth(edgeSpace: number, lineHeight: number, rotation: number, rightAligned: boolean, edge: 'first' | 'last'): number { - const halfLH = lineHeight / 2; - if (rotation === LABEL_ROTATIONS.HORIZONTAL) { - return 2 * edgeSpace; - } - if (rotation === LABEL_ROTATIONS.DIAGONAL) { - if (rightAligned) { - // Right-aligned: only first label can overflow left, last label's right overhang is constant - return edge === 'first' ? Math.max(0, edgeSpace / SIN_45 - halfLH) : Infinity; - } - // Centered: symmetric overhang on both edges - return Math.max(0, 2 * (edgeSpace / SIN_45 - halfLH)); - } - // Vertical: overhang is lineHeight/2, doesn't depend on label width - return Infinity; -} - function useChartLabelLayout({data, font, tickSpacing, labelAreaWidth, firstTickLeftSpace = Infinity, lastTickRightSpace = Infinity, allowTightDiagonalPacking = false}: LabelLayoutConfig) { return useMemo(() => { if (!font || data.length === 0 || tickSpacing <= 0 || labelAreaWidth <= 0) { @@ -230,5 +128,5 @@ function useChartLabelLayout({data, font, tickSpacing, labelAreaWidth, firstTick }, [font, tickSpacing, labelAreaWidth, firstTickLeftSpace, lastTickRightSpace, data, allowTightDiagonalPacking]); } -export {LABEL_ROTATIONS, useChartLabelLayout}; +export {useChartLabelLayout}; export type {LabelLayoutConfig}; diff --git a/src/components/Charts/utils.ts b/src/components/Charts/utils.ts index ac7bb158d6c33..199abc54c62a2 100644 --- a/src/components/Charts/utils.ts +++ b/src/components/Charts/utils.ts @@ -180,6 +180,118 @@ function processDataIntoSlices(data: ChartDataPoint[], startAngle: number): PieS ).slices; } +/** Supported label rotation angles in degrees */ +const LABEL_ROTATIONS = { + HORIZONTAL: 0, + DIAGONAL: 45, + VERTICAL: 90, +} as const; + +const SIN_45 = Math.sin(Math.PI / 4); + +/** Minimum gap between adjacent labels (px) */ +const LABEL_PADDING = 4; + +const ELLIPSIS = '...'; + +/** Minimum visible characters (excluding ellipsis) for truncation to be worthwhile */ +const MIN_TRUNCATED_CHARS = 10; + +/** Truncate `label` so its pixel width fits within `maxWidth`, adding ellipsis. */ +function truncateLabel(label: string, labelWidth: number, maxWidth: number, ellipsisWidth: number): string { + if (labelWidth <= maxWidth) { + return label; + } + const available = maxWidth - ellipsisWidth; + if (available <= 0) { + return ELLIPSIS; + } + const maxChars = Math.max(1, Math.floor(label.length * (available / labelWidth))); + return label.slice(0, maxChars) + ELLIPSIS; +} + +/** Horizontal footprint of a label at a given rotation angle (for inter-tick overlap checks). */ +function effectiveWidth(labelWidth: number, lineHeight: number, rotation: number): number { + if (rotation === LABEL_ROTATIONS.VERTICAL) { + return lineHeight; + } + if (rotation === LABEL_ROTATIONS.DIAGONAL) { + return labelWidth * SIN_45; + } + return labelWidth; +} + +/** Vertical footprint of a label at a given rotation angle. */ +function effectiveHeight(labelWidth: number, lineHeight: number, rotation: number): number { + if (rotation === LABEL_ROTATIONS.VERTICAL) { + return labelWidth; + } + if (rotation === LABEL_ROTATIONS.DIAGONAL) { + return labelWidth * SIN_45 + lineHeight * SIN_45; + } + return lineHeight; +} + +/** How many labels fit side-by-side in `areaWidth` given each takes `itemWidth`. */ +function maxVisibleCount(areaWidth: number, itemWidth: number): number { + return Math.floor(areaWidth / (itemWidth + LABEL_PADDING)); +} + +/** + * How far a label extends beyond its tick position after rotation. + * Accounts for the rotatedLabelCenterCorrection translateX applied during rendering. + */ +function labelOverhang(labelWidth: number, lineHeight: number, rotation: number, rightAligned: boolean): {left: number; right: number} { + if (rotation === LABEL_ROTATIONS.HORIZONTAL) { + return {left: labelWidth / 2, right: labelWidth / 2}; + } + if (rotation === LABEL_ROTATIONS.DIAGONAL) { + const halfLH = lineHeight / 2; + if (rightAligned) { + return { + left: (labelWidth + halfLH) * SIN_45, + right: halfLH * SIN_45, + }; + } + const overhang = (labelWidth / 2 + halfLH) * SIN_45; + return {left: overhang, right: overhang}; + } + return {left: lineHeight / 2, right: lineHeight / 2}; +} + +/** Check if first and last labels fit within the available canvas edge space. */ +function edgeLabelsFit( + firstLabelWidth: number, + lastLabelWidth: number, + lineHeight: number, + rotation: number, + firstTickLeftSpace: number, + lastTickRightSpace: number, + rightAligned: boolean, +): boolean { + const first = labelOverhang(firstLabelWidth, lineHeight, rotation, rightAligned); + const last = labelOverhang(lastLabelWidth, lineHeight, rotation, rightAligned); + return first.left <= firstTickLeftSpace && last.right <= lastTickRightSpace; +} + +/** + * Maximum label width that fits within the available edge space at a given rotation. + * Returns Infinity when the overhang at that edge doesn't depend on label width. + */ +function edgeMaxLabelWidth(edgeSpace: number, lineHeight: number, rotation: number, rightAligned: boolean, edge: 'first' | 'last'): number { + const halfLH = lineHeight / 2; + if (rotation === LABEL_ROTATIONS.HORIZONTAL) { + return 2 * edgeSpace; + } + if (rotation === LABEL_ROTATIONS.DIAGONAL) { + if (rightAligned) { + return edge === 'first' ? Math.max(0, edgeSpace / SIN_45 - halfLH) : Infinity; + } + return Math.max(0, 2 * (edgeSpace / SIN_45 - halfLH)); + } + return Infinity; +} + export { getChartColor, DEFAULT_CHART_COLOR, @@ -191,4 +303,16 @@ export { isAngleInSlice, findSliceAtPosition, processDataIntoSlices, + LABEL_ROTATIONS, + SIN_45, + LABEL_PADDING, + ELLIPSIS, + MIN_TRUNCATED_CHARS, + truncateLabel, + effectiveWidth, + effectiveHeight, + maxVisibleCount, + labelOverhang, + edgeLabelsFit, + edgeMaxLabelWidth, }; diff --git a/tests/unit/Charts/useChartLabelLayout.test.ts b/tests/unit/Charts/useChartLabelLayout.test.ts index 759d20ba7f23c..b79d3864e23ed 100644 --- a/tests/unit/Charts/useChartLabelLayout.test.ts +++ b/tests/unit/Charts/useChartLabelLayout.test.ts @@ -68,6 +68,14 @@ describe('useChartLabelLayout', () => { expect(result.current.truncatedLabels).toEqual(['AAAAAA', 'BBBBBB']); }); + it('picks 45° when labelAreaWidth is too narrow for 0° despite sufficient tickSpacing', () => { + // "AAA" = 21px. tickSpacing=30: 21+4=25 ≤ 30 ✓ (tick check passes). + // BUT labelAreaWidth=40: maxVisibleCount(40,21) = floor(40/25) = 1 < 3 → 0° fails. + // At 45°: 21*SIN_45 ≈ 14.85, 14.85+4=18.85 ≤ 30 ✓ → 45° selected. + const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAA', 'BBB', 'CCC'), font, tickSpacing: 30, labelAreaWidth: 40})); + expect(result.current.labelRotation).toBe(-45); + }); + it('picks 90° when labels overflow at all rotations', () => { // tickSpacing=20: 0° fails (46>20), 45° fails (29.7+4=33.7>20) const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 20, labelAreaWidth: 400})); From 3cf2ca789682685e2ad1168bec63cf152809eefd Mon Sep 17 00:00:00 2001 From: Mateusz Rajski Date: Mon, 2 Mar 2026 13:42:30 +0100 Subject: [PATCH 05/13] Move constants to constants.ts --- src/components/Charts/constants.ts | 32 ++++++++++++++++++- .../Charts/hooks/useChartLabelFormats.ts | 2 +- .../Charts/hooks/useChartLabelLayout.ts | 16 ++-------- src/components/Charts/utils.ts | 23 +------------ 4 files changed, 35 insertions(+), 38 deletions(-) diff --git a/src/components/Charts/constants.ts b/src/components/Charts/constants.ts index 13a780a35c217..df9c098afb7f8 100644 --- a/src/components/Charts/constants.ts +++ b/src/components/Charts/constants.ts @@ -19,4 +19,34 @@ const Y_AXIS_LINE_WIDTH = 1; /** Starting angle for pie chart (0 = 3 o'clock, -90 = 12 o'clock) */ const PIE_CHART_START_ANGLE = -90; -export {Y_AXIS_TICK_COUNT, AXIS_LABEL_GAP, CHART_PADDING, CHART_CONTENT_MIN_HEIGHT, X_AXIS_LINE_WIDTH, Y_AXIS_LINE_WIDTH, PIE_CHART_START_ANGLE}; +/** Supported label rotation angles in degrees */ +const LABEL_ROTATIONS = { + HORIZONTAL: 0, + DIAGONAL: 45, + VERTICAL: 90, +} as const; + +const SIN_45 = Math.sin(Math.PI / 4); + +/** Minimum gap between adjacent labels (px) */ +const LABEL_PADDING = 4; + +const ELLIPSIS = '...'; + +/** Minimum visible characters (excluding ellipsis) for truncation to be worthwhile */ +const MIN_TRUNCATED_CHARS = 10; + +export { + Y_AXIS_TICK_COUNT, + AXIS_LABEL_GAP, + CHART_PADDING, + CHART_CONTENT_MIN_HEIGHT, + X_AXIS_LINE_WIDTH, + Y_AXIS_LINE_WIDTH, + PIE_CHART_START_ANGLE, + LABEL_ROTATIONS, + SIN_45, + LABEL_PADDING, + ELLIPSIS, + MIN_TRUNCATED_CHARS, +}; diff --git a/src/components/Charts/hooks/useChartLabelFormats.ts b/src/components/Charts/hooks/useChartLabelFormats.ts index 4948d90f0eff4..9b983be1c2726 100644 --- a/src/components/Charts/hooks/useChartLabelFormats.ts +++ b/src/components/Charts/hooks/useChartLabelFormats.ts @@ -1,6 +1,6 @@ import type {SkFont} from '@shopify/react-native-skia'; +import {LABEL_ROTATIONS} from '@components/Charts/constants'; import type {ChartDataPoint, UnitPosition, UnitWithFallback} from '@components/Charts/types'; -import {LABEL_ROTATIONS} from '@components/Charts/utils'; import useLocalize from '@hooks/useLocalize'; type UseChartLabelFormatsProps = { diff --git a/src/components/Charts/hooks/useChartLabelLayout.ts b/src/components/Charts/hooks/useChartLabelLayout.ts index a979a746a961e..30fe45c36cef4 100644 --- a/src/components/Charts/hooks/useChartLabelLayout.ts +++ b/src/components/Charts/hooks/useChartLabelLayout.ts @@ -1,20 +1,8 @@ import type {SkFont} from '@shopify/react-native-skia'; import {useMemo} from 'react'; +import {ELLIPSIS, LABEL_PADDING, LABEL_ROTATIONS, MIN_TRUNCATED_CHARS, SIN_45} from '@components/Charts/constants'; import type {ChartDataPoint} from '@components/Charts/types'; -import { - edgeLabelsFit, - edgeMaxLabelWidth, - effectiveHeight, - effectiveWidth, - ELLIPSIS, - LABEL_PADDING, - LABEL_ROTATIONS, - maxVisibleCount, - measureTextWidth, - MIN_TRUNCATED_CHARS, - SIN_45, - truncateLabel, -} from '@components/Charts/utils'; +import {edgeLabelsFit, edgeMaxLabelWidth, effectiveHeight, effectiveWidth, maxVisibleCount, measureTextWidth, truncateLabel} from '@components/Charts/utils'; type LabelLayoutConfig = { data: ChartDataPoint[]; diff --git a/src/components/Charts/utils.ts b/src/components/Charts/utils.ts index 199abc54c62a2..cb1fa6097c0a1 100644 --- a/src/components/Charts/utils.ts +++ b/src/components/Charts/utils.ts @@ -1,5 +1,6 @@ import type {SkFont} from '@shopify/react-native-skia'; import colors from '@styles/theme/colors'; +import {ELLIPSIS, LABEL_PADDING, LABEL_ROTATIONS, SIN_45} from './constants'; import type {ChartDataPoint, PieSlice} from './types'; /** @@ -180,23 +181,6 @@ function processDataIntoSlices(data: ChartDataPoint[], startAngle: number): PieS ).slices; } -/** Supported label rotation angles in degrees */ -const LABEL_ROTATIONS = { - HORIZONTAL: 0, - DIAGONAL: 45, - VERTICAL: 90, -} as const; - -const SIN_45 = Math.sin(Math.PI / 4); - -/** Minimum gap between adjacent labels (px) */ -const LABEL_PADDING = 4; - -const ELLIPSIS = '...'; - -/** Minimum visible characters (excluding ellipsis) for truncation to be worthwhile */ -const MIN_TRUNCATED_CHARS = 10; - /** Truncate `label` so its pixel width fits within `maxWidth`, adding ellipsis. */ function truncateLabel(label: string, labelWidth: number, maxWidth: number, ellipsisWidth: number): string { if (labelWidth <= maxWidth) { @@ -303,11 +287,6 @@ export { isAngleInSlice, findSliceAtPosition, processDataIntoSlices, - LABEL_ROTATIONS, - SIN_45, - LABEL_PADDING, - ELLIPSIS, - MIN_TRUNCATED_CHARS, truncateLabel, effectiveWidth, effectiveHeight, From 7c08f2ec091fd7fb9e87d8951b3493a99a5d47cb Mon Sep 17 00:00:00 2001 From: Mateusz Rajski Date: Mon, 2 Mar 2026 13:55:40 +0100 Subject: [PATCH 06/13] Remove manual memoization --- .../Charts/BarChart/BarChartContent.tsx | 196 ++++++++---------- .../Charts/LineChart/LineChartContent.tsx | 123 +++++------ .../Charts/hooks/useChartLabelLayout.ts | 179 ++++++++-------- 3 files changed, 225 insertions(+), 273 deletions(-) diff --git a/src/components/Charts/BarChart/BarChartContent.tsx b/src/components/Charts/BarChart/BarChartContent.tsx index b3625c06c9a21..b8dd49e2f964c 100644 --- a/src/components/Charts/BarChart/BarChartContent.tsx +++ b/src/components/Charts/BarChart/BarChartContent.tsx @@ -1,5 +1,5 @@ import {useFont} from '@shopify/react-native-skia'; -import React, {useCallback, useMemo, useState} from 'react'; +import React, {useState} from 'react'; import type {LayoutChangeEvent} from 'react-native'; import {View} from 'react-native'; import {useSharedValue} from 'react-native-reanimated'; @@ -47,41 +47,34 @@ function BarChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUni const [boundsRight, setBoundsRight] = useState(0); const defaultBarColor = DEFAULT_CHART_COLOR; - // prepare data for display - const chartData = useMemo(() => { - return data.map((point, index) => ({ - x: index, - y: point.total, - })); - }, [data]); + const chartData = data.map((point, index) => ({ + x: index, + y: point.total, + })); const yAxisDomain = useDynamicYDomain(data); - // Handle bar press callback - const handleBarPress = useCallback( - (index: number) => { - if (index < 0 || index >= data.length) { - return; - } - const dataPoint = data.at(index); - if (dataPoint && onBarPress) { - onBarPress(dataPoint, index); - } - }, - [data, onBarPress], - ); + const handleBarPress = (index: number) => { + if (index < 0 || index >= data.length) { + return; + } + const dataPoint = data.at(index); + if (dataPoint && onBarPress) { + onBarPress(dataPoint, index); + } + }; - const handleLayout = useCallback((event: LayoutChangeEvent) => { + const handleLayout = (event: LayoutChangeEvent) => { setChartWidth(event.nativeEvent.layout.width); - }, []); + }; - const domainPadding = useMemo(() => { + const domainPadding = (() => { if (chartWidth === 0) { return BASE_DOMAIN_PADDING; } const horizontalPadding = calculateMinDomainPadding(chartWidth, data.length, BAR_INNER_PADDING); return {...BASE_DOMAIN_PADDING, left: horizontalPadding, right: horizontalPadding}; - }, [chartWidth, data.length]); + })(); const totalDomainPadding = domainPadding.left + domainPadding.right; const paddingScale = barAreaWidth > 0 ? barAreaWidth / (barAreaWidth + totalDomainPadding) : 0; @@ -102,52 +95,40 @@ function BarChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUni unitPosition: yAxisUnitPosition, }); - // Store bar geometry for hit-testing (only constants, no arrays) const barWidth = useSharedValue(0); const chartBottom = useSharedValue(0); const yZero = useSharedValue(0); - const handleChartBoundsChange = useCallback( - (bounds: ChartBounds) => { - const domainWidth = bounds.right - bounds.left; - const calculatedBarWidth = ((1 - BAR_INNER_PADDING) * domainWidth) / data.length; - barWidth.set(calculatedBarWidth); - chartBottom.set(bounds.bottom); - yZero.set(0); - setBarAreaWidth(domainWidth); - setBoundsLeft(bounds.left); - setBoundsRight(bounds.right); - }, - [data.length, barWidth, chartBottom, yZero], - ); - - const handleScaleChange = useCallback( - (_xScale: Scale, yScale: Scale) => { - yZero.set(yScale(0)); - }, - [yZero], - ); - - const checkIsOverBar = useCallback( - (args: HitTestArgs) => { - 'worklet'; - - const currentBarWidth = barWidth.get(); - const currentYZero = yZero.get(); - if (currentBarWidth === 0) { - return false; - } - const barLeft = args.targetX - currentBarWidth / 2; - const barRight = args.targetX + currentBarWidth / 2; - // For positive bars: targetY < yZero, bar goes from targetY (top) to yZero (bottom) - // For negative bars: targetY > yZero, bar goes from yZero (top) to targetY (bottom) - const barTop = Math.min(args.targetY, currentYZero); - const barBottom = Math.max(args.targetY, currentYZero); + const handleChartBoundsChange = (bounds: ChartBounds) => { + const domainWidth = bounds.right - bounds.left; + const calculatedBarWidth = ((1 - BAR_INNER_PADDING) * domainWidth) / data.length; + barWidth.set(calculatedBarWidth); + chartBottom.set(bounds.bottom); + yZero.set(0); + setBarAreaWidth(domainWidth); + setBoundsLeft(bounds.left); + setBoundsRight(bounds.right); + }; + + const handleScaleChange = (_xScale: Scale, yScale: Scale) => { + yZero.set(yScale(0)); + }; + + const checkIsOverBar = (args: HitTestArgs) => { + 'worklet'; + + const currentBarWidth = barWidth.get(); + const currentYZero = yZero.get(); + if (currentBarWidth === 0) { + return false; + } + const barLeft = args.targetX - currentBarWidth / 2; + const barRight = args.targetX + currentBarWidth / 2; + const barTop = Math.min(args.targetY, currentYZero); + const barBottom = Math.max(args.targetY, currentYZero); - return args.cursorX >= barLeft && args.cursorX <= barRight && args.cursorY >= barTop && args.cursorY <= barBottom; - }, - [barWidth, yZero], - ); + return args.cursorX >= barLeft && args.cursorX <= barRight && args.cursorY >= barTop && args.cursorY <= barBottom; + }; const {actionsRef, customGestures, activeDataIndex, isTooltipActive, initialTooltipPosition} = useChartInteractions({ handlePress: handleBarPress, @@ -158,56 +139,45 @@ function BarChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUni const tooltipData = useTooltipData(activeDataIndex, data, formatValue); - const renderBar = useCallback( - (point: PointsArray[number], chartBounds: ChartBounds, barCount: number) => { - const dataIndex = point.xValue as number; - const dataPoint = data.at(dataIndex); - const barColor = useSingleColor ? defaultBarColor : getChartColor(dataIndex); + const renderBar = (point: PointsArray[number], chartBounds: ChartBounds, barCount: number) => { + const dataIndex = point.xValue as number; + const dataPoint = data.at(dataIndex); + const barColor = useSingleColor ? defaultBarColor : getChartColor(dataIndex); - return ( - - ); - }, - [data, useSingleColor, defaultBarColor], - ); + return ( + + ); + }; - const renderCustomXLabels = useCallback( - (args: CartesianChartRenderArg<{x: number; y: number}, 'y'>) => { - if (!font) { - return null; - } - return ( - - ); - }, - [font, truncatedLabels, labelRotation, labelSkipInterval, theme.textSupporting], - ); + const renderCustomXLabels = (args: CartesianChartRenderArg<{x: number; y: number}, 'y'>) => { + if (!font) { + return null; + } + return ( + + ); + }; - // When labels are rotated 90°, add measured label height to container - // This keeps bar area at ~250px while giving labels their needed vertical space - const dynamicChartStyle = useMemo( - () => ({ - height: CHART_CONTENT_MIN_HEIGHT + (xAxisLabelHeight ?? 0), - }), - [xAxisLabelHeight], - ); + const dynamicChartStyle = { + height: CHART_CONTENT_MIN_HEIGHT + (xAxisLabelHeight ?? 0), + }; if (isLoading || !font) { return ( diff --git a/src/components/Charts/LineChart/LineChartContent.tsx b/src/components/Charts/LineChart/LineChartContent.tsx index 7a7959a754606..b82155493ce3c 100644 --- a/src/components/Charts/LineChart/LineChartContent.tsx +++ b/src/components/Charts/LineChart/LineChartContent.tsx @@ -1,5 +1,5 @@ import {useFont} from '@shopify/react-native-skia'; -import React, {useCallback, useMemo, useState} from 'react'; +import React, {useState} from 'react'; import type {LayoutChangeEvent} from 'react-native'; import {View} from 'react-native'; import type {CartesianChartRenderArg, ChartBounds} from 'victory-native'; @@ -49,38 +49,32 @@ function LineChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUn const [boundsRight, setBoundsRight] = useState(0); const yAxisDomain = useDynamicYDomain(data); - const chartData = useMemo(() => { - return data.map((point, index) => ({ - x: index, - y: point.total, - })); - }, [data]); - - const handlePointPress = useCallback( - (index: number) => { - if (index < 0 || index >= data.length) { - return; - } - const dataPoint = data.at(index); - if (dataPoint && onPointPress) { - onPointPress(dataPoint, index); - } - }, - [data, onPointPress], - ); + const chartData = data.map((point, index) => ({ + x: index, + y: point.total, + })); + + const handlePointPress = (index: number) => { + if (index < 0 || index >= data.length) { + return; + } + const dataPoint = data.at(index); + if (dataPoint && onPointPress) { + onPointPress(dataPoint, index); + } + }; - const handleLayout = useCallback((event: LayoutChangeEvent) => { + const handleLayout = (event: LayoutChangeEvent) => { setChartWidth(event.nativeEvent.layout.width); - }, []); + }; - const handleChartBoundsChange = useCallback((bounds: ChartBounds) => { + const handleChartBoundsChange = (bounds: ChartBounds) => { setPlotAreaWidth(bounds.right - bounds.left); setBoundsLeft(bounds.left); setBoundsRight(bounds.right); - }, []); + }; - // Optimize by reducing wasted space when edge labels are shorter than tick spacing - const domainPadding = useMemo(() => { + const domainPadding = (() => { if (chartWidth === 0 || data.length === 0) { return BASE_DOMAIN_PADDING; } @@ -94,7 +88,6 @@ function LineChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUn const firstLabelWidth = measureTextWidth(data.at(0)?.label ?? '', font); const lastLabelWidth = measureTextWidth(data.at(-1)?.label ?? '', font); - // At 0° rotation, centered labels extend by half their width const firstLabelNeeds = firstLabelWidth / 2; const lastLabelNeeds = lastLabelWidth / 2; @@ -102,20 +95,16 @@ function LineChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUn const wastedRight = geometricPadding - lastLabelNeeds; const reclaimablePadding = Math.min(wastedLeft, wastedRight); - // Only reduce if both sides have excess space (labels short enough for 0°) if (reclaimablePadding <= 0) { return {...BASE_DOMAIN_PADDING, left: geometricPadding, right: geometricPadding}; } const horizontalPadding = Math.max(geometricPadding - reclaimablePadding, MIN_SAFE_PADDING); return {...BASE_DOMAIN_PADDING, left: horizontalPadding, right: horizontalPadding}; - }, [chartWidth, data, font]); + })(); const tickSpacing = plotAreaWidth > 0 && data.length > 0 ? plotAreaWidth / data.length : 0; - // Victory's domainPadding extends the data domain, not the pixel range directly. - // The actual pixel offset for the first tick is smaller than domainPadding when - // the padding is large relative to the plot area. const totalDomainPadding = domainPadding.left + domainPadding.right; const paddingScale = plotAreaWidth > 0 ? plotAreaWidth / (plotAreaWidth + totalDomainPadding) : 0; @@ -136,13 +125,13 @@ function LineChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUn unitPosition: yAxisUnitPosition, }); - const checkIsOverDot = useCallback((args: HitTestArgs) => { + const checkIsOverDot = (args: HitTestArgs) => { 'worklet'; const dx = args.cursorX - args.targetX; const dy = args.cursorY - args.targetY; return Math.sqrt(dx * dx + dy * dy) <= DOT_RADIUS + DOT_HOVER_EXTRA_RADIUS; - }, []); + }; const {actionsRef, customGestures, activeDataIndex, isTooltipActive, initialTooltipPosition} = useChartInteractions({ handlePress: handlePointPress, @@ -151,44 +140,38 @@ function LineChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUn const tooltipData = useTooltipData(activeDataIndex, data, formatValue); - const renderOutsideComponents = useCallback( - (args: CartesianChartRenderArg<{x: number; y: number}, 'y'>) => { - return ( - <> - - ) => { + return ( + <> + + + {!!font && ( + - {!!font && ( - - )} - - ); - }, - [font, truncatedLabels, labelRotation, labelSkipInterval, theme.textSupporting, theme.border], - ); + )} + + ); + }; - const dynamicChartStyle = useMemo( - () => ({ - height: CHART_CONTENT_MIN_HEIGHT + (xAxisLabelHeight ?? 0), - }), - [xAxisLabelHeight], - ); + const dynamicChartStyle = { + height: CHART_CONTENT_MIN_HEIGHT + (xAxisLabelHeight ?? 0), + }; if (isLoading || !font) { return ( diff --git a/src/components/Charts/hooks/useChartLabelLayout.ts b/src/components/Charts/hooks/useChartLabelLayout.ts index 30fe45c36cef4..ae5fb114b9c0b 100644 --- a/src/components/Charts/hooks/useChartLabelLayout.ts +++ b/src/components/Charts/hooks/useChartLabelLayout.ts @@ -1,119 +1,118 @@ import type {SkFont} from '@shopify/react-native-skia'; -import {useMemo} from 'react'; import {ELLIPSIS, LABEL_PADDING, LABEL_ROTATIONS, MIN_TRUNCATED_CHARS, SIN_45} from '@components/Charts/constants'; import type {ChartDataPoint} from '@components/Charts/types'; import {edgeLabelsFit, edgeMaxLabelWidth, effectiveHeight, effectiveWidth, maxVisibleCount, measureTextWidth, truncateLabel} from '@components/Charts/utils'; type LabelLayoutConfig = { + /** Chart data points whose labels will be laid out. */ data: ChartDataPoint[]; + + /** Skia font used for measuring label text widths. */ font: SkFont | null; + + /** Distance in pixels between adjacent tick marks. */ tickSpacing: number; + + /** Total width in pixels of the plot area where labels are rendered. */ labelAreaWidth: number; + /** Pixels from first tick to left edge of canvas. Defaults to Infinity (no constraint). */ firstTickLeftSpace?: number; + /** Pixels from last tick to right edge of canvas. Defaults to Infinity (no constraint). */ lastTickRightSpace?: number; + /** When true, allows tighter label packing at 45° by accounting for vertical offset between right-aligned labels. */ allowTightDiagonalPacking?: boolean; }; function useChartLabelLayout({data, font, tickSpacing, labelAreaWidth, firstTickLeftSpace = Infinity, lastTickRightSpace = Infinity, allowTightDiagonalPacking = false}: LabelLayoutConfig) { - return useMemo(() => { - if (!font || data.length === 0 || tickSpacing <= 0 || labelAreaWidth <= 0) { - return {labelRotation: 0, labelSkipInterval: 1, truncatedLabels: []}; - } - - const fontMetrics = font.getMetrics(); - const lineHeight = Math.abs(fontMetrics.descent) + Math.abs(fontMetrics.ascent); - const ellipsisWidth = measureTextWidth(ELLIPSIS, font); - const labelWidths = data.map((point) => measureTextWidth(point.label, font)); - const maxLabelWidth = Math.max(...labelWidths); - const firstLabelWidth = labelWidths.at(0) ?? 0; - const lastLabelWidth = labelWidths.at(-1) ?? 0; - const rightAligned = allowTightDiagonalPacking; - - // Maximum width of labels after truncation to MIN_TRUNCATED_CHARS characters. - const minTruncatedWidth = Math.max( - ...data.map((point, index) => { - if (point.label.length <= MIN_TRUNCATED_CHARS) { - return labelWidths.at(index) ?? 0; - } - return measureTextWidth(point.label.slice(0, MIN_TRUNCATED_CHARS) + ELLIPSIS, font); - }), - ); - - // Min truncated widths for edge labels specifically - const firstLabel = data.at(0)?.label ?? ''; - const lastLabel = data.at(-1)?.label ?? ''; - const firstMinTrunc = firstLabel.length <= MIN_TRUNCATED_CHARS ? firstLabelWidth : measureTextWidth(firstLabel.slice(0, MIN_TRUNCATED_CHARS) + ELLIPSIS, font); - const lastMinTrunc = lastLabel.length <= MIN_TRUNCATED_CHARS ? lastLabelWidth : measureTextWidth(lastLabel.slice(0, MIN_TRUNCATED_CHARS) + ELLIPSIS, font); - - // --- 1. Pick rotation (prefer 0° → 45° → 90°) --- - let rotation: number = LABEL_ROTATIONS.VERTICAL; - - // 0°: labels fit horizontally without truncation AND edge labels fit - const hWidth = effectiveWidth(maxLabelWidth, lineHeight, LABEL_ROTATIONS.HORIZONTAL); - const hFitsInTicks = hWidth + LABEL_PADDING <= tickSpacing && maxVisibleCount(labelAreaWidth, hWidth) >= data.length; - const hEdgeFits = edgeLabelsFit(firstLabelWidth, lastLabelWidth, lineHeight, LABEL_ROTATIONS.HORIZONTAL, firstTickLeftSpace, lastTickRightSpace, false); - - if (hFitsInTicks && hEdgeFits) { - rotation = LABEL_ROTATIONS.HORIZONTAL; - } else { - // 45°: viable if MIN_TRUNCATED_CHARS + ellipsis fits between ticks - const diagonalOverlap = allowTightDiagonalPacking ? lineHeight * SIN_45 : 0; - const minDiagWidth = minTruncatedWidth * SIN_45 - diagonalOverlap; - const dFitsInTicks = minDiagWidth + LABEL_PADDING <= tickSpacing; - - // Edge check: can first/last labels show at least MIN_TRUNCATED_CHARS at 45°? - const firstEdgeMax = edgeMaxLabelWidth(firstTickLeftSpace, lineHeight, LABEL_ROTATIONS.DIAGONAL, rightAligned, 'first'); - const lastEdgeMax = edgeMaxLabelWidth(lastTickRightSpace, lineHeight, LABEL_ROTATIONS.DIAGONAL, rightAligned, 'last'); - const dEdgeFits = firstEdgeMax >= firstMinTrunc && lastEdgeMax >= lastMinTrunc; - - if (dFitsInTicks && dEdgeFits) { - rotation = LABEL_ROTATIONS.DIAGONAL; + if (!font || data.length === 0 || tickSpacing <= 0 || labelAreaWidth <= 0) { + return {labelRotation: 0, labelSkipInterval: 1, truncatedLabels: [] as string[]}; + } + + const fontMetrics = font.getMetrics(); + const lineHeight = Math.abs(fontMetrics.descent) + Math.abs(fontMetrics.ascent); + const ellipsisWidth = measureTextWidth(ELLIPSIS, font); + const labelWidths = data.map((point) => measureTextWidth(point.label, font)); + const maxLabelWidth = Math.max(...labelWidths); + const firstLabelWidth = labelWidths.at(0) ?? 0; + const lastLabelWidth = labelWidths.at(-1) ?? 0; + const minTruncatedWidth = Math.max( + ...data.map((point, index) => { + if (point.label.length <= MIN_TRUNCATED_CHARS) { + return labelWidths.at(index) ?? 0; } + return measureTextWidth(point.label.slice(0, MIN_TRUNCATED_CHARS) + ELLIPSIS, font); + }), + ); + + const firstLabel = data.at(0)?.label ?? ''; + const lastLabel = data.at(-1)?.label ?? ''; + const firstMinTrunc = firstLabel.length <= MIN_TRUNCATED_CHARS ? firstLabelWidth : measureTextWidth(firstLabel.slice(0, MIN_TRUNCATED_CHARS) + ELLIPSIS, font); + const lastMinTrunc = lastLabel.length <= MIN_TRUNCATED_CHARS ? lastLabelWidth : measureTextWidth(lastLabel.slice(0, MIN_TRUNCATED_CHARS) + ELLIPSIS, font); + + // --- 1. Pick rotation (prefer 0° → 45° → 90°) --- + let rotation: number = LABEL_ROTATIONS.VERTICAL; + + const hWidth = effectiveWidth(maxLabelWidth, lineHeight, LABEL_ROTATIONS.HORIZONTAL); + const hFitsInTicks = hWidth + LABEL_PADDING <= tickSpacing && maxVisibleCount(labelAreaWidth, hWidth) >= data.length; + const hEdgeFits = edgeLabelsFit(firstLabelWidth, lastLabelWidth, lineHeight, LABEL_ROTATIONS.HORIZONTAL, firstTickLeftSpace, lastTickRightSpace, false); + + if (hFitsInTicks && hEdgeFits) { + rotation = LABEL_ROTATIONS.HORIZONTAL; + } else { + const diagonalOverlap = allowTightDiagonalPacking ? lineHeight * SIN_45 : 0; + const minDiagWidth = minTruncatedWidth * SIN_45 - diagonalOverlap; + const dFitsInTicks = minDiagWidth + LABEL_PADDING <= tickSpacing; + + const firstEdgeMax = edgeMaxLabelWidth(firstTickLeftSpace, lineHeight, LABEL_ROTATIONS.DIAGONAL, allowTightDiagonalPacking, 'first'); + const lastEdgeMax = edgeMaxLabelWidth(lastTickRightSpace, lineHeight, LABEL_ROTATIONS.DIAGONAL, allowTightDiagonalPacking, 'last'); + const dEdgeFits = firstEdgeMax >= firstMinTrunc && lastEdgeMax >= lastMinTrunc; + + if (dFitsInTicks && dEdgeFits) { + rotation = LABEL_ROTATIONS.DIAGONAL; } + } - // --- 2. Truncate labels --- - const truncDiagonalOverlap = allowTightDiagonalPacking ? lineHeight : 0; - const tickMaxWidth = rotation === LABEL_ROTATIONS.DIAGONAL ? (tickSpacing - LABEL_PADDING) / SIN_45 + truncDiagonalOverlap : Infinity; - - const finalLabels = data.map((point, index) => { - let maxWidth = tickMaxWidth; + // --- 2. Truncate labels --- + const truncDiagonalOverlap = allowTightDiagonalPacking ? lineHeight : 0; + const tickMaxWidth = rotation === LABEL_ROTATIONS.DIAGONAL ? (tickSpacing - LABEL_PADDING) / SIN_45 + truncDiagonalOverlap : Infinity; - // Apply edge constraints for first/last labels - if (index === 0) { - const edgeMax = edgeMaxLabelWidth(firstTickLeftSpace, lineHeight, rotation, rightAligned, 'first'); - maxWidth = Math.min(maxWidth, edgeMax); - } - if (index === data.length - 1) { - const edgeMax = edgeMaxLabelWidth(lastTickRightSpace, lineHeight, rotation, rightAligned, 'last'); - maxWidth = Math.min(maxWidth, edgeMax); - } + const finalLabels = data.map((point, index) => { + let maxWidth = tickMaxWidth; - return truncateLabel(point.label, labelWidths.at(index) ?? 0, maxWidth, ellipsisWidth); - }); - - // --- 3. Compute skip interval (only at 90°) --- - const finalWidths = finalLabels.map((label) => measureTextWidth(label, font)); - const finalMaxWidth = Math.max(...finalWidths); - let skipInterval = 1; - if (rotation === LABEL_ROTATIONS.VERTICAL) { - const verticalWidth = effectiveWidth(finalMaxWidth, lineHeight, rotation); - const visibleCount = maxVisibleCount(labelAreaWidth, verticalWidth); - skipInterval = visibleCount >= data.length ? 1 : Math.ceil(data.length / Math.max(1, visibleCount)); + if (index === 0) { + const edgeMax = edgeMaxLabelWidth(firstTickLeftSpace, lineHeight, rotation, allowTightDiagonalPacking, 'first'); + maxWidth = Math.min(maxWidth, edgeMax); + } + if (index === data.length - 1) { + const edgeMax = edgeMaxLabelWidth(lastTickRightSpace, lineHeight, rotation, allowTightDiagonalPacking, 'last'); + maxWidth = Math.min(maxWidth, edgeMax); } - // --- 4. Compute vertical space needed for x-axis labels --- - const xAxisLabelHeight = effectiveHeight(finalMaxWidth, lineHeight, rotation); - - return { - labelRotation: -rotation, - labelSkipInterval: skipInterval, - truncatedLabels: finalLabels, - xAxisLabelHeight, - }; - }, [font, tickSpacing, labelAreaWidth, firstTickLeftSpace, lastTickRightSpace, data, allowTightDiagonalPacking]); + return truncateLabel(point.label, labelWidths.at(index) ?? 0, maxWidth, ellipsisWidth); + }); + + // --- 3. Compute skip interval (only at 90°) --- + const finalWidths = finalLabels.map((label) => measureTextWidth(label, font)); + const finalMaxWidth = Math.max(...finalWidths); + let skipInterval = 1; + if (rotation === LABEL_ROTATIONS.VERTICAL) { + const verticalWidth = effectiveWidth(finalMaxWidth, lineHeight, rotation); + const visibleCount = maxVisibleCount(labelAreaWidth, verticalWidth); + skipInterval = visibleCount >= data.length ? 1 : Math.ceil(data.length / Math.max(1, visibleCount)); + } + + // --- 4. Compute vertical space needed for x-axis labels --- + const xAxisLabelHeight = effectiveHeight(finalMaxWidth, lineHeight, rotation); + + return { + labelRotation: -rotation, + labelSkipInterval: skipInterval, + truncatedLabels: finalLabels, + xAxisLabelHeight, + }; } export {useChartLabelLayout}; From 550715f32caf141b2cd266e02b3a55aad0644e8e Mon Sep 17 00:00:00 2001 From: Mateusz Rajski Date: Mon, 2 Mar 2026 14:12:59 +0100 Subject: [PATCH 07/13] Add isolated utils tests --- .../Charts/components/ChartXAxisLabels.tsx | 14 ++ tests/unit/Charts/useChartLabelLayout.test.ts | 2 +- tests/unit/components/Charts/utils.test.ts | 155 +++++++++++++++++- 3 files changed, 169 insertions(+), 2 deletions(-) diff --git a/src/components/Charts/components/ChartXAxisLabels.tsx b/src/components/Charts/components/ChartXAxisLabels.tsx index f87c0fa28bcbf..d5ecaab6a0da7 100644 --- a/src/components/Charts/components/ChartXAxisLabels.tsx +++ b/src/components/Charts/components/ChartXAxisLabels.tsx @@ -5,13 +5,27 @@ import {AXIS_LABEL_GAP} from '@components/Charts/constants'; import {measureTextWidth, rotatedLabelCenterCorrection, rotatedLabelYOffset} from '@components/Charts/utils'; type ChartXAxisLabelsProps = { + /** Processed label strings (already truncated by the layout hook). */ labels: string[]; + + /** Label rotation in degrees (negative value, e.g. -45). */ labelRotation: number; + + /** Show every Nth label (1 = all, 2 = every other, etc.). */ labelSkipInterval: number; + + /** Skia font used for measuring and rendering labels. */ font: SkFont; + + /** Fill color for the label text. */ labelColor: string; + + /** Maps a data-point index to its x-pixel position on the chart. */ xScale: (value: number) => number; + + /** Y-pixel coordinate of the bottom edge of the chart plot area. */ chartBoundsBottom: number; + /** When true, rotated labels are centered on the tick. When false, they are right-aligned (end of text at tick). */ centerRotatedLabels?: boolean; }; diff --git a/tests/unit/Charts/useChartLabelLayout.test.ts b/tests/unit/Charts/useChartLabelLayout.test.ts index b79d3864e23ed..56c9ed25f6b17 100644 --- a/tests/unit/Charts/useChartLabelLayout.test.ts +++ b/tests/unit/Charts/useChartLabelLayout.test.ts @@ -1,5 +1,6 @@ import type {SkFont} from '@shopify/react-native-skia'; import {renderHook} from '@testing-library/react-native'; +import {SIN_45} from '@components/Charts/constants'; import {useChartLabelLayout} from '@components/Charts/hooks/useChartLabelLayout'; import type {ChartDataPoint} from '@components/Charts/types'; @@ -22,7 +23,6 @@ function makeData(...labels: string[]): ChartDataPoint[] { return labels.map((label, i) => ({label, total: (i + 1) * 100})); } -const SIN_45 = Math.sin(Math.PI / 4); const LINE_HEIGHT = 16; // |ascent(12)| + |descent(4)| describe('useChartLabelLayout', () => { diff --git a/tests/unit/components/Charts/utils.test.ts b/tests/unit/components/Charts/utils.test.ts index f7f613ac5df7b..df440572d31dc 100644 --- a/tests/unit/components/Charts/utils.test.ts +++ b/tests/unit/components/Charts/utils.test.ts @@ -1,5 +1,158 @@ +import {SIN_45} from '@components/Charts/constants'; import type {ChartDataPoint, PieSlice} from '@components/Charts/types'; -import {findSliceAtPosition, isAngleInSlice, normalizeAngle, processDataIntoSlices} from '@components/Charts/utils'; +import { + edgeLabelsFit, + edgeMaxLabelWidth, + effectiveHeight, + effectiveWidth, + findSliceAtPosition, + isAngleInSlice, + labelOverhang, + maxVisibleCount, + normalizeAngle, + processDataIntoSlices, + truncateLabel, +} from '@components/Charts/utils'; + +const LINE_HEIGHT = 16; + +describe('truncateLabel', () => { + const ellipsisWidth = 21; + + it('returns label unchanged when it fits within maxWidth', () => { + expect(truncateLabel('Hello', 35, 50, ellipsisWidth)).toBe('Hello'); + }); + + it('truncates and adds ellipsis when label exceeds maxWidth', () => { + // available = 40 - 21 = 19, maxChars = floor(9 * 19/63) = 2 + expect(truncateLabel('LongLabel', 63, 40, ellipsisWidth)).toBe('Lo...'); + }); + + it('returns label unchanged when labelWidth exactly equals maxWidth', () => { + expect(truncateLabel('Hello', 50, 50, ellipsisWidth)).toBe('Hello'); + }); + + it('returns only ellipsis when available space is zero or negative', () => { + expect(truncateLabel('Text', 28, 20, ellipsisWidth)).toBe('...'); + }); + + it('keeps at least 1 character before ellipsis', () => { + // available = 25 - 21 = 4, maxChars = max(1, floor(6 * 4/42)) = 1 + expect(truncateLabel('ABCDEF', 42, 25, ellipsisWidth)).toBe('A...'); + }); +}); + +describe('effectiveWidth', () => { + it('returns labelWidth at 0°', () => { + expect(effectiveWidth(100, LINE_HEIGHT, 0)).toBe(100); + }); + + it('returns labelWidth * sin(45°) at 45°', () => { + expect(effectiveWidth(100, LINE_HEIGHT, 45)).toBeCloseTo(100 * SIN_45); + }); + + it('returns lineHeight at 90°', () => { + expect(effectiveWidth(100, LINE_HEIGHT, 90)).toBe(LINE_HEIGHT); + }); +}); + +describe('effectiveHeight', () => { + it('returns lineHeight at 0°', () => { + expect(effectiveHeight(100, LINE_HEIGHT, 0)).toBe(LINE_HEIGHT); + }); + + it('returns (labelWidth + lineHeight) * sin(45°) at 45°', () => { + expect(effectiveHeight(100, LINE_HEIGHT, 45)).toBeCloseTo((100 + LINE_HEIGHT) * SIN_45); + }); + + it('returns labelWidth at 90°', () => { + expect(effectiveHeight(100, LINE_HEIGHT, 90)).toBe(100); + }); +}); + +describe('maxVisibleCount', () => { + it('returns correct count for simple case', () => { + // Each label takes 20px + 4px padding = 24px. 100 / 24 = 4.16 → floor = 4 + expect(maxVisibleCount(100, 20)).toBe(4); + }); + + it('returns 0 when area is smaller than one label', () => { + expect(maxVisibleCount(10, 20)).toBe(0); + }); + + it('returns 0 when area is zero', () => { + expect(maxVisibleCount(0, 20)).toBe(0); + }); +}); + +describe('labelOverhang', () => { + it('returns symmetric halves at 0° (horizontal)', () => { + const result = labelOverhang(100, LINE_HEIGHT, 0, false); + expect(result.left).toBe(50); + expect(result.right).toBe(50); + }); + + it('returns symmetric overhang at 45° when centered', () => { + const result = labelOverhang(100, LINE_HEIGHT, 45, false); + expect(result.left).toBeCloseTo(result.right); + }); + + it('returns asymmetric overhang at 45° when right-aligned', () => { + const result = labelOverhang(100, LINE_HEIGHT, 45, true); + expect(result.left).toBeGreaterThan(result.right); + }); + + it('returns lineHeight/2 on both sides at 90°', () => { + const result = labelOverhang(100, LINE_HEIGHT, 90, false); + expect(result.left).toBe(LINE_HEIGHT / 2); + expect(result.right).toBe(LINE_HEIGHT / 2); + }); +}); + +describe('edgeLabelsFit', () => { + it('returns true when both edges have enough space', () => { + expect(edgeLabelsFit(40, 40, LINE_HEIGHT, 0, 30, 30, false)).toBe(true); + }); + + it('returns false when first label overflows left', () => { + expect(edgeLabelsFit(100, 40, LINE_HEIGHT, 0, 30, 30, false)).toBe(false); + }); + + it('returns false when last label overflows right', () => { + expect(edgeLabelsFit(40, 100, LINE_HEIGHT, 0, 30, 30, false)).toBe(false); + }); +}); + +describe('edgeMaxLabelWidth', () => { + it('returns 2 * edgeSpace at 0°', () => { + expect(edgeMaxLabelWidth(50, LINE_HEIGHT, 0, false, 'first')).toBe(100); + }); + + it('returns Infinity at 90° (overhang is constant)', () => { + expect(edgeMaxLabelWidth(50, LINE_HEIGHT, 90, false, 'first')).toBe(Infinity); + }); + + it('returns finite value at 45° for first label when centered', () => { + const result = edgeMaxLabelWidth(50, LINE_HEIGHT, 45, false, 'first'); + expect(result).toBeGreaterThan(0); + expect(result).not.toBe(Infinity); + }); + + it('returns Infinity at 45° for last label when right-aligned', () => { + expect(edgeMaxLabelWidth(50, LINE_HEIGHT, 45, true, 'last')).toBe(Infinity); + }); + + it('returns finite value at 45° for first label when right-aligned', () => { + const result = edgeMaxLabelWidth(50, LINE_HEIGHT, 45, true, 'first'); + expect(result).toBeGreaterThan(0); + expect(result).not.toBe(Infinity); + }); + + it('returns 0 when edgeSpace is too small at 45° centered', () => { + // edgeSpace/SIN_45 - halfLH ≤ 0 → Math.max(0, ...) = 0 + expect(edgeMaxLabelWidth(1, LINE_HEIGHT, 45, false, 'first')).toBe(0); + }); +}); describe('normalizeAngle', () => { it('returns angle unchanged when within 0-360', () => { From 90b3a26964734940bbe303a03437103fb4e648ba Mon Sep 17 00:00:00 2001 From: Mateusz Rajski Date: Mon, 2 Mar 2026 14:23:27 +0100 Subject: [PATCH 08/13] Refactor tests --- tests/unit/Charts/useChartLabelLayout.test.ts | 51 +++---------------- tests/unit/components/Charts/utils.test.ts | 4 +- 2 files changed, 10 insertions(+), 45 deletions(-) diff --git a/tests/unit/Charts/useChartLabelLayout.test.ts b/tests/unit/Charts/useChartLabelLayout.test.ts index 56c9ed25f6b17..bae21e145cf6c 100644 --- a/tests/unit/Charts/useChartLabelLayout.test.ts +++ b/tests/unit/Charts/useChartLabelLayout.test.ts @@ -58,6 +58,8 @@ describe('useChartLabelLayout', () => { const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAA', 'BBB', 'CCC'), font, tickSpacing: 30, labelAreaWidth: 90})); expect(result.current.labelRotation).toBe(-0); expect(result.current.truncatedLabels).toEqual(['AAA', 'BBB', 'CCC']); + expect(result.current.xAxisLabelHeight).toBe(LINE_HEIGHT); + expect(result.current.labelSkipInterval).toBe(1); }); it('picks 45° when labels overflow horizontally but fit diagonally', () => { @@ -66,6 +68,8 @@ describe('useChartLabelLayout', () => { const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 40, labelAreaWidth: 400})); expect(result.current.labelRotation).toBe(-45); expect(result.current.truncatedLabels).toEqual(['AAAAAA', 'BBBBBB']); + expect(result.current.xAxisLabelHeight).toBeCloseTo((42 + LINE_HEIGHT) * SIN_45, 5); + expect(result.current.labelSkipInterval).toBe(1); }); it('picks 45° when labelAreaWidth is too narrow for 0° despite sufficient tickSpacing', () => { @@ -220,7 +224,7 @@ describe('useChartLabelLayout', () => { describe('skip interval', () => { it('computes skip interval > 1 at 90° when too many labels for the area', () => { - // 10 labels, forced to 90°. verticalWidth = lineHeight = 16. + // 10 labels, forced to 90°. At 90°, effectiveWidth = lineHeight = 16. // maxVisibleCount(100, 16) = floor(100/20) = 5 < 10 → skip = ceil(10/5) = 2 const labels = Array.from({length: 10}, (_, i) => `L${String(i).padStart(4, '0')}`); const {result} = renderHook(() => useChartLabelLayout({data: makeData(...labels), font, tickSpacing: 10, labelAreaWidth: 100})); @@ -232,37 +236,7 @@ describe('useChartLabelLayout', () => { const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 10, labelAreaWidth: 400})); expect(result.current.labelRotation).toBe(-90); expect(result.current.labelSkipInterval).toBe(1); - }); - - it('never uses skip interval at 0° or 45°', () => { - const {result: h} = renderHook(() => useChartLabelLayout({data: makeData('AAA', 'BBB'), font, tickSpacing: 50, labelAreaWidth: 200})); - expect(h.current.labelRotation).toBe(-0); - expect(h.current.labelSkipInterval).toBe(1); - - const {result: d} = renderHook(() => useChartLabelLayout({data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 40, labelAreaWidth: 400})); - expect(d.current.labelRotation).toBe(-45); - expect(d.current.labelSkipInterval).toBe(1); - }); - }); - - describe('xAxisLabelHeight', () => { - it('equals lineHeight at 0°', () => { - const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAA', 'BBB'), font, tickSpacing: 50, labelAreaWidth: 200})); - expect(result.current.labelRotation).toBe(-0); - expect(result.current.xAxisLabelHeight).toBe(LINE_HEIGHT); - }); - - it('uses diagonal formula at 45°', () => { - // effectiveHeight = (finalMaxWidth + lineHeight) * SIN_45. finalMaxWidth = 42 (no truncation) - const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 40, labelAreaWidth: 400})); - expect(result.current.labelRotation).toBe(-45); - expect(result.current.xAxisLabelHeight).toBeCloseTo((42 + LINE_HEIGHT) * SIN_45, 5); - }); - - it('equals label width at 90°', () => { - // effectiveHeight at 90° = finalMaxWidth = 42 - const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 10, labelAreaWidth: 400})); - expect(result.current.labelRotation).toBe(-90); + expect(result.current.truncatedLabels).toEqual(['AAAAAA', 'BBBBBB']); expect(result.current.xAxisLabelHeight).toBe(42); }); }); @@ -270,20 +244,9 @@ describe('useChartLabelLayout', () => { describe('edge cases', () => { it('handles single data point', () => { const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAA'), font, tickSpacing: 50, labelAreaWidth: 50})); + expect(result.current.labelRotation).toBe(-0); expect(result.current.truncatedLabels).toEqual(['AAA']); expect(result.current.labelSkipInterval).toBe(1); }); - - it('preserves all labels at 0° without truncation', () => { - const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAA', 'BBB'), font, tickSpacing: 50, labelAreaWidth: 200})); - expect(result.current.labelRotation).toBe(-0); - expect(result.current.truncatedLabels).toEqual(['AAA', 'BBB']); - }); - - it('preserves labels at 90° without truncation', () => { - const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 10, labelAreaWidth: 400})); - expect(result.current.labelRotation).toBe(-90); - expect(result.current.truncatedLabels).toEqual(['AAAAAA', 'BBBBBB']); - }); }); }); diff --git a/tests/unit/components/Charts/utils.test.ts b/tests/unit/components/Charts/utils.test.ts index df440572d31dc..09bb7defda15a 100644 --- a/tests/unit/components/Charts/utils.test.ts +++ b/tests/unit/components/Charts/utils.test.ts @@ -36,8 +36,10 @@ describe('truncateLabel', () => { expect(truncateLabel('Text', 28, 20, ellipsisWidth)).toBe('...'); }); - it('keeps at least 1 character before ellipsis', () => { + it('keeps at least 1 character before ellipsis even when space is extremely tight', () => { // available = 25 - 21 = 4, maxChars = max(1, floor(6 * 4/42)) = 1 + // Note: the hook enforces MIN_TRUNCATED_CHARS (10) so this extreme case + // only tests the pure function's floor behavior. expect(truncateLabel('ABCDEF', 42, 25, ellipsisWidth)).toBe('A...'); }); }); From da79b17b29f275082325e36146b0db85905b7930 Mon Sep 17 00:00:00 2001 From: Mateusz Rajski Date: Mon, 2 Mar 2026 15:01:00 +0100 Subject: [PATCH 09/13] Simplify hooks and tests --- .../Charts/components/ChartXAxisLabels.tsx | 2 +- .../Charts/hooks/useChartLabelFormats.ts | 2 +- .../Charts/hooks/useChartLabelLayout.ts | 4 +- src/components/Charts/utils.ts | 26 ++++++---- .../Charts/useChartLabelFormats.test.ts | 2 +- .../Charts/useChartLabelLayout.test.ts | 48 +++++++++---------- tests/unit/components/Charts/utils.test.ts | 8 ++-- 7 files changed, 51 insertions(+), 41 deletions(-) rename tests/unit/{ => components}/Charts/useChartLabelFormats.test.ts (98%) rename tests/unit/{ => components}/Charts/useChartLabelLayout.test.ts (88%) diff --git a/src/components/Charts/components/ChartXAxisLabels.tsx b/src/components/Charts/components/ChartXAxisLabels.tsx index d5ecaab6a0da7..be82416d936e2 100644 --- a/src/components/Charts/components/ChartXAxisLabels.tsx +++ b/src/components/Charts/components/ChartXAxisLabels.tsx @@ -8,7 +8,7 @@ type ChartXAxisLabelsProps = { /** Processed label strings (already truncated by the layout hook). */ labels: string[]; - /** Label rotation in degrees (negative value, e.g. -45). */ + /** Label rotation in degrees (e.g. 0, 45, 90). */ labelRotation: number; /** Show every Nth label (1 = all, 2 = every other, etc.). */ diff --git a/src/components/Charts/hooks/useChartLabelFormats.ts b/src/components/Charts/hooks/useChartLabelFormats.ts index 9b983be1c2726..f33220a42d13d 100644 --- a/src/components/Charts/hooks/useChartLabelFormats.ts +++ b/src/components/Charts/hooks/useChartLabelFormats.ts @@ -60,7 +60,7 @@ export default function useChartLabelFormats({data, font, unit, unitPosition = ' return ''; } - const sourceToUse = labelRotation === -LABEL_ROTATIONS.VERTICAL || !truncatedLabels ? data.map((p) => p.label) : truncatedLabels; + const sourceToUse = labelRotation === LABEL_ROTATIONS.VERTICAL || !truncatedLabels ? data.map((p) => p.label) : truncatedLabels; return sourceToUse.at(index) ?? ''; }; diff --git a/src/components/Charts/hooks/useChartLabelLayout.ts b/src/components/Charts/hooks/useChartLabelLayout.ts index ae5fb114b9c0b..64b23eeeee035 100644 --- a/src/components/Charts/hooks/useChartLabelLayout.ts +++ b/src/components/Charts/hooks/useChartLabelLayout.ts @@ -57,7 +57,7 @@ function useChartLabelLayout({data, font, tickSpacing, labelAreaWidth, firstTick const hWidth = effectiveWidth(maxLabelWidth, lineHeight, LABEL_ROTATIONS.HORIZONTAL); const hFitsInTicks = hWidth + LABEL_PADDING <= tickSpacing && maxVisibleCount(labelAreaWidth, hWidth) >= data.length; - const hEdgeFits = edgeLabelsFit(firstLabelWidth, lastLabelWidth, lineHeight, LABEL_ROTATIONS.HORIZONTAL, firstTickLeftSpace, lastTickRightSpace, false); + const hEdgeFits = edgeLabelsFit({firstLabelWidth, lastLabelWidth, lineHeight, rotation: LABEL_ROTATIONS.HORIZONTAL, firstTickLeftSpace, lastTickRightSpace, rightAligned: false}); if (hFitsInTicks && hEdgeFits) { rotation = LABEL_ROTATIONS.HORIZONTAL; @@ -108,7 +108,7 @@ function useChartLabelLayout({data, font, tickSpacing, labelAreaWidth, firstTick const xAxisLabelHeight = effectiveHeight(finalMaxWidth, lineHeight, rotation); return { - labelRotation: -rotation, + labelRotation: rotation, labelSkipInterval: skipInterval, truncatedLabels: finalLabels, xAxisLabelHeight, diff --git a/src/components/Charts/utils.ts b/src/components/Charts/utils.ts index cb1fa6097c0a1..e02270f794b16 100644 --- a/src/components/Charts/utils.ts +++ b/src/components/Charts/utils.ts @@ -244,15 +244,23 @@ function labelOverhang(labelWidth: number, lineHeight: number, rotation: number, } /** Check if first and last labels fit within the available canvas edge space. */ -function edgeLabelsFit( - firstLabelWidth: number, - lastLabelWidth: number, - lineHeight: number, - rotation: number, - firstTickLeftSpace: number, - lastTickRightSpace: number, - rightAligned: boolean, -): boolean { +function edgeLabelsFit({ + firstLabelWidth, + lastLabelWidth, + lineHeight, + rotation, + firstTickLeftSpace, + lastTickRightSpace, + rightAligned, +}: { + firstLabelWidth: number; + lastLabelWidth: number; + lineHeight: number; + rotation: number; + firstTickLeftSpace: number; + lastTickRightSpace: number; + rightAligned: boolean; +}): boolean { const first = labelOverhang(firstLabelWidth, lineHeight, rotation, rightAligned); const last = labelOverhang(lastLabelWidth, lineHeight, rotation, rightAligned); return first.left <= firstTickLeftSpace && last.right <= lastTickRightSpace; diff --git a/tests/unit/Charts/useChartLabelFormats.test.ts b/tests/unit/components/Charts/useChartLabelFormats.test.ts similarity index 98% rename from tests/unit/Charts/useChartLabelFormats.test.ts rename to tests/unit/components/Charts/useChartLabelFormats.test.ts index f07cfbc9e1979..418f53671ef26 100644 --- a/tests/unit/Charts/useChartLabelFormats.test.ts +++ b/tests/unit/components/Charts/useChartLabelFormats.test.ts @@ -84,7 +84,7 @@ describe('formatLabel', () => { }); it('ignores truncatedLabels when rotation is vertical', () => { - const {result} = renderHook(() => useChartLabelFormats({data: SAMPLE_DATA, truncatedLabels: ['J', 'F'], labelRotation: -90})); + const {result} = renderHook(() => useChartLabelFormats({data: SAMPLE_DATA, truncatedLabels: ['J', 'F'], labelRotation: 90})); expect(result.current.formatLabel(0)).toBe('Jan'); expect(result.current.formatLabel(1)).toBe('Feb'); diff --git a/tests/unit/Charts/useChartLabelLayout.test.ts b/tests/unit/components/Charts/useChartLabelLayout.test.ts similarity index 88% rename from tests/unit/Charts/useChartLabelLayout.test.ts rename to tests/unit/components/Charts/useChartLabelLayout.test.ts index bae21e145cf6c..596a8c4978b67 100644 --- a/tests/unit/Charts/useChartLabelLayout.test.ts +++ b/tests/unit/components/Charts/useChartLabelLayout.test.ts @@ -56,7 +56,7 @@ describe('useChartLabelLayout', () => { it('picks 0° when labels fit horizontally', () => { // "AAA" = 21px. 21+4=25 ≤ tickSpacing(30). maxVisibleCount(90,21)=3 ≥ 3 const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAA', 'BBB', 'CCC'), font, tickSpacing: 30, labelAreaWidth: 90})); - expect(result.current.labelRotation).toBe(-0); + expect(result.current.labelRotation).toBe(0); expect(result.current.truncatedLabels).toEqual(['AAA', 'BBB', 'CCC']); expect(result.current.xAxisLabelHeight).toBe(LINE_HEIGHT); expect(result.current.labelSkipInterval).toBe(1); @@ -66,7 +66,7 @@ describe('useChartLabelLayout', () => { // "AAAAAA" = 42px. 42+4=46 > tickSpacing(40) → 0° fails. // At 45°: 42*SIN_45 ≈ 29.7, 29.7+4 ≤ 40 ✓ const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 40, labelAreaWidth: 400})); - expect(result.current.labelRotation).toBe(-45); + expect(result.current.labelRotation).toBe(45); expect(result.current.truncatedLabels).toEqual(['AAAAAA', 'BBBBBB']); expect(result.current.xAxisLabelHeight).toBeCloseTo((42 + LINE_HEIGHT) * SIN_45, 5); expect(result.current.labelSkipInterval).toBe(1); @@ -77,13 +77,13 @@ describe('useChartLabelLayout', () => { // BUT labelAreaWidth=40: maxVisibleCount(40,21) = floor(40/25) = 1 < 3 → 0° fails. // At 45°: 21*SIN_45 ≈ 14.85, 14.85+4=18.85 ≤ 30 ✓ → 45° selected. const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAA', 'BBB', 'CCC'), font, tickSpacing: 30, labelAreaWidth: 40})); - expect(result.current.labelRotation).toBe(-45); + expect(result.current.labelRotation).toBe(45); }); it('picks 90° when labels overflow at all rotations', () => { // tickSpacing=20: 0° fails (46>20), 45° fails (29.7+4=33.7>20) const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 20, labelAreaWidth: 400})); - expect(result.current.labelRotation).toBe(-90); + expect(result.current.labelRotation).toBe(90); expect(result.current.truncatedLabels).toEqual(['AAAAAA', 'BBBBBB']); }); }); @@ -107,7 +107,7 @@ describe('useChartLabelLayout', () => { lastTickRightSpace: Infinity, }), ); - expect(result.current.labelRotation).toBe(-0); + expect(result.current.labelRotation).toBe(0); }); }); @@ -117,11 +117,11 @@ describe('useChartLabelLayout', () => { const config = {data: makeData('A'.repeat(16), 'BB', 'CC'), font, tickSpacing: 120, labelAreaWidth: 360}; const {result: noEdge} = renderHook(() => useChartLabelLayout(config)); - expect(noEdge.current.labelRotation).toBe(-0); + expect(noEdge.current.labelRotation).toBe(0); // firstTickLeftSpace=40 < 56 → 0° edge fails → escalates to 45° const {result: withEdge} = renderHook(() => useChartLabelLayout({...config, firstTickLeftSpace: 40, lastTickRightSpace: 200})); - expect(withEdge.current.labelRotation).toBe(-45); + expect(withEdge.current.labelRotation).toBe(45); }); it('escalates to 90° when edge space is too small for both 0° and 45°', () => { @@ -129,7 +129,7 @@ describe('useChartLabelLayout', () => { const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 50, labelAreaWidth: 200, firstTickLeftSpace: 5, lastTickRightSpace: 5}), ); - expect(result.current.labelRotation).toBe(-90); + expect(result.current.labelRotation).toBe(90); }); it('allowTightDiagonalPacking enables 45° at tighter tick spacing', () => { @@ -139,10 +139,10 @@ describe('useChartLabelLayout', () => { const base = {data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 30, labelAreaWidth: 400, firstTickLeftSpace: 100, lastTickRightSpace: 100}; const {result: noPacking} = renderHook(() => useChartLabelLayout({...base, allowTightDiagonalPacking: false})); - expect(noPacking.current.labelRotation).toBe(-90); + expect(noPacking.current.labelRotation).toBe(90); const {result: withPacking} = renderHook(() => useChartLabelLayout({...base, allowTightDiagonalPacking: true})); - expect(withPacking.current.labelRotation).toBe(-45); + expect(withPacking.current.labelRotation).toBe(45); }); }); @@ -160,10 +160,10 @@ describe('useChartLabelLayout', () => { lastTickRightSpace: 200, }), ); - expect(result.current.labelRotation).toBe(-45); - expect(result.current.truncatedLabels?.at(0)).toBe(`${'A'.repeat(10)}...`); - expect(result.current.truncatedLabels?.at(1)).toBe('BB'); - expect(result.current.truncatedLabels?.at(2)).toBe('CC'); + expect(result.current.labelRotation).toBe(45); + expect(result.current.truncatedLabels.at(0)).toBe(`${'A'.repeat(10)}...`); + expect(result.current.truncatedLabels.at(1)).toBe('BB'); + expect(result.current.truncatedLabels.at(2)).toBe('CC'); }); it('truncates first label due to edge constraint (right-aligned)', () => { @@ -180,9 +180,9 @@ describe('useChartLabelLayout', () => { allowTightDiagonalPacking: true, }), ); - expect(result.current.labelRotation).toBe(-45); - expect(result.current.truncatedLabels?.at(0)).toBe(`${'A'.repeat(10)}...`); - expect(result.current.truncatedLabels?.at(1)).toBe('BB'); + expect(result.current.labelRotation).toBe(45); + expect(result.current.truncatedLabels.at(0)).toBe(`${'A'.repeat(10)}...`); + expect(result.current.truncatedLabels.at(1)).toBe('BB'); }); it('truncates last label when centered due to symmetric overhang', () => { @@ -198,8 +198,8 @@ describe('useChartLabelLayout', () => { lastTickRightSpace: 40, }), ); - expect(result.current.labelRotation).toBe(-45); - expect(result.current.truncatedLabels?.at(2)).toBe(`${'A'.repeat(10)}...`); + expect(result.current.labelRotation).toBe(45); + expect(result.current.truncatedLabels.at(2)).toBe(`${'A'.repeat(10)}...`); }); it('does NOT truncate last label when right-aligned despite tight right edge', () => { @@ -217,8 +217,8 @@ describe('useChartLabelLayout', () => { allowTightDiagonalPacking: true, }), ); - expect(result.current.labelRotation).toBe(-45); - expect(result.current.truncatedLabels?.at(2)).toBe('A'.repeat(16)); + expect(result.current.labelRotation).toBe(45); + expect(result.current.truncatedLabels.at(2)).toBe('A'.repeat(16)); }); }); @@ -228,13 +228,13 @@ describe('useChartLabelLayout', () => { // maxVisibleCount(100, 16) = floor(100/20) = 5 < 10 → skip = ceil(10/5) = 2 const labels = Array.from({length: 10}, (_, i) => `L${String(i).padStart(4, '0')}`); const {result} = renderHook(() => useChartLabelLayout({data: makeData(...labels), font, tickSpacing: 10, labelAreaWidth: 100})); - expect(result.current.labelRotation).toBe(-90); + expect(result.current.labelRotation).toBe(90); expect(result.current.labelSkipInterval).toBe(2); }); it('returns skip interval 1 at 90° when labels fit', () => { const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAAAAA', 'BBBBBB'), font, tickSpacing: 10, labelAreaWidth: 400})); - expect(result.current.labelRotation).toBe(-90); + expect(result.current.labelRotation).toBe(90); expect(result.current.labelSkipInterval).toBe(1); expect(result.current.truncatedLabels).toEqual(['AAAAAA', 'BBBBBB']); expect(result.current.xAxisLabelHeight).toBe(42); @@ -244,7 +244,7 @@ describe('useChartLabelLayout', () => { describe('edge cases', () => { it('handles single data point', () => { const {result} = renderHook(() => useChartLabelLayout({data: makeData('AAA'), font, tickSpacing: 50, labelAreaWidth: 50})); - expect(result.current.labelRotation).toBe(-0); + expect(result.current.labelRotation).toBe(0); expect(result.current.truncatedLabels).toEqual(['AAA']); expect(result.current.labelSkipInterval).toBe(1); }); diff --git a/tests/unit/components/Charts/utils.test.ts b/tests/unit/components/Charts/utils.test.ts index 09bb7defda15a..d575f8ffc5382 100644 --- a/tests/unit/components/Charts/utils.test.ts +++ b/tests/unit/components/Charts/utils.test.ts @@ -112,16 +112,18 @@ describe('labelOverhang', () => { }); describe('edgeLabelsFit', () => { + const base = {firstLabelWidth: 40, lastLabelWidth: 40, lineHeight: LINE_HEIGHT, rotation: 0, firstTickLeftSpace: 30, lastTickRightSpace: 30, rightAligned: false}; + it('returns true when both edges have enough space', () => { - expect(edgeLabelsFit(40, 40, LINE_HEIGHT, 0, 30, 30, false)).toBe(true); + expect(edgeLabelsFit(base)).toBe(true); }); it('returns false when first label overflows left', () => { - expect(edgeLabelsFit(100, 40, LINE_HEIGHT, 0, 30, 30, false)).toBe(false); + expect(edgeLabelsFit({...base, firstLabelWidth: 100})).toBe(false); }); it('returns false when last label overflows right', () => { - expect(edgeLabelsFit(40, 100, LINE_HEIGHT, 0, 30, 30, false)).toBe(false); + expect(edgeLabelsFit({...base, lastLabelWidth: 100})).toBe(false); }); }); From cfb7792a81d3bd1f29f7d995e27c4a3d84920a03 Mon Sep 17 00:00:00 2001 From: Mateusz Rajski Date: Mon, 2 Mar 2026 15:34:31 +0100 Subject: [PATCH 10/13] Unify chart padding in both cartesian charts --- src/components/Charts/BarChart/BarChartContent.tsx | 8 ++++---- src/components/Charts/LineChart/LineChartContent.tsx | 8 ++++---- src/components/Charts/constants.ts | 4 ++-- src/components/Charts/hooks/useChartLabelLayout.ts | 8 ++++---- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/components/Charts/BarChart/BarChartContent.tsx b/src/components/Charts/BarChart/BarChartContent.tsx index b8dd49e2f964c..c13020040eb0b 100644 --- a/src/components/Charts/BarChart/BarChartContent.tsx +++ b/src/components/Charts/BarChart/BarChartContent.tsx @@ -175,9 +175,9 @@ function BarChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUni ); }; - const dynamicChartStyle = { - height: CHART_CONTENT_MIN_HEIGHT + (xAxisLabelHeight ?? 0), - }; + const labelSpace = AXIS_LABEL_GAP + (xAxisLabelHeight ?? 0); + const dynamicChartStyle = {height: CHART_CONTENT_MIN_HEIGHT + labelSpace}; + const chartPadding = {...CHART_PADDING, bottom: labelSpace + CHART_PADDING.bottom}; if (isLoading || !font) { return ( @@ -204,7 +204,7 @@ function BarChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUni {chartWidth > 0 && ( 0 && ( measureTextWidth(label, font)); const finalMaxWidth = Math.max(...finalWidths); let skipInterval = 1; @@ -104,7 +104,7 @@ function useChartLabelLayout({data, font, tickSpacing, labelAreaWidth, firstTick skipInterval = visibleCount >= data.length ? 1 : Math.ceil(data.length / Math.max(1, visibleCount)); } - // --- 4. Compute vertical space needed for x-axis labels --- + // Compute vertical space needed for x-axis labels const xAxisLabelHeight = effectiveHeight(finalMaxWidth, lineHeight, rotation); return { From a4b612f2a42b8bf8daac97b02e3b5eead17a80a5 Mon Sep 17 00:00:00 2001 From: Mateusz Rajski Date: Wed, 4 Mar 2026 13:07:48 +0100 Subject: [PATCH 11/13] Prevent label jumping --- src/components/Charts/BarChart/BarChartContent.tsx | 6 +++--- src/components/Charts/LineChart/LineChartContent.tsx | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/Charts/BarChart/BarChartContent.tsx b/src/components/Charts/BarChart/BarChartContent.tsx index c13020040eb0b..d563e4bae0463 100644 --- a/src/components/Charts/BarChart/BarChartContent.tsx +++ b/src/components/Charts/BarChart/BarChartContent.tsx @@ -157,8 +157,8 @@ function BarChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUni ); }; - const renderCustomXLabels = (args: CartesianChartRenderArg<{x: number; y: number}, 'y'>) => { - if (!font) { + const renderOutside = (args: CartesianChartRenderArg<{x: number; y: number}, 'y'>) => { + if (!font || xAxisLabelHeight === undefined) { return null; } return ( @@ -211,7 +211,7 @@ function BarChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUni customGestures={customGestures} onChartBoundsChange={handleChartBoundsChange} onScaleChange={handleScaleChange} - renderOutside={renderCustomXLabels} + renderOutside={renderOutside} xAxis={{ tickCount: data.length, lineWidth: X_AXIS_LINE_WIDTH, diff --git a/src/components/Charts/LineChart/LineChartContent.tsx b/src/components/Charts/LineChart/LineChartContent.tsx index fd606dcaafb09..12d8cbe458b57 100644 --- a/src/components/Charts/LineChart/LineChartContent.tsx +++ b/src/components/Charts/LineChart/LineChartContent.tsx @@ -140,7 +140,7 @@ function LineChartContent({data, title, titleIcon, isLoading, yAxisUnit, yAxisUn const tooltipData = useTooltipData(activeDataIndex, data, formatValue); - const renderOutsideComponents = (args: CartesianChartRenderArg<{x: number; y: number}, 'y'>) => { + const renderOutside = (args: CartesianChartRenderArg<{x: number; y: number}, 'y'>) => { return ( <> - {!!font && ( + {!!font && xAxisLabelHeight !== undefined && ( Date: Wed, 4 Mar 2026 13:22:09 +0100 Subject: [PATCH 12/13] Address review --- .../Charts/components/ChartXAxisLabels.tsx | 3 +- .../Charts/hooks/useChartLabelFormats.ts | 4 +- .../Charts/hooks/useChartLabelLayout.ts | 6 +-- src/components/Charts/types.ts | 6 ++- src/components/Charts/utils.ts | 12 ++--- tests/unit/components/Charts/utils.test.ts | 44 +++++++++++-------- 6 files changed, 44 insertions(+), 31 deletions(-) diff --git a/src/components/Charts/components/ChartXAxisLabels.tsx b/src/components/Charts/components/ChartXAxisLabels.tsx index be82416d936e2..6e1193ef2cf9d 100644 --- a/src/components/Charts/components/ChartXAxisLabels.tsx +++ b/src/components/Charts/components/ChartXAxisLabels.tsx @@ -2,6 +2,7 @@ import {Group, Text as SkiaText, vec} from '@shopify/react-native-skia'; import type {SkFont} from '@shopify/react-native-skia'; import React, {useMemo} from 'react'; import {AXIS_LABEL_GAP} from '@components/Charts/constants'; +import type {LabelRotation} from '@components/Charts/types'; import {measureTextWidth, rotatedLabelCenterCorrection, rotatedLabelYOffset} from '@components/Charts/utils'; type ChartXAxisLabelsProps = { @@ -9,7 +10,7 @@ type ChartXAxisLabelsProps = { labels: string[]; /** Label rotation in degrees (e.g. 0, 45, 90). */ - labelRotation: number; + labelRotation: LabelRotation; /** Show every Nth label (1 = all, 2 = every other, etc.). */ labelSkipInterval: number; diff --git a/src/components/Charts/hooks/useChartLabelFormats.ts b/src/components/Charts/hooks/useChartLabelFormats.ts index f33220a42d13d..e9c2d84b2c037 100644 --- a/src/components/Charts/hooks/useChartLabelFormats.ts +++ b/src/components/Charts/hooks/useChartLabelFormats.ts @@ -1,6 +1,6 @@ import type {SkFont} from '@shopify/react-native-skia'; import {LABEL_ROTATIONS} from '@components/Charts/constants'; -import type {ChartDataPoint, UnitPosition, UnitWithFallback} from '@components/Charts/types'; +import type {ChartDataPoint, LabelRotation, UnitPosition, UnitWithFallback} from '@components/Charts/types'; import useLocalize from '@hooks/useLocalize'; type UseChartLabelFormatsProps = { @@ -9,7 +9,7 @@ type UseChartLabelFormatsProps = { unit?: UnitWithFallback | string; unitPosition?: UnitPosition; labelSkipInterval?: number; - labelRotation?: number; + labelRotation?: LabelRotation; truncatedLabels?: string[]; }; diff --git a/src/components/Charts/hooks/useChartLabelLayout.ts b/src/components/Charts/hooks/useChartLabelLayout.ts index 5b7d0abfda06a..bd3fe4e383fd2 100644 --- a/src/components/Charts/hooks/useChartLabelLayout.ts +++ b/src/components/Charts/hooks/useChartLabelLayout.ts @@ -1,6 +1,6 @@ import type {SkFont} from '@shopify/react-native-skia'; import {ELLIPSIS, LABEL_PADDING, LABEL_ROTATIONS, MIN_TRUNCATED_CHARS, SIN_45} from '@components/Charts/constants'; -import type {ChartDataPoint} from '@components/Charts/types'; +import type {ChartDataPoint, LabelRotation} from '@components/Charts/types'; import {edgeLabelsFit, edgeMaxLabelWidth, effectiveHeight, effectiveWidth, maxVisibleCount, measureTextWidth, truncateLabel} from '@components/Charts/utils'; type LabelLayoutConfig = { @@ -28,7 +28,7 @@ type LabelLayoutConfig = { function useChartLabelLayout({data, font, tickSpacing, labelAreaWidth, firstTickLeftSpace = Infinity, lastTickRightSpace = Infinity, allowTightDiagonalPacking = false}: LabelLayoutConfig) { if (!font || data.length === 0 || tickSpacing <= 0 || labelAreaWidth <= 0) { - return {labelRotation: 0, labelSkipInterval: 1, truncatedLabels: [] as string[]}; + return {labelRotation: LABEL_ROTATIONS.HORIZONTAL, labelSkipInterval: 1, truncatedLabels: [] as string[]}; } const fontMetrics = font.getMetrics(); @@ -53,7 +53,7 @@ function useChartLabelLayout({data, font, tickSpacing, labelAreaWidth, firstTick const lastMinTrunc = lastLabel.length <= MIN_TRUNCATED_CHARS ? lastLabelWidth : measureTextWidth(lastLabel.slice(0, MIN_TRUNCATED_CHARS) + ELLIPSIS, font); // Pick rotation (prefer 0° → 45° → 90°) - let rotation: number = LABEL_ROTATIONS.VERTICAL; + let rotation: LabelRotation = LABEL_ROTATIONS.VERTICAL; const hWidth = effectiveWidth(maxLabelWidth, lineHeight, LABEL_ROTATIONS.HORIZONTAL); const hFitsInTicks = hWidth + LABEL_PADDING <= tickSpacing && maxVisibleCount(labelAreaWidth, hWidth) >= data.length; diff --git a/src/components/Charts/types.ts b/src/components/Charts/types.ts index 92b6bf0f41f41..62d5c342239d4 100644 --- a/src/components/Charts/types.ts +++ b/src/components/Charts/types.ts @@ -1,4 +1,6 @@ +import type {ValueOf} from 'type-fest'; import type IconAsset from '@src/types/utils/IconAsset'; +import {LABEL_ROTATIONS} from './constants'; type ChartDataPoint = { /** Label displayed under the data point (e.g., "Amazon", "Nov 2025") */ @@ -65,4 +67,6 @@ type PieSlice = { originalIndex: number; }; -export type {ChartDataPoint, ChartProps, CartesianChartProps, PieSlice, UnitPosition, UnitWithFallback}; +type LabelRotation = ValueOf; + +export type {ChartDataPoint, ChartProps, CartesianChartProps, LabelRotation, PieSlice, UnitPosition, UnitWithFallback}; diff --git a/src/components/Charts/utils.ts b/src/components/Charts/utils.ts index e02270f794b16..07a2de4d7589b 100644 --- a/src/components/Charts/utils.ts +++ b/src/components/Charts/utils.ts @@ -1,7 +1,7 @@ import type {SkFont} from '@shopify/react-native-skia'; import colors from '@styles/theme/colors'; import {ELLIPSIS, LABEL_PADDING, LABEL_ROTATIONS, SIN_45} from './constants'; -import type {ChartDataPoint, PieSlice} from './types'; +import type {ChartDataPoint, LabelRotation, PieSlice} from './types'; /** * Expensify Chart Color Palette. @@ -195,7 +195,7 @@ function truncateLabel(label: string, labelWidth: number, maxWidth: number, elli } /** Horizontal footprint of a label at a given rotation angle (for inter-tick overlap checks). */ -function effectiveWidth(labelWidth: number, lineHeight: number, rotation: number): number { +function effectiveWidth(labelWidth: number, lineHeight: number, rotation: LabelRotation): number { if (rotation === LABEL_ROTATIONS.VERTICAL) { return lineHeight; } @@ -206,7 +206,7 @@ function effectiveWidth(labelWidth: number, lineHeight: number, rotation: number } /** Vertical footprint of a label at a given rotation angle. */ -function effectiveHeight(labelWidth: number, lineHeight: number, rotation: number): number { +function effectiveHeight(labelWidth: number, lineHeight: number, rotation: LabelRotation): number { if (rotation === LABEL_ROTATIONS.VERTICAL) { return labelWidth; } @@ -225,7 +225,7 @@ function maxVisibleCount(areaWidth: number, itemWidth: number): number { * How far a label extends beyond its tick position after rotation. * Accounts for the rotatedLabelCenterCorrection translateX applied during rendering. */ -function labelOverhang(labelWidth: number, lineHeight: number, rotation: number, rightAligned: boolean): {left: number; right: number} { +function labelOverhang(labelWidth: number, lineHeight: number, rotation: LabelRotation, rightAligned: boolean): {left: number; right: number} { if (rotation === LABEL_ROTATIONS.HORIZONTAL) { return {left: labelWidth / 2, right: labelWidth / 2}; } @@ -256,7 +256,7 @@ function edgeLabelsFit({ firstLabelWidth: number; lastLabelWidth: number; lineHeight: number; - rotation: number; + rotation: LabelRotation; firstTickLeftSpace: number; lastTickRightSpace: number; rightAligned: boolean; @@ -270,7 +270,7 @@ function edgeLabelsFit({ * Maximum label width that fits within the available edge space at a given rotation. * Returns Infinity when the overhang at that edge doesn't depend on label width. */ -function edgeMaxLabelWidth(edgeSpace: number, lineHeight: number, rotation: number, rightAligned: boolean, edge: 'first' | 'last'): number { +function edgeMaxLabelWidth(edgeSpace: number, lineHeight: number, rotation: LabelRotation, rightAligned: boolean, edge: 'first' | 'last'): number { const halfLH = lineHeight / 2; if (rotation === LABEL_ROTATIONS.HORIZONTAL) { return 2 * edgeSpace; diff --git a/tests/unit/components/Charts/utils.test.ts b/tests/unit/components/Charts/utils.test.ts index d575f8ffc5382..2e2987c8b19be 100644 --- a/tests/unit/components/Charts/utils.test.ts +++ b/tests/unit/components/Charts/utils.test.ts @@ -1,4 +1,4 @@ -import {SIN_45} from '@components/Charts/constants'; +import {LABEL_ROTATIONS, SIN_45} from '@components/Charts/constants'; import type {ChartDataPoint, PieSlice} from '@components/Charts/types'; import { edgeLabelsFit, @@ -46,29 +46,29 @@ describe('truncateLabel', () => { describe('effectiveWidth', () => { it('returns labelWidth at 0°', () => { - expect(effectiveWidth(100, LINE_HEIGHT, 0)).toBe(100); + expect(effectiveWidth(100, LINE_HEIGHT, LABEL_ROTATIONS.HORIZONTAL)).toBe(100); }); it('returns labelWidth * sin(45°) at 45°', () => { - expect(effectiveWidth(100, LINE_HEIGHT, 45)).toBeCloseTo(100 * SIN_45); + expect(effectiveWidth(100, LINE_HEIGHT, LABEL_ROTATIONS.DIAGONAL)).toBeCloseTo(100 * SIN_45); }); it('returns lineHeight at 90°', () => { - expect(effectiveWidth(100, LINE_HEIGHT, 90)).toBe(LINE_HEIGHT); + expect(effectiveWidth(100, LINE_HEIGHT, LABEL_ROTATIONS.VERTICAL)).toBe(LINE_HEIGHT); }); }); describe('effectiveHeight', () => { it('returns lineHeight at 0°', () => { - expect(effectiveHeight(100, LINE_HEIGHT, 0)).toBe(LINE_HEIGHT); + expect(effectiveHeight(100, LINE_HEIGHT, LABEL_ROTATIONS.HORIZONTAL)).toBe(LINE_HEIGHT); }); it('returns (labelWidth + lineHeight) * sin(45°) at 45°', () => { - expect(effectiveHeight(100, LINE_HEIGHT, 45)).toBeCloseTo((100 + LINE_HEIGHT) * SIN_45); + expect(effectiveHeight(100, LINE_HEIGHT, LABEL_ROTATIONS.DIAGONAL)).toBeCloseTo((100 + LINE_HEIGHT) * SIN_45); }); it('returns labelWidth at 90°', () => { - expect(effectiveHeight(100, LINE_HEIGHT, 90)).toBe(100); + expect(effectiveHeight(100, LINE_HEIGHT, LABEL_ROTATIONS.VERTICAL)).toBe(100); }); }); @@ -89,30 +89,38 @@ describe('maxVisibleCount', () => { describe('labelOverhang', () => { it('returns symmetric halves at 0° (horizontal)', () => { - const result = labelOverhang(100, LINE_HEIGHT, 0, false); + const result = labelOverhang(100, LINE_HEIGHT, LABEL_ROTATIONS.HORIZONTAL, false); expect(result.left).toBe(50); expect(result.right).toBe(50); }); it('returns symmetric overhang at 45° when centered', () => { - const result = labelOverhang(100, LINE_HEIGHT, 45, false); + const result = labelOverhang(100, LINE_HEIGHT, LABEL_ROTATIONS.DIAGONAL, false); expect(result.left).toBeCloseTo(result.right); }); it('returns asymmetric overhang at 45° when right-aligned', () => { - const result = labelOverhang(100, LINE_HEIGHT, 45, true); + const result = labelOverhang(100, LINE_HEIGHT, LABEL_ROTATIONS.DIAGONAL, true); expect(result.left).toBeGreaterThan(result.right); }); it('returns lineHeight/2 on both sides at 90°', () => { - const result = labelOverhang(100, LINE_HEIGHT, 90, false); + const result = labelOverhang(100, LINE_HEIGHT, LABEL_ROTATIONS.VERTICAL, false); expect(result.left).toBe(LINE_HEIGHT / 2); expect(result.right).toBe(LINE_HEIGHT / 2); }); }); describe('edgeLabelsFit', () => { - const base = {firstLabelWidth: 40, lastLabelWidth: 40, lineHeight: LINE_HEIGHT, rotation: 0, firstTickLeftSpace: 30, lastTickRightSpace: 30, rightAligned: false}; + const base = { + firstLabelWidth: 40, + lastLabelWidth: 40, + lineHeight: LINE_HEIGHT, + rotation: LABEL_ROTATIONS.HORIZONTAL, + firstTickLeftSpace: 30, + lastTickRightSpace: 30, + rightAligned: false, + }; it('returns true when both edges have enough space', () => { expect(edgeLabelsFit(base)).toBe(true); @@ -129,32 +137,32 @@ describe('edgeLabelsFit', () => { describe('edgeMaxLabelWidth', () => { it('returns 2 * edgeSpace at 0°', () => { - expect(edgeMaxLabelWidth(50, LINE_HEIGHT, 0, false, 'first')).toBe(100); + expect(edgeMaxLabelWidth(50, LINE_HEIGHT, LABEL_ROTATIONS.HORIZONTAL, false, 'first')).toBe(100); }); it('returns Infinity at 90° (overhang is constant)', () => { - expect(edgeMaxLabelWidth(50, LINE_HEIGHT, 90, false, 'first')).toBe(Infinity); + expect(edgeMaxLabelWidth(50, LINE_HEIGHT, LABEL_ROTATIONS.VERTICAL, false, 'first')).toBe(Infinity); }); it('returns finite value at 45° for first label when centered', () => { - const result = edgeMaxLabelWidth(50, LINE_HEIGHT, 45, false, 'first'); + const result = edgeMaxLabelWidth(50, LINE_HEIGHT, LABEL_ROTATIONS.DIAGONAL, false, 'first'); expect(result).toBeGreaterThan(0); expect(result).not.toBe(Infinity); }); it('returns Infinity at 45° for last label when right-aligned', () => { - expect(edgeMaxLabelWidth(50, LINE_HEIGHT, 45, true, 'last')).toBe(Infinity); + expect(edgeMaxLabelWidth(50, LINE_HEIGHT, LABEL_ROTATIONS.DIAGONAL, true, 'last')).toBe(Infinity); }); it('returns finite value at 45° for first label when right-aligned', () => { - const result = edgeMaxLabelWidth(50, LINE_HEIGHT, 45, true, 'first'); + const result = edgeMaxLabelWidth(50, LINE_HEIGHT, LABEL_ROTATIONS.DIAGONAL, true, 'first'); expect(result).toBeGreaterThan(0); expect(result).not.toBe(Infinity); }); it('returns 0 when edgeSpace is too small at 45° centered', () => { // edgeSpace/SIN_45 - halfLH ≤ 0 → Math.max(0, ...) = 0 - expect(edgeMaxLabelWidth(1, LINE_HEIGHT, 45, false, 'first')).toBe(0); + expect(edgeMaxLabelWidth(1, LINE_HEIGHT, LABEL_ROTATIONS.DIAGONAL, false, 'first')).toBe(0); }); }); From d3c130e79d2628b2e0ca2a91233f4195a14235c9 Mon Sep 17 00:00:00 2001 From: Mateusz Rajski Date: Wed, 4 Mar 2026 13:56:29 +0100 Subject: [PATCH 13/13] Fix eslint --- src/components/Charts/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Charts/types.ts b/src/components/Charts/types.ts index 62d5c342239d4..72c7f008c49fd 100644 --- a/src/components/Charts/types.ts +++ b/src/components/Charts/types.ts @@ -1,6 +1,6 @@ import type {ValueOf} from 'type-fest'; import type IconAsset from '@src/types/utils/IconAsset'; -import {LABEL_ROTATIONS} from './constants'; +import type {LABEL_ROTATIONS} from './constants'; type ChartDataPoint = { /** Label displayed under the data point (e.g., "Amazon", "Nov 2025") */