Harden Tailwind customization across Reflex charts - #328
Conversation
📝 WalkthroughWalkthroughThe change adds build-time Tailwind class inventories for Reflex charts, updates chart chrome cascade and selection-slot behavior, restores durable view state across live chrome rebuilds, and documents and tests the resulting styling and interaction contracts. ChangesTailwind styling and live chart integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ReflexChart
participant TailwindScanner
participant XYChart
participant ChartView
participant SelectionMask
ReflexChart->>TailwindScanner: provide normalized class inventory
TailwindScanner->>XYChart: emit scan-only tailwind_class_tokens
XYChart->>ChartView: rebuild mounted chrome when constructor inputs change
ChartView->>ChartView: restore ranges and selection geometry silently
ChartView->>SelectionMask: request selection mask
SelectionMask-->>XYChart: return suppressed selection response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
tests/test_view_state_client.py (1)
634-640: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTautological probe assertion in both new tests.
assert result == {key: True for key in result}derives its expectation fromresult, so an empty or key-dropped probe payload passes silently.
tests/test_view_state_client.py#L634-L640: assert the explicit nine-key expectation for the Tailwind lasso slot probe.tests/test_view_state_client.py#L978-L984: assert the explicit nine-key expectation for the republish durable-state probe.Keep as-is if this matches the existing convention elsewhere in the suite.
🤖 Prompt for 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. In `@tests/test_view_state_client.py` around lines 634 - 640, Replace the tautological assertions in tests/test_view_state_client.py at lines 634-640 and 978-984 with explicit expectations containing all nine required probe keys, each mapped to True. Update both the Tailwind lasso slot probe and republish durable-state probe assertions; do not derive expected keys from result.python/reflex-xy/reflex_xy/component.py (1)
209-214: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: extract the merge-and-inject pair.
Lines 209-214 and 312-317 are the same three-step sequence (discover, merge, inject). A tiny helper (e.g.
_apply_tailwind_tokens(props, figure, tailwind_manifest)) would keep the two call sites from drifting.🤖 Prompt for 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. In `@python/reflex-xy/reflex_xy/component.py` around lines 209 - 214, Extract the duplicated Tailwind token discovery, merge, and injection sequence into a helper such as _apply_tailwind_tokens, preserving the existing conditional injection behavior. Replace both sequences around the visible class_manifest logic and the corresponding second call site with this helper so they remain consistent.tests/test_tailwind_root_customization.py (1)
144-204: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffThese two tests assert on JSX source text, not behavior.
Exact-substring checks like
"const chromeChanged = Boolean(view && !sameMountedChromeSpec(view.spec, spec));"break on a formatter run or a local rename while the contract is unchanged. SinceXYChart.jsxis a non-importable asset this is a reasonable stopgap, but consider narrowing to the identifiers that actually carry the contract (sameMountedChromeSpec,updatePayload,suppress_event) rather than whole formatted statements.🤖 Prompt for 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. In `@tests/test_tailwind_root_customization.py` around lines 144 - 204, Refactor the source-text assertions in test_live_wrapper_rebuilds_constructor_owned_chrome_only_when_needed and test_live_wrapper_silently_hydrates_durable_selection_and_all_axis_ranges to check stable contract identifiers such as sameMountedChromeSpec, updatePayload, and suppress_event instead of complete formatted statements. Preserve the existing ordering and semantic checks while making assertions resilient to formatting changes and local variable renames.
🤖 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 `@js/src/20_theme.ts`:
- Line 115: Update the custom-tooltip CSS rule for
[data-xy-slot="tooltip"][data-xy-custom-tooltip] to also remove the default
box-shadow, while preserving its existing transparent background, border, and
padding overrides.
In `@js/src/53_interaction.ts`:
- Around line 868-875: Update the selection restoration branches in
js/src/53_interaction.ts at lines 868-875 and 892-903: when dispatch is
suppressed, invoke _selectLocal(x0, x1, y0, y1) or _selectLocalPolygon(polygon)
respectively; only send through this.comm when dispatch is enabled, while
preserving the existing dispatch-enabled behavior.
In `@tests/test_tailwind_root_customization.py`:
- Around line 170-177: Update the assertion around the axisLayoutSpec and
mountedChromeSpec slice to first assert that the axisLayoutSpec declaration
index is before the mountedChromeSpec declaration index, then perform the
existing range-exclusion check. Keep the current substring assertion unchanged
for the valid ordering case.
---
Nitpick comments:
In `@python/reflex-xy/reflex_xy/component.py`:
- Around line 209-214: Extract the duplicated Tailwind token discovery, merge,
and injection sequence into a helper such as _apply_tailwind_tokens, preserving
the existing conditional injection behavior. Replace both sequences around the
visible class_manifest logic and the corresponding second call site with this
helper so they remain consistent.
In `@tests/test_tailwind_root_customization.py`:
- Around line 144-204: Refactor the source-text assertions in
test_live_wrapper_rebuilds_constructor_owned_chrome_only_when_needed and
test_live_wrapper_silently_hydrates_durable_selection_and_all_axis_ranges to
check stable contract identifiers such as sameMountedChromeSpec, updatePayload,
and suppress_event instead of complete formatted statements. Preserve the
existing ordering and semantic checks while making assertions resilient to
formatting changes and local variable renames.
In `@tests/test_view_state_client.py`:
- Around line 634-640: Replace the tautological assertions in
tests/test_view_state_client.py at lines 634-640 and 978-984 with explicit
expectations containing all nine required probe keys, each mapped to True.
Update both the Tailwind lasso slot probe and republish durable-state probe
assertions; do not derive expected keys from result.
🪄 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: e07c20ff-2f4e-4c65-a59c-7b8cddb13f7c
⛔ Files ignored due to path filters (3)
spec/assets/tailwind-cascade-before-after.jpgis excluded by!**/*.jpgspec/assets/tailwind-live-after.jpgis excluded by!**/*.jpgspec/assets/tailwind-live-before.jpgis excluded by!**/*.jpg
📒 Files selected for processing (30)
docs/app/rxconfig.pydocs/app/tests/test_docs_site.pydocs/styling/chrome-slots.mddocs/styling/index.mddocs/styling/themes-and-tokens.mdjs/src/20_theme.tsjs/src/50_chartview.tsjs/src/53_interaction.tsjs/src/54_kernel.tsjs/src/57_viewstate.tspython/reflex-xy/CHANGELOG.mdpython/reflex-xy/README.mdpython/reflex-xy/reflex_xy/assets/XYChart.jsxpython/reflex-xy/reflex_xy/component.pypython/xy/_figure.pypython/xy/components.pyspec/README.mdspec/api/chart-roadmap.mdspec/api/export.mdspec/api/styling.mdspec/design/reflex-integration.mdspec/design/reflex-shaped-api.mdspec/process/tailwind-customizability-audit-2026-07-26.mdtests/reflex_adapter/test_component.pytests/test_components.pytests/test_export_style_survival.pytests/test_legend_highlight.pytests/test_static_client_security.pytests/test_tailwind_root_customization.pytests/test_view_state_client.py
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 `@js/src/53_interaction.ts`:
- Around line 958-962: Update the local restoration logic around _applySelMask
so traces excluded by the opts.localMask condition are explicitly cleared before
continuing. Ensure non-scatter and _legendHidden traces cannot retain stale
selection masks, while preserving the existing restoration behavior for eligible
visible scatter traces.
🪄 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: cb8dbfe6-168c-4593-956d-151ebd93bc2d
📒 Files selected for processing (12)
js/src/20_theme.tsjs/src/50_chartview.tsjs/src/53_interaction.tspython/reflex-xy/reflex_xy/assets/XYChart.jsxpython/xy/_figure.pypython/xy/components.pyspec/api/styling.mdtests/test_components.pytests/test_static_client_security.pytests/test_tailwind_root_customization.pytests/test_text_weight_defaults.pytests/test_view_state_client.py
🚧 Files skipped from review as they are similar to previous changes (9)
- python/xy/_figure.py
- tests/test_static_client_security.py
- python/xy/components.py
- tests/test_tailwind_root_customization.py
- spec/api/styling.md
- tests/test_view_state_client.py
- tests/test_components.py
- python/reflex-xy/reflex_xy/assets/XYChart.jsx
- js/src/50_chartview.ts
| // Restoration mirrors the kernel selection universe exactly. Otherwise | ||
| // a provisional line/hidden-series mask would survive forever because | ||
| // the authoritative reply only contains visible scatter trace ids. | ||
| if (opts.localMask === true | ||
| && (g.trace.kind !== "scatter" || g._legendHidden)) continue; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C 5 '_restoreSelectionLocalMask|_applySelMask|selection.*mask|mask.*selection' js/src testsRepository: reflex-dev/xy
Length of output: 15455
Clear excluded-trace masks during local restoration.
These continue branches skip _applySelMask, so a non-scatter or hidden trace that already has a mask can stay highlighted. Clear excluded masks here, or reset every trace before this preview path.
🤖 Prompt for 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.
In `@js/src/53_interaction.ts` around lines 958 - 962, Update the local
restoration logic around _applySelMask so traces excluded by the opts.localMask
condition are explicitly cleared before continuing. Ensure non-scatter and
_legendHidden traces cannot retain stale selection masks, while preserving the
existing restoration behavior for eligible visible scatter traces.
Only conflict was a comment in tests/test_export_style_survival.py that #328 updated from 23 to 29 slots. Kept this branch's wording, which reads the count from CHART_DOM_SLOTS instead of writing it down, so the next slot addition cannot leave it stale again.
Summary
reflex_xy.chart(..., tailwind_classes=...)a verbatim compile-time inventory for live token/Varcharts, including quoted content, raw backslashes, Unicode, arbitrary values/properties, and escaped selectorsselectionslot to lasso paths/handles while preserving structural pointer behaviorUser-facing API
tailwind_classesaccepts a complete string or ordered iterable, rejects mappings/unordered sets, de-duplicates candidates, merges static discovery, and applies to every facet panel. It is compile-only and never becomes a DOM attribute. State-driven charts must inventory every complete class they may emit.Production Tailwind matrix
Verified in a real
reflex run --env prod --single-portbuild:md:, arbitrarymax-*,@container, and arbitrary@min-*dark:,hover:,focus-visible:,active:,not-*,has-*, andsupports-*Measured results include matching static/live styles, no scan-prop leak, selector dark mode, a 31px→23px arbitrary container-query title change at 780px→480px, and renderer-default plus Tailwind-override coverage for solid, gradient-scatter, and line legend handles.
Boundaries
Tailwind owns durable visual appearance on DOM slots. XY still owns structural geometry and conditional interaction state. CSS can style
canvas/chromeas whole elements but cannot select individual WebGL or canvas-painted pixels. Full constructor-chrome replacement preserves durable viewport/selection state; transient view-local state such as the selected drag tool, history, legend toggles, and moved modebar position resets with the replaced view.The detailed audit is in
spec/process/tailwind-customizability-audit-2026-07-26.md.Before / after
Before: live classes reached DOM, but Tailwind emitted no matching rules.
After: the same production app emits and applies the requested utilities.
The second comparison holds utilities constant and isolates former inline-cascade blockers versus the fixed client:
Validation
uv run --with pre-commit pre-commit run --all-filesuv run ruff check .uv run ruff format --check .node js/build.mjsnpm run typecheckSummary by CodeRabbit
tailwind_classessupport for live and static charts (including facet panels) with ordered class inventory.