Skip to content

Commit 963fbb9

Browse files
committed
More chart label layout fixes + remove unused util arguments
1 parent 0e1929e commit 963fbb9

7 files changed

Lines changed: 41 additions & 137 deletions

File tree

src/components/Charts/BarChart/BarChartContent.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ function BarChartContent({data, isLoading, yAxisUnit, yAxisUnitPosition = 'left'
231231
fontMgr={fontMgr}
232232
labelColor={theme.textSupporting}
233233
xScale={args.xScale}
234-
chartBoundsBottom={args.chartBounds.bottom}
234+
chartBoundsBottom={data.some((p) => p.total < 0) ? args.chartBounds.bottom : args.yScale(0)}
235235
/>
236236
<ChartYAxisLabels
237237
yTicks={args.yTicks}
@@ -257,7 +257,7 @@ function BarChartContent({data, isLoading, yAxisUnit, yAxisUnitPosition = 'left'
257257
fontMgr,
258258
variables.iconSizeExtraSmall,
259259
);
260-
const chartPadding = {...CHART_PADDING, bottom: labelSpace + CHART_PADDING.bottom + variables.iconSizeExtraSmall, left: yAxisLabelWidth + GLYPH_PADDING};
260+
const chartPadding = {...CHART_PADDING, bottom: labelSpace + CHART_PADDING.bottom, left: yAxisLabelWidth + GLYPH_PADDING};
261261

262262
if (isLoading || !fontMgr) {
263263
const reasonAttributes: SkeletonSpanReasonAttributes = {context: 'BarChartContent', isLoading, isFontLoading: !fontMgr};

src/components/Charts/LineChart/LineChartContent.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,6 @@ function LineChartContent({data, isLoading, yAxisUnit, yAxisUnitPosition = 'left
139139
labelAreaWidth: plotAreaWidth,
140140
firstTickLeftSpace: boundsLeft + domainPadding.left * paddingScale,
141141
lastTickRightSpace: chartWidth > 0 ? chartWidth - boundsRight + domainPadding.right * paddingScale : 0,
142-
allowTightDiagonalPacking: true,
143142
measurements,
144143
});
145144

@@ -224,7 +223,7 @@ function LineChartContent({data, isLoading, yAxisUnit, yAxisUnitPosition = 'left
224223
fontMgr={fontMgr}
225224
labelColor={theme.textSupporting}
226225
xScale={args.xScale}
227-
chartBoundsBottom={args.chartBounds.bottom}
226+
chartBoundsBottom={data.some((p) => p.total < 0) ? args.chartBounds.bottom : args.yScale(0)}
228227
/>
229228
)}
230229
{!!fontMgr && (
@@ -253,7 +252,7 @@ function LineChartContent({data, isLoading, yAxisUnit, yAxisUnitPosition = 'left
253252
fontMgr,
254253
variables.iconSizeExtraSmall,
255254
);
256-
const chartPadding = {...CHART_PADDING, bottom: labelSpace + CHART_PADDING.bottom + variables.iconSizeExtraSmall, left: yAxisLabelWidth + GLYPH_PADDING};
255+
const chartPadding = {...CHART_PADDING, bottom: labelSpace + CHART_PADDING.bottom, left: yAxisLabelWidth + GLYPH_PADDING};
257256

258257
if (isLoading || !fontMgr) {
259258
const reasonAttributes: SkeletonSpanReasonAttributes = {context: 'LineChartContent', isLoading, isFontLoading: !fontMgr};

src/components/Charts/components/ChartXAxisLabels.tsx

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import {AXIS_LABEL_GAP, GLYPH_PADDING, MAX_X_AXIS_LABEL_WIDTH} from '@components
55
import useChartParagraphs from '@components/Charts/hooks/useChartParagraphs';
66
import type {LabelRotation} from '@components/Charts/types';
77
import {getFontLineMetrics, rotatedLabelCenterCorrection, rotatedLabelYOffset, truncateLabel} from '@components/Charts/utils';
8-
import variables from '@styles/variables';
98

109
type ChartXAxisLabelsProps = {
1110
/** Original (non-truncated) label strings from the data. */
@@ -79,8 +78,6 @@ function ChartXAxisLabels({
7978

8079
const paragraphs = useChartParagraphs(truncatedLabels, fontMgr, fontSize, labelColor, MAX_X_AXIS_LABEL_WIDTH);
8180

82-
const renderedWidths = truncatedLabels.map((_, i) => paragraphs?.at(i)?.width ?? 0);
83-
8481
// Derive ascent/descent from the first available paragraph's line metrics.
8582
const {ascent, descent} = getFontLineMetrics(fontMgr, fontSize);
8683

@@ -106,7 +103,7 @@ function ChartXAxisLabels({
106103
key={`x-label-${label}-${tickX}`}
107104
paragraph={paraData.para}
108105
x={tickX - renderWidth / 2}
109-
y={labelY - variables.iconSizeExtraSmall}
106+
y={labelY - ascent}
110107
width={renderWidth + GLYPH_PADDING}
111108
/>
112109
);

src/components/Charts/hooks/useChartLabelLayout.ts

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,6 @@ type LabelLayoutConfig = {
2626
/** Pixels from last tick to right edge of canvas. Defaults to Infinity (no constraint). */
2727
lastTickRightSpace?: number;
2828

29-
/** When true, allows tighter label packing at 45° by accounting for vertical offset between right-aligned labels. */
30-
allowTightDiagonalPacking?: boolean;
31-
3229
/** Measurements of the label text. */
3330
measurements: ReturnType<typeof useChartLabelMeasurements>;
3431
};
@@ -45,16 +42,7 @@ const EMPTY_LAYOUT = {
4542
ellipsisWidth: 0,
4643
};
4744

48-
function useChartLabelLayout({
49-
data,
50-
fontMgr,
51-
tickSpacing,
52-
labelAreaWidth,
53-
firstTickLeftSpace = Infinity,
54-
lastTickRightSpace = Infinity,
55-
allowTightDiagonalPacking = false,
56-
measurements,
57-
}: LabelLayoutConfig) {
45+
function useChartLabelLayout({data, fontMgr, tickSpacing, labelAreaWidth, firstTickLeftSpace = Infinity, lastTickRightSpace = Infinity, measurements}: LabelLayoutConfig) {
5846
// Phase 1: font/data measurements — stable across geometry-only changes (resize).
5947

6048
// Phase 2: layout decisions + label truncation.
@@ -83,18 +71,16 @@ function useChartLabelLayout({
8371
rotation: LABEL_ROTATIONS.HORIZONTAL,
8472
firstTickLeftSpace: effectiveFirstTickLeftSpace,
8573
lastTickRightSpace: effectiveLastTickRightSpace,
86-
rightAligned: false,
8774
});
8875

8976
if (hFitsInTicks && hEdgeFits) {
9077
rotation = LABEL_ROTATIONS.HORIZONTAL;
9178
} else {
92-
const diagonalOverlap = allowTightDiagonalPacking ? lineHeight * SIN_45 : 0;
93-
const minDiagWidth = minTruncatedWidth * SIN_45 - diagonalOverlap;
79+
const minDiagWidth = minTruncatedWidth * SIN_45 - lineHeight * SIN_45;
9480
const dFitsInTicks = minDiagWidth + LABEL_PADDING <= tickSpacing;
9581

96-
const firstEdgeMax = edgeMaxLabelWidth(effectiveFirstTickLeftSpace, lineHeight, LABEL_ROTATIONS.DIAGONAL, allowTightDiagonalPacking, 'first');
97-
const lastEdgeMax = edgeMaxLabelWidth(effectiveLastTickRightSpace, lineHeight, LABEL_ROTATIONS.DIAGONAL, allowTightDiagonalPacking, 'last');
82+
const firstEdgeMax = edgeMaxLabelWidth(effectiveFirstTickLeftSpace, lineHeight, LABEL_ROTATIONS.DIAGONAL, 'first');
83+
const lastEdgeMax = edgeMaxLabelWidth(effectiveLastTickRightSpace, lineHeight, LABEL_ROTATIONS.DIAGONAL, 'last');
9884
const dEdgeFits = firstEdgeMax >= firstMinTrunc && lastEdgeMax >= lastMinTrunc;
9985

10086
if (dFitsInTicks && dEdgeFits) {
@@ -103,17 +89,16 @@ function useChartLabelLayout({
10389
}
10490

10591
// Compute per-label max-width constraints (used by ChartXAxisLabels for truncation).
106-
const truncDiagonalOverlap = allowTightDiagonalPacking ? lineHeight : 0;
107-
const tickMaxWidth = rotation === LABEL_ROTATIONS.DIAGONAL ? (tickSpacing - LABEL_PADDING) / SIN_45 + truncDiagonalOverlap : Infinity;
92+
const tickMaxWidth = rotation === LABEL_ROTATIONS.DIAGONAL ? (tickSpacing - LABEL_PADDING) / SIN_45 + lineHeight : Infinity;
10893

10994
const labelMaxWidths = data.map((_, index) => {
11095
let maxWidth = tickMaxWidth;
11196
if (index === 0) {
112-
const edgeMax = edgeMaxLabelWidth(effectiveFirstTickLeftSpace, lineHeight, rotation, allowTightDiagonalPacking, 'first');
97+
const edgeMax = edgeMaxLabelWidth(effectiveFirstTickLeftSpace, lineHeight, rotation, 'first');
11398
maxWidth = Math.min(maxWidth, edgeMax);
11499
}
115100
if (index === data.length - 1) {
116-
const edgeMax = edgeMaxLabelWidth(effectiveLastTickRightSpace, lineHeight, rotation, allowTightDiagonalPacking, 'last');
101+
const edgeMax = edgeMaxLabelWidth(effectiveLastTickRightSpace, lineHeight, rotation, 'last');
117102
maxWidth = Math.min(maxWidth, edgeMax);
118103
}
119104
return maxWidth;

src/components/Charts/utils.ts

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -332,20 +332,16 @@ function maxVisibleCount(areaWidth: number, itemWidth: number): number {
332332
* How far a label extends beyond its tick position after rotation.
333333
* Accounts for the rotatedLabelCenterCorrection translateX applied during rendering.
334334
*/
335-
function labelOverhang(labelWidth: number, lineHeight: number, rotation: LabelRotation, rightAligned: boolean): {left: number; right: number} {
335+
function labelOverhang(labelWidth: number, lineHeight: number, rotation: LabelRotation): {left: number; right: number} {
336336
if (rotation === LABEL_ROTATIONS.HORIZONTAL) {
337337
return {left: labelWidth / 2, right: labelWidth / 2};
338338
}
339339
if (rotation === LABEL_ROTATIONS.DIAGONAL) {
340340
const halfLH = lineHeight / 2;
341-
if (rightAligned) {
342-
return {
343-
left: (labelWidth + halfLH) * SIN_45,
344-
right: halfLH * SIN_45,
345-
};
346-
}
347-
const overhang = (labelWidth / 2 + halfLH) * SIN_45;
348-
return {left: overhang, right: overhang};
341+
return {
342+
left: (labelWidth + halfLH) * SIN_45,
343+
right: halfLH * SIN_45,
344+
};
349345
}
350346
return {left: lineHeight / 2, right: lineHeight / 2};
351347
}
@@ -358,35 +354,29 @@ function edgeLabelsFit({
358354
rotation,
359355
firstTickLeftSpace,
360356
lastTickRightSpace,
361-
rightAligned,
362357
}: {
363358
firstLabelWidth: number;
364359
lastLabelWidth: number;
365360
lineHeight: number;
366361
rotation: LabelRotation;
367362
firstTickLeftSpace: number;
368363
lastTickRightSpace: number;
369-
rightAligned: boolean;
370364
}): boolean {
371-
const first = labelOverhang(firstLabelWidth, lineHeight, rotation, rightAligned);
372-
const last = labelOverhang(lastLabelWidth, lineHeight, rotation, rightAligned);
365+
const first = labelOverhang(firstLabelWidth, lineHeight, rotation);
366+
const last = labelOverhang(lastLabelWidth, lineHeight, rotation);
373367
return first.left <= firstTickLeftSpace && last.right <= lastTickRightSpace;
374368
}
375369

376370
/**
377371
* Maximum label width that fits within the available edge space at a given rotation.
378372
* Returns Infinity when the overhang at that edge doesn't depend on label width.
379373
*/
380-
function edgeMaxLabelWidth(edgeSpace: number, lineHeight: number, rotation: LabelRotation, rightAligned: boolean, edge: 'first' | 'last'): number {
381-
const halfLH = lineHeight / 2;
374+
function edgeMaxLabelWidth(edgeSpace: number, lineHeight: number, rotation: LabelRotation, edge: 'first' | 'last'): number {
382375
if (rotation === LABEL_ROTATIONS.HORIZONTAL) {
383376
return 2 * edgeSpace;
384377
}
385378
if (rotation === LABEL_ROTATIONS.DIAGONAL) {
386-
if (rightAligned) {
387-
return edge === 'first' ? Math.max(0, edgeSpace / SIN_45 - halfLH) : Infinity;
388-
}
389-
return Math.max(0, 2 * (edgeSpace / SIN_45 - halfLH));
379+
return edge === 'first' ? Math.max(0, edgeSpace / SIN_45 - lineHeight / 2) : Infinity;
390380
}
391381
return Infinity;
392382
}

tests/unit/components/Charts/useChartLabelLayout.test.ts

Lines changed: 8 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -136,19 +136,22 @@ describe('useChartLabelLayout', () => {
136136

137137
describe('edge-constrained rotation', () => {
138138
it('same data picks 0° without edge constraint but 45° with edge constraint', () => {
139-
// "A".repeat(16) = 112px. At 0°: overhang = 56px.
140-
const config = {data: makeData('A'.repeat(16), 'BB', 'CC'), fontMgr: mockFontMgr, fontSize: FONT_SIZE, tickSpacing: 120, labelAreaWidth: 360};
139+
// "A".repeat(22) = 154px. firstMinTrunc = (10+3)*7 = 91px.
140+
// At 0°: centered overhang = 77px. firstTickLeftSpace=72 < 77 → 0° edge fails.
141+
// At 45° right-aligned: edgeMax = 72/SIN_45−8 ≈ 93.8 ≥ 91 → 45° edge fits.
142+
const config = {data: makeData('A'.repeat(22), 'BB', 'CC'), fontMgr: mockFontMgr, fontSize: FONT_SIZE, tickSpacing: 160, labelAreaWidth: 480};
141143

142144
const {result: noEdge} = renderLayout(config);
143145
expect(noEdge.current.labelRotation).toBe(0);
144146

145-
// firstTickLeftSpace=40 < 56 → 0° edge fails → escalates to 45°
146-
const {result: withEdge} = renderLayout({...config, firstTickLeftSpace: 40, lastTickRightSpace: 200});
147+
// firstTickLeftSpace=72 < 77 → 0° edge fails → escalates to 45°
148+
const {result: withEdge} = renderLayout({...config, firstTickLeftSpace: 72, lastTickRightSpace: 200});
147149
expect(withEdge.current.labelRotation).toBe(45);
148150
});
149151

150152
it('escalates to 90° when edge space is too small for both 0° and 45°', () => {
151-
// firstTickLeftSpace=5: at 45° centered edgeMax = max(0, 2*(5/SIN_45-8)) ≈ 0 → fails
153+
// "AAAAAA" = 42px (6 chars <= MIN_TRUNCATED_CHARS=10), so firstMinTrunc = 42.
154+
// firstTickLeftSpace=5: at 45° right-aligned edgeMax = max(0, 5/SIN_45−8) = 0 < 42 → fails
152155
const {result} = renderLayout({
153156
data: makeData('AAAAAA', 'BBBBBB'),
154157
fontMgr: mockFontMgr,
@@ -160,50 +163,9 @@ describe('useChartLabelLayout', () => {
160163
});
161164
expect(result.current.labelRotation).toBe(90);
162165
});
163-
164-
it('allowTightDiagonalPacking enables 45° at tighter tick spacing', () => {
165-
// "AAAAAA" = 42px. tickSpacing=30.
166-
// Without packing: minDiagWidth = 42*SIN_45 ≈ 29.7, 29.7+4=33.7 > 30 → 45° fails
167-
// With packing: diagonalOverlap = 16*SIN_45 ≈ 11.3, minDiagWidth = 29.7-11.3=18.4, 18.4+4=22.4 ≤ 30 ✓
168-
const base = {
169-
data: makeData('AAAAAA', 'BBBBBB'),
170-
fontMgr: mockFontMgr,
171-
fontSize: FONT_SIZE,
172-
tickSpacing: 30,
173-
labelAreaWidth: 400,
174-
firstTickLeftSpace: 100,
175-
lastTickRightSpace: 100,
176-
};
177-
178-
const {result: noPacking} = renderLayout({...base, allowTightDiagonalPacking: false});
179-
expect(noPacking.current.labelRotation).toBe(90);
180-
181-
const {result: withPacking} = renderLayout({...base, allowTightDiagonalPacking: true});
182-
expect(withPacking.current.labelRotation).toBe(45);
183-
});
184166
});
185167

186168
describe('edge-aware max-width constraints', () => {
187-
it('constrains first label below full width when centered and edge is tight', () => {
188-
// First label: 16 chars = 112px. tickMaxWidth ≈ 164. edgeMax ≈ 97 (stricter).
189-
// labelMaxWidths[0] should be < 112; middle/last labels unconstrained.
190-
const {result} = renderLayout({
191-
data: makeData('A'.repeat(16), 'BB', 'CC'),
192-
fontMgr: mockFontMgr,
193-
fontSize: FONT_SIZE,
194-
tickSpacing: 120,
195-
labelAreaWidth: 360,
196-
firstTickLeftSpace: 40,
197-
lastTickRightSpace: 200,
198-
});
199-
expect(result.current.labelRotation).toBe(45);
200-
// Edge constraint tightens first label below its natural width
201-
expect(result.current.labelMaxWidths.at(0)).toBeLessThan(16 * PX_PER_CHAR);
202-
// Middle and last labels are only tick-constrained (much wider than 'BB'/'CC')
203-
expect(result.current.labelMaxWidths.at(1)).toBeGreaterThanOrEqual(2 * PX_PER_CHAR);
204-
expect(result.current.labelMaxWidths.at(2)).toBeGreaterThanOrEqual(2 * PX_PER_CHAR);
205-
});
206-
207169
it('constrains first label below full width when right-aligned and edge is tight', () => {
208170
// Right-aligned first label: edgeMax = 72/SIN_45 - 8 ≈ 93.8 < 112 → constrained.
209171
const {result} = renderLayout({
@@ -214,28 +176,12 @@ describe('useChartLabelLayout', () => {
214176
labelAreaWidth: 360,
215177
firstTickLeftSpace: 72,
216178
lastTickRightSpace: 200,
217-
allowTightDiagonalPacking: true,
218179
});
219180
expect(result.current.labelRotation).toBe(45);
220181
expect(result.current.labelMaxWidths.at(0)).toBeLessThan(16 * PX_PER_CHAR);
221182
expect(result.current.labelMaxWidths.at(1)).toBeGreaterThanOrEqual(2 * PX_PER_CHAR);
222183
});
223184

224-
it('constrains last label below full width when centered and right edge is tight', () => {
225-
// lastTickRightSpace=40: edgeMax = 2*(40/SIN_45-8) ≈ 97.1 < 112 → constrained.
226-
const {result} = renderLayout({
227-
data: makeData('AA', 'BB', 'A'.repeat(16)),
228-
fontMgr: mockFontMgr,
229-
fontSize: FONT_SIZE,
230-
tickSpacing: 200,
231-
labelAreaWidth: 600,
232-
firstTickLeftSpace: 200,
233-
lastTickRightSpace: 40,
234-
});
235-
expect(result.current.labelRotation).toBe(45);
236-
expect(result.current.labelMaxWidths.at(2)).toBeLessThan(16 * PX_PER_CHAR);
237-
});
238-
239185
it('does NOT constrain last label when right-aligned despite tight right edge', () => {
240186
// Right-aligned: last label right overhang = halfLH*SIN_45 ≈ 5.6 (constant, tiny).
241187
// lastTickRightSpace=40 >> 5.6 → edgeMax = Infinity → no edge constraint.
@@ -248,7 +194,6 @@ describe('useChartLabelLayout', () => {
248194
labelAreaWidth: 600,
249195
firstTickLeftSpace: 200,
250196
lastTickRightSpace: 40,
251-
allowTightDiagonalPacking: true,
252197
});
253198
expect(result.current.labelRotation).toBe(45);
254199
expect(result.current.labelMaxWidths.at(2)).toBeGreaterThanOrEqual(16 * PX_PER_CHAR);

0 commit comments

Comments
 (0)