Skip to content

Commit a953932

Browse files
committed
fix(engine): use vertical overlap detection for y-axis tick thinning
ticksOverlap() was using label text width to detect overlap on both axes. For the y-axis, labels are stacked vertically, so overlap should be checked against font height, not text width. This caused aggressive thinning down to 2 ticks (min/max) instead of showing ~5 gridlines.
1 parent 2f78d09 commit a953932

2 files changed

Lines changed: 89 additions & 4 deletions

File tree

packages/engine/src/__tests__/axes.test.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,3 +655,71 @@ describe('axis config properties', () => {
655655
expect(axes.x!.labelFlush).toBeUndefined();
656656
});
657657
});
658+
659+
// ---------------------------------------------------------------------------
660+
// Y-axis produces ~5 ticks at standard sizes
661+
// ---------------------------------------------------------------------------
662+
663+
describe('y-axis tick density', () => {
664+
it('produces 5+ y-axis ticks at standard chart size with full density', () => {
665+
const scales = computeScales(lineSpec, chartArea, lineSpec.data);
666+
const axes = computeAxes(scales, chartArea, fullStrategy, theme);
667+
668+
// A 500x300 chart with domain [100, 500] should show ~5+ ticks, not just 2
669+
expect(axes.y!.ticks.length).toBeGreaterThanOrEqual(5);
670+
});
671+
672+
it('y-axis thinning uses vertical overlap, not horizontal text width', () => {
673+
// Even with a measureText that reports very wide labels, y-axis should
674+
// not thin aggressively because overlap is checked vertically
675+
const wideMeasure = () => ({ width: 500, height: 12 });
676+
const scales = computeScales(lineSpec, chartArea, lineSpec.data);
677+
const axes = computeAxes(scales, chartArea, fullStrategy, theme, wideMeasure);
678+
679+
// Wide label text shouldn't cause y-axis thinning (only height matters)
680+
expect(axes.y!.ticks.length).toBeGreaterThanOrEqual(5);
681+
});
682+
});
683+
684+
// ---------------------------------------------------------------------------
685+
// Vertical orientation overlap detection
686+
// ---------------------------------------------------------------------------
687+
688+
describe('ticksOverlap with vertical orientation', () => {
689+
const fontSize = 12;
690+
const fontWeight = 400;
691+
692+
it('returns false when vertical ticks have sufficient spacing', () => {
693+
// Labels at 30px intervals with 12px font (14.4px with lineHeight)
694+
const ticks: AxisTick[] = [
695+
{ value: 0, position: 0, label: '100' },
696+
{ value: 1, position: 30, label: '200' },
697+
{ value: 2, position: 60, label: '300' },
698+
{ value: 3, position: 90, label: '400' },
699+
{ value: 4, position: 120, label: '500' },
700+
];
701+
expect(ticksOverlap(ticks, fontSize, fontWeight, undefined, 'vertical')).toBe(false);
702+
});
703+
704+
it('returns true when vertical ticks are too close', () => {
705+
// Labels at 10px intervals with 12px font - should overlap vertically
706+
const ticks: AxisTick[] = [
707+
{ value: 0, position: 0, label: '100' },
708+
{ value: 1, position: 10, label: '200' },
709+
{ value: 2, position: 20, label: '300' },
710+
];
711+
expect(ticksOverlap(ticks, fontSize, fontWeight, undefined, 'vertical')).toBe(true);
712+
});
713+
714+
it('ignores label text width for vertical orientation', () => {
715+
// Very wide labels but well-spaced vertically - should NOT overlap
716+
const ticks: AxisTick[] = [
717+
{ value: 0, position: 0, label: 'Very Long Label Text Here' },
718+
{ value: 1, position: 40, label: 'Another Very Long Label' },
719+
{ value: 2, position: 80, label: 'Yet Another Long Label' },
720+
];
721+
// Horizontal would detect overlap, vertical should not
722+
expect(ticksOverlap(ticks, fontSize, fontWeight, undefined, 'horizontal')).toBe(true);
723+
expect(ticksOverlap(ticks, fontSize, fontWeight, undefined, 'vertical')).toBe(false);
724+
});
725+
});

packages/engine/src/layout/axes.ts

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,15 +122,31 @@ function measureLabel(
122122
: estimateTextWidth(text, fontSize, fontWeight);
123123
}
124124

125-
/** Check whether any adjacent tick labels overlap horizontally. */
125+
/** Check whether any adjacent tick labels overlap along the axis direction. */
126126
export function ticksOverlap(
127127
ticks: AxisTick[],
128128
fontSize: number,
129129
fontWeight: number,
130130
measureText?: MeasureTextFn,
131+
orientation: 'horizontal' | 'vertical' = 'horizontal',
131132
): boolean {
132133
if (ticks.length < 2) return false;
133134
const minGap = fontSize * MIN_TICK_GAP_FACTOR;
135+
136+
if (orientation === 'vertical') {
137+
// Y-axis: labels are stacked vertically. Check if vertical extent
138+
// (based on font height) overlaps between adjacent ticks.
139+
// Positions decrease going up in SVG coords, so sort ascending.
140+
const sorted = [...ticks].sort((a, b) => a.position - b.position);
141+
const labelHeight = fontSize * 1.2; // lineHeight
142+
for (let i = 0; i < sorted.length - 1; i++) {
143+
const aBottom = sorted[i].position + labelHeight / 2;
144+
const bTop = sorted[i + 1].position - labelHeight / 2;
145+
if (aBottom + minGap > bTop) return true;
146+
}
147+
return false;
148+
}
149+
134150
for (let i = 0; i < ticks.length - 1; i++) {
135151
const aWidth = measureLabel(ticks[i].label, fontSize, fontWeight, measureText);
136152
const bWidth = measureLabel(ticks[i + 1].label, fontSize, fontWeight, measureText);
@@ -151,8 +167,9 @@ export function thinTicksUntilFit(
151167
fontSize: number,
152168
fontWeight: number,
153169
measureText?: MeasureTextFn,
170+
orientation: 'horizontal' | 'vertical' = 'horizontal',
154171
): AxisTick[] {
155-
if (!ticksOverlap(ticks, fontSize, fontWeight, measureText)) return ticks;
172+
if (!ticksOverlap(ticks, fontSize, fontWeight, measureText, orientation)) return ticks;
156173

157174
let current = ticks;
158175
while (current.length > MIN_TICK_COUNT) {
@@ -164,7 +181,7 @@ export function thinTicksUntilFit(
164181
if (current.length > 1) thinned.push(current[current.length - 1]);
165182
current = thinned;
166183

167-
if (!ticksOverlap(current, fontSize, fontWeight, measureText)) break;
184+
if (!ticksOverlap(current, fontSize, fontWeight, measureText, orientation)) break;
168185
}
169186
return current;
170187
}
@@ -455,7 +472,7 @@ export function computeAxes(
455472
// Thin tick labels to prevent overlap (skip for band scales, explicit tickCount, and values).
456473
const shouldThin = scales.y.type !== 'band' && !axisConfig?.tickCount && !axisConfig?.values;
457474
const ticks = shouldThin
458-
? thinTicksUntilFit(allTicks, fontSize, fontWeight, measureText)
475+
? thinTicksUntilFit(allTicks, fontSize, fontWeight, measureText, 'vertical')
459476
: allTicks;
460477

461478
// Gridlines match the tick set so every gridline has a label.

0 commit comments

Comments
 (0)