Improve tooltip and chart chrome customizability - #311
Merged
Conversation
|
Warning Review limit reached
Next review available in: 19 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughTooltip labels with structured DOM slots, expanded chrome contracts for legend and tooltip sub-elements, theme refresh observation of ChangesTooltip labels and structured rendering
Chrome slots and legend rendering
Chrome documentation and navigation
Editable palette playground
Estimated code review effort: 4 (Complex) | ~50 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ChartPlaygroundState
participant chart_playground
participant xy_theme
User->>ChartPlaygroundState: set primary/secondary/accent color
ChartPlaygroundState->>ChartPlaygroundState: mark preset Custom
chart_playground->>xy_theme: apply xy.theme(palette=[...])
xy_theme->>User: render themed chart grid
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Alek99
marked this pull request as ready for review
July 26, 2026 08:25
Merging this PR will not alter performance
Comparing 
Footnotes
|
…-chrome-customizability
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@spec/api/export.md`:
- Around line 168-169: Update the stale source reference in the specification’s
description of the data-theme observer to point to the implementation range at
lines 476-482 instead of 50_chartview.ts:4037. Keep the surrounding explanation
about local browser refresh and observed data-theme/inline-style changes
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 780f4df2-51f6-4cbe-ab29-0efff8bc46dc
⛔ Files ignored due to path filters (2)
spec/assets/tooltip-customizability-after.pngis excluded by!**/*.pngspec/assets/tooltip-customizability-before.pngis excluded by!**/*.png
📒 Files selected for processing (25)
CHANGELOG.mddocs/api-reference/public-types.mddocs/app/tests/test_docs_site.pydocs/app/xy_docs/config.pydocs/app/xy_docs/playground.pydocs/components/legends.mddocs/components/tooltips.mddocs/styling/chrome-slots.mddocs/styling/component-variations.mddocs/styling/customize.mddocs/styling/examples.mddocs/styling/index.mddocs/styling/playground.mdjs/src/20_theme.tsjs/src/50_chartview.tsjs/src/52_tooltip.tspython/xy/components.pypython/xy/dom.pyspec/api/export.mdspec/api/styling.mdspec/design-dossier.mdtests/test_components.pytests/test_static_client_security.pytests/test_type_surface.pytests/test_ui_issue_regressions.py
Alek99
added a commit
that referenced
this pull request
Jul 27, 2026
Closing the `loc` vocabulary caught XY's own documentation: the components/facets-and-layers page passes `loc="top left"`, which is not Matplotlib's spelling, so the substring match found "left" but neither "upper" nor "lower" and the page has been rendering a *center*-left legend. Refusing it would be pedantry — `top` and `bottom` name the same edges as `upper` and `lower`, and they are what CSS, Plotly and that doc page all say. So the vocabulary normalizes what is unambiguous and refuses only what is genuinely a guess: case and whitespace are free, `-`/`_` separate words, either word order works, `right`/`left` alone mean the centered edges, and `top`/`bottom` map to `upper`/`lower`. `northeast`, `outside right` and `middle left` still raise, because there is no honest answer for them. Also here, both surfaced by the same audit: - A legend style written in ordinary kebab-case CSS — the spelling every doc example uses — lost its `box-shadow` and `border-radius` outright, because the writers key on the browser's camelCase property names. Folding the slot into the merged declaration block fixes it; pinned by a test that asserts the two spellings produce identical bytes. - `docs/styling/chrome-slots.md` still published the pre-#311 slot count and the old all-dropped row.
Alek99
added a commit
that referenced
this pull request
Jul 27, 2026
Closing the `loc` vocabulary caught XY's own documentation: the components/facets-and-layers page passes `loc="top left"`, which is not Matplotlib's spelling, so the substring match found "left" but neither "upper" nor "lower" and the page has been rendering a *center*-left legend. Refusing it would be pedantry — `top` and `bottom` name the same edges as `upper` and `lower`, and they are what CSS, Plotly and that doc page all say. So the vocabulary normalizes what is unambiguous and refuses only what is genuinely a guess: case and whitespace are free, `-`/`_` separate words, either word order works, `right`/`left` alone mean the centered edges, and `top`/`bottom` map to `upper`/`lower`. `northeast`, `outside right` and `middle left` still raise, because there is no honest answer for them. Also here, both surfaced by the same audit: - A legend style written in ordinary kebab-case CSS — the spelling every doc example uses — lost its `box-shadow` and `border-radius` outright, because the writers key on the browser's camelCase property names. Folding the slot into the merged declaration block fixes it; pinned by a test that asserts the two spellings produce identical bytes. - `docs/styling/chrome-slots.md` still published the pre-#311 slot count and the old all-dropped row.
This was referenced Jul 27, 2026
Alek99
added a commit
that referenced
this pull request
Jul 27, 2026
* Let chart chrome keep its styling all the way into a file
XY publishes "five ways to style" a chart, and one of them stopped at the
browser. `styles={slot: {...}}` validated, shipped on the wire, and was then
dropped in full by the SVG, PNG and PDF writers — the capability inventory put
it at 2 of 29 slots styleable natively. A chart styled for its live view
exported unstyled, silently, which is the failure mode §28 exists to prevent.
The nine slots that name chrome a static file actually contains — title,
axis_title, tick_label, the three legend slots, the three colorbar slots — now
carry font-size, font-weight, font-style, font-family, letter-spacing, opacity
and the text paint into all three renderers. That is 10 of 29. The rest are
live-only chrome (tooltip, modebar, crosshair, selection, badge) with nothing
in a file to paint, and `class_names` cannot apply in a file at all: a class
selects a rule out of a stylesheet an export does not have.
The plumbing was already there — `spec["dom"]["styles"]` has always shipped —
so this is a read, not a protocol change. Unstyled output stays byte-identical;
a test pins that. The raster's baked atlas is one face, so it honors a slot's
size and paint and leaves the typeface properties to the vector writers, which
is the one divergence and is pinned too. Where two surfaces name the same
chrome the narrower selector wins: an axis's own label_color over the
chart-wide slot.
Three more gaps the same audit turned up, all in the same seam:
- The legend had three spellings that disagreed. `styles={"legend": ...}`,
`xy.legend(style=...)` and `--chart-legend-bg` now merge into one declaration
block before either writer sees it. The theme token is documented and public
and neither static writer read it at all. An explicit `background` also
painted at 8% opacity while the browser painted it opaque; the frame-alpha
token stays the separate knob for the default grey frame.
- `loc="best"` — Matplotlib's default, so the first spelling users try — fell
through the writers' substring match and parked the legend dead center, on
top of the data. It is implemented now, scoring each candidate box by the
fraction of sampled marks inside it. It resolves once at payload-build time,
so the client and both writers receive a settled location and cannot
disagree. The vocabulary is also closed: `"top-left"`, `"northeast"` and
`"LOWER RIGHT"` used to land the legend somewhere arbitrary rather than fail.
- `theme()` accepted any keyword and emitted a CSS declaration no renderer
reads, so `theme(grid_colour=...)` changed nothing and said nothing. Unknown
tokens raise and name the accepted set — and the real `--chart-*` tokens
(tooltip_bg, legend_bg, annotation_text, accent, focus, …) became reachable
by name instead of being dead CSS.
Evidence: spec/assets/chrome-styling-before-after.png.
* Accept the legend-placement spellings that are unambiguous
Closing the `loc` vocabulary caught XY's own documentation: the
components/facets-and-layers page passes `loc="top left"`, which is not
Matplotlib's spelling, so the substring match found "left" but neither "upper"
nor "lower" and the page has been rendering a *center*-left legend. Refusing it
would be pedantry — `top` and `bottom` name the same edges as `upper` and
`lower`, and they are what CSS, Plotly and that doc page all say.
So the vocabulary normalizes what is unambiguous and refuses only what is
genuinely a guess: case and whitespace are free, `-`/`_` separate words, either
word order works, `right`/`left` alone mean the centered edges, and
`top`/`bottom` map to `upper`/`lower`. `northeast`, `outside right` and
`middle left` still raise, because there is no honest answer for them.
Also here, both surfaced by the same audit:
- A legend style written in ordinary kebab-case CSS — the spelling every doc
example uses — lost its `box-shadow` and `border-radius` outright, because the
writers key on the browser's camelCase property names. Folding the slot into
the merged declaration block fixes it; pinned by a test that asserts the two
spellings produce identical bytes.
- `docs/styling/chrome-slots.md` still published the pre-#311 slot count and the
old all-dropped row.
* Score legend placement where the marks are actually drawn
Review of the previous two commits found four real defects, three of them in
code they introduced.
`loc="best"` scored occupancy in *value* space. On a log axis that is the wrong
space: 1..10000 is four evenly spaced decades on screen, but raw subtraction
puts them at 0, .001, .01, .1, 1 — so `best` saw a series crushed against one
edge and guarded a corner the marks do not occupy. Placement now applies the
axis display transform (`log`/`symlog`, matching `_svg._Scale`) before scoring.
Out-of-domain samples were clamped onto the plot edges with `np.clip`. Every
renderer clips those marks away, so clamping invented occupancy in a corner the
viewer sees as empty — a chart with a fixed `domain=` and a tail outside it
would refuse the corner that tail visibly vacated. They are dropped instead, and
a series with nothing visible left is not scored at all.
The raster writer applied the legend slot and the `--chart-legend-bg` token to
the *main* legend only; `_svg` applied them to every legend. So a chart with a
categorical color channel — which adds a second legend — styled correctly in
SVG and PDF and silently skipped that legend in PNG. Both writers now fold the
same declaration block into every legend.
`theme()` grew a `__name` escape hatch that mapped to `--name`, which quietly
reopened the dead-CSS hole the closed vocabulary exists to shut:
`theme(__grid_colour=...)` emitted `--grid-colour` and changed nothing. Removed;
`style=` is the documented, validated way to set a custom property.
Also, without changing pixels:
- `_SLOT_RASTER_PROPS` was dead and overstated. The glyph primitive takes a size
and one RGBA paint, so `opacity` and `letter-spacing` are vector-only along
with the typeface properties. Now `SLOT_RASTER_PROPS`, used by the tests, and
the spec says which subset each writer reads instead of implying all three
honor all eight.
- An unstyled colorbar renders 11px in SVG (inherited from the root) and 10px in
the raster. That predates per-slot support and closing it would change the
pixels of every existing unstyled colorbar, so it is registered as a known
renderer divergence rather than silently left for someone to find.
- The generated matrix's one-line summary said the 10 native-reaching slots got
there "through a channel other than per-slot styles" — true of `root` alone,
and the opposite of what these commits ship for the other nine.
- The `best` sampling policy — 4096-point stride, 512-sample cap, display space,
drop-not-clamp, candidate order, 0.02 tie band — is now normative in the spec
(§28), not left for a reader to infer from the code.
- `xy.legend(style=...)` is canonicalized to the writers' property spelling like
the slot already was. It reaches them through `chrome_styles` today, so this
changes nothing now, but a legend built without that mirror would have lost
its kebab-case declarations.
* Name the colorbar's font size instead of inheriting one
A second review pass found the earlier "known divergence" note was the wrong
call, and two of the tests added with it were vacuous.
The colorbar text size is not a divergence to document, it is an SVG bug. The
`colorbar` slot's stylesheet rule is `font-size:10px` and the raster writer
passes 10 explicitly; the SVG writer emitted no size at all and inherited the
root `<svg>`'s 11px, so it was the odd renderer out on every unstyled colorbar.
It names the size now, from a `COLORBAR_FONT_SIZE` both writers read, and the
divergence entry is removed because the divergence is gone. Every other slot
already agreed — colorbar was the single exception, which is what made it look
like a policy rather than a mistake.
The log and symlog tests did not test anything. Both fixtures were symmetric:
`test_best_scores_a_log_axis_in_display_space` put identical decades on both
axes, so value-space scoring saw the same diagonal and returned `"upper left"`
too, and the symlog case used `[-100, 0, 100]` on a symmetric domain, which is
invariant to the constant because symlog is odd. Both passed with the transform
removed. They now use a scatter whose display-space and value-space occupancy
genuinely disagree (`"lower left"` vs `"upper right"`) and interior asymmetric
symlog values with two explicit constants, and a guard test asserts the fixture
itself still discriminates. Verified by mutation: reverting the transform, the
constant, or drop-not-clamp each fails the tests that cover it.
Two smaller corrections:
- The spec claimed placement "costs O(1) on a large series". It is a fast path,
not a bound — a sample with no finite pair falls back to scanning the full
array, so the worst case is O(n) once at build time. Says so now.
- The generated matrix still told readers the native writers "read the
chart-level `style=` token bag and nothing per-slot", which is what this
branch stopped being true. It now describes the property subset the writers
actually read, and why `class_names=` still reaches neither.
This was referenced Jul 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
xy.tooltip(labels={...})for readable presentation names without changing source-field lookup, formatting keys, title placeholders, or event payloadsfields=and the default x/y/color/size rowstooltip_title,tooltip_row,tooltip_label,tooltip_value,legend_title, andlegend_labelDOM slotsTooltipdataclasses before serializationdata-themeattributeWhy
The existing customization surface was strong at the chart and container level but inconsistent inside browser chrome:
<br>, so labels and values could not be aligned or styled independentlyclassand inlinestyle, missing the commondata-themepatternUser impact
Users can now keep stable data schemas while presenting readable tooltip copy, independently style tooltip and legend typography/layout through the same slot API as other chart chrome, and rely on
data-themeswitches to repaint non-DOM chart surfaces. The docs now expose these capabilities and provide an editable playground rather than workaround-oriented examples.Evidence
The same chart, data, dimensions, hovered point, and viewport were used in both captures.
Before
The tooltip exposed raw source identifiers and each line was one indivisible text fragment.
After
The source fields now have presentation labels, with title, rows, labels, and values independently styleable through stable slots.
Compatibility and safety
Tooltip.labelsis appended after the released dataclass fields, preserving positional construction compatibility.textContent; hostile markup remains inert.Validation
159 passed— component API, public type surface, and static-client security tests31 passed— tooltip/legend interaction, styling, and responsive-layout browser regressions in headless Chrome92 passed, 1 xfailed— complete docs-app suitenode js/build.mjs— TypeScript check and both browser bundlesgit diff --checkThe repository-wide quick gate was also attempted in the restricted workspace: 2,361 tests passed. Its remaining failures were blocked Chrome/loopback-socket launches plus the existing asset assertion that searches for unminified function spellings in the intentionally minified generated bundle; the affected focused browser suites were rerun outside the sandbox and passed.
Follow-up opportunities from the audit
axis(format=...)and host-computed CSS themesSummary by CodeRabbit
xy.tooltip(labels=...)to customize tooltip field text while keeping the same field lookup and formatting behavior, with label content treated as text (not HTML).data-theme.