Reserve the axis gutter in a right-to-left container - #118
Conversation
A y scale with `side: 'right'` took its tick-label anchor from the physical side alone. That anchor reaches the DOM as SVG `text-anchor`, which resolves against inline base direction, so an RTL container painted the labels leftward into the plot. The margin pass then read those bounds correctly and reserved nothing on the right, widening the plot until it ran under its own labels. Anchor the far side with `end` once the container reads right to left, and mirror the same relation in the text estimator so a host without a DOM measurer resolves the identical layout.
The layout fix costs 73 B raw in shared code, which puts the difference-mark increment 0.02 kB over its ceiling. Gzip falls on most entries.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe change fixes right-side y-axis gutter reservation in RTL containers. Axis anchors and estimated text bounds now use RTL direction. Regression tests cover layout and text positioning. Bundle-size baselines and one gzip budget are updated. ChangesRTL axis gutter correction
Bundle size records
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This localized change makes right-side RTL axis labels reserve the correct gutter and keeps fallback measurement aligned with browser rendering. No actionable merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant compileSceneLayout
participant createAxes
participant createTickLabelCandidates
participant estimateSceneText
compileSceneLayout->>createAxes: pass RTL typography direction
createAxes->>createTickLabelCandidates: pass rightToLeft
createTickLabelCandidates->>estimateSceneText: create direction-aware label bounds
estimateSceneText-->>compileSceneLayout: return mirrored text origin
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 5 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Fixes #117.
The defect
A y scale with
side: 'right'reserves no gutter when the container isdir="rtl". The axis draws at the right edge, the plot keeps the full widthunderneath it, and the tick labels are painted across the plot with the grid
lines running through them. Measured in the rendered SVGs, both 640px wide,
coordinates relative to each SVG's own left edge:
text-anchorltrstartrtl, beforestartrtl, afterendWhy
createTickLabelCandidatesderives the tick-label anchor from the physicalside alone:
That anchor reaches the DOM as SVG
text-anchor, which resolves against inlinebase direction, so under
dir="rtl"a right-side axis labelledstartpaintsleftward, into the plot rather than away from it.
Everything downstream is then faithful to that. The measurement mirrors with it,
as it should: the same string measures
actualBoundingBoxLeft = -1.2, Right = 35.3underltrand34.5 / -0.4underrtl.includeBoundsMarginsees a box on the plot side of the axis line, so
margin.rightstays at theinset,
chart.widthgrows, the axis slides further right, and the layoutconverges on the overlap.
So the fix belongs at the anchor default rather than in the margin pass: the far
side takes
endonce the container reads right to left.estimateSceneTextneeded the same mirror. It resolvedanchorwith aleft-to-right relation unconditionally, which happened to cancel the first bug
whenever no DOM measurer was present. Left alone, the anchor fix would have made
a host without one disagree with the browser instead.
Scope
Two source lines behind two internal parameters. No public API change, no new
type, no docs change. An author-supplied
axis.ticks.anchorstill landsliterally; only the default mirrors.
Untested and deliberately unchanged: an x axis picks its anchor from the rotate
sign, which is a different relation and may or may not need the same treatment.
Tests
Both fail on
mainand pass here.cartesian-scales.test.tsasserts a right-side axis reserves the same gutterin both directions, through a measurer that mirrors the way the DOM one does.
Without the fix the plot is 28px wider than its labels allow.
guide-layout.test.tsasserts the estimated painted box mirrors underrtl.The scene-level test needs the mirroring measurer specifically: with the plain
estimator the two left-to-right assumptions cancel and nothing fails.
The second commit
The layout fix costs 73 B raw in shared code, which puts the difference-mark
increment 0.02 kB over its ceiling; gzip falls on most other entries. The second
commit relocks that one budget and refreshes the universal baseline. Drop it if
you would rather relock yourself.
Gates run locally:
typecheck,test(1927 passing),docs:check,package:check,bundle:check, and Prettier.Summary by CodeRabbit
Bug Fixes
Chores