Unified view-state layer: spec + implementation (history, programmatic control, axis gestures, hover payload)#129
Conversation
One serializable view-state object with a single clamped mutation path, specified as the shared foundation for view history (#110), custom tooltip renderers (#111), axis-scoped gestures (#120), and out-of-band programmatic zoom/pan/select via reflex_xy (#121). Builds on the per-axis ranges and semantic view events introduced in PR #117.
Merging this PR will not alter performance
Comparing Footnotes
|
Alek99
left a comment
There was a problem hiding this comment.
The overall direction is strong, but these four contract gaps should be resolved before implementation so the durability and multi-client invariants remain achievable.
Greptile SummaryThis PR adds a unified view-state layer across the browser client, notebook widget, static charts, and Reflex adapter. The main changes are:
Confidence Score: 5/5This looks safe to merge. The legacy hover callback still receives the bare row value. The structured hover payload uses a separate callback surface. Pointer exit cleanup does not change either callback shape. No blocking issues were found in the changed code.
What T-Rex did
Important Files Changed
Reviews (8): Last reviewed commit: "Fix stranded hover state when the canvas..." | Re-trigger Greptile |
…gaps - Rows-selections are non-durable and non-undoable: excluded from the state schema, the history stack, and round-trip guarantees; view_state() reports an opaque marker instead of index sets. - Remove drag_action from the durable document: the modebar tool is UI mode the kernel cannot observe, not view state. - Drop Python-side view_back()/view_forward() and the back/forward wire ops: history stacks are client-local, so broadcast navigation cannot converge; Back/Forward live only in the modebar and the static JS handle, and view_nav carries only reset. - Key cursor.data by exact axis ID and add per-point x_axis/y_axis bindings so tooltips are unambiguous with secondary axes. - Ship the hover payload on a new on_hover prop; on_point_hover(row) keeps its bare-row contract (nesting row was not compatibility). - Record that set_view's figure-lock hold is bounded, mirroring append's handler-safety contract.
One canonical durable-state document (v:1 ranges + geometric selection) behind a single apply-a-state-patch implementation in the client (js/src/57_viewstate.js), per spec/design/view-state.md: - #121 programmatic control: state_patch / view_nav / selection_rows kernel messages; Chart/FigureWidget set_view, reset_view, select, clear_selection, view_state (event-fed kernel cache, no round-trip); reflex_xy.set_view et al. pushed room-wide like .append. - #110 history: client-local 64-snapshot stack coalesced per gesture by interaction_id, modebar Back/Forward, reset-as-navigation, interaction_config(history=...); root.xy handle exposes applyState/state/back/forward on every mount. - #120 axis-band gestures: per-axis bands with resize cursors; wheel, drag-along (pan), and drag-across (span box-zoom) scoped to the hovered axis through the existing clamp pipeline. - #111 structured hover: additive cursor/points payload on xy:hover (cursor.data keyed by exact axis ID), new reflex on_hover prop, and the xy.tooltip(render=...) mount positioned by the client. End-phase view events now always ship to feed view_state(); update phases stay listener-gated. Rows-selections are non-durable by design. Chart's stored select switch moves to a private name so the kwarg no longer shadows the new Chart.select() method. Specs updated as shipped authorities (interaction.md, wire-protocol.md; view-state.md flipped to implemented with recorded divergences). Tests: 21 kernel-side, 9 headless-Chromium probes, 6 reflex push tests.
Alek99
left a comment
There was a problem hiding this comment.
The overall direction is strong, but these four implementation gaps should be fixed before merge: the custom tooltip renderer is not fed hover data, rows selections retain stale masks on omitted traces, retargeted animations corrupt history, and row indices are encoded without validation.
…index validation
- XYChart.jsx: clone element children with the live §7.1 hover payload
({active, cursor, points, exact}) driven by hover state, so
xy.tooltip(render=...) renders live content client-side instead of a
frozen slot; DOM-tag children stay uncloned so no non-DOM attributes
leak, and the state churn is skipped when no children are mounted.
- 57_viewstate.js: a selection_rows document replaces the whole
selection — deactivate every existing mask before applying, so traces
omitted from the document clear instead of keeping stale highlights
while total reports only the new rows.
- 53_interaction.js: a write landing mid-animation coalesces into the
in-flight navigation — no intermediate history snapshot — and the
settle event carries the latest writer's source/phase/interaction id
by storing them on the animation instead of the first call's closure.
- _figure.py: validate canonical row indices (bounds, integrality,
dtype) before the uint32 encoding, so -1 can no longer ship as
4294967295; deduplicate and report total from validated unique rows.
Specs updated (view-state.md §4/§5.1, wire-protocol.md selection_rows).
Tests: index-validation cases, two headless-Chromium probes (mask
clearing across two traces, retarget coalescing + event metadata), and
a JSX contract test for the payload injection.
Conflict in spec/api/interaction.md was additive on both sides: main's zoom-limit boundaries (#101) and this branch's axis-band scoping paragraph plus the Back/Forward modebar buttons. Kept both and refreshed the _zoomAxisRange line citation, which the merge shifted.
A pan-only axis band (in pan_axes but not zoom_axes) showed the same resize arrow as a zoomable one, promising a zoom the policy ignores. The band cursor now reflects the axis's actual capability: ew/ns-resize only when the axis can zoom, a grab hand when it can only pan, and grabbing while a pan drag is active (restored on release). Specs updated (view-state.md §6, interaction.md gesture table); the axis-band policy probe now asserts all three cursor states, with the no-band-for-excluded-axis case split into its own probe.
…view-state Both sides reworked the selection path; the resolution unions them: - _sendSelect keeps this branch's durable-selection history push AND main's data-space _lastBrush retention for drill-swap mask survival. - The kernel 'selection' handler keeps the shared _applySelectionBuffers helper (selection_rows reuses it) with main's enriched-reply brush adoption and empty-clear layered in front. - channel.py select_polygon records the view-state cache entry AND ships main's include_rows/kind/seq enrichment (box path auto-merged with both). - XYChart.jsx onPayload composes both: main's republish view/selection restore plus this branch's hover flag and tooltip-slot lifecycle; the live spec is now withHoverFlag(eventSpec(...)), and the asset contract test asserts the composed form. - reflex_xy __all__ carries both API sets (set_view et al + resolve_selection). - New seam fix: _applyRowsSelection clears _lastBrush — a rows document replaces the selection, so the retained box/lasso geometry must not re-derive the old mask over it on the next drill swap. Bundles regenerated; full suite (2199), both browser smokes, ruff, and pre-commit pass on the merged tree.
Retest regression on /hover: hover a point, move into the page header, and the custom tooltip could remain visible with the on_hover payload stuck at active: true. Canvas pointerleave was the ONLY thing clearing pointer hover, and browsers skip boundary events when the element under a stationary cursor changes (page scroll, overlay/hit-test churn) — the readout then never self-heals while the pointer roams the page. Factor the pointerleave cleanup into _pointerHoverExit and add a document-level backstop: while a pointer-owned readout is live, any pointerover whose target left the chart root runs the same exit path (tooltip hidden, state cleared, xy:leave with active: false). Keyboard readouts are exempt — keyboard traversal also stores an XY for exact replies, but its readout must survive mouse movement elsewhere; Escape owns dismissal. Document listeners ride the existing _listen registry, so destroy() still removes them. New headless probe covers the full contract: pointer hover set, inside pointerover kept, outside pointerover cleared + leave dispatched, keyboard readout kept. Verified live against the stranded state in the PR test lab. Specs updated (interaction.md §3 xy:leave, view-state.md §7.1 active-flag guarantee).
Summary
Adds
spec/design/view-state.md— the design for one canonical, serializable view-state object behind a single clamped mutation path — and implements all four of its rollout phases in the same PR. The spec's status is flipped to implemented;spec/api/interaction.mdandspec/design/wire-protocol.mdare updated as the shipped authorities, and view-state.md §12 records the implementation divergences.Four open requests land as derivations of one primitive:
chart.set_view / reset_view / select / clear_selection / view_state(notebook widget), andreflex_xy.set_view(token, …)et al. mirroring the.appendpath — figure lock → one wire message → room-wide apply, no re-render, no payload reship.interaction_id; modebar Back/Forward buttons; reset is navigation, so Back undoes a double-click.interaction_config(history=…)switches it.pan_axes/zoom_axes/zoom_limits/containment/bounds govern unchanged.xy:hovergains an additive payload (cursor.px,cursor.datakeyed by exact axis ID, resolvedpointswith axis bindings and series color);reflex_xygrows a newon_hoverprop (legacyon_point_hover(row)untouched);xy.tooltip(render=…)finally mounts — the client positions the component with the built-in placement logic, static charts included.Implementation spine
_applyStatePatchinjs/src/57_viewstate.jsis the single "apply a state patch" implementation;state_patch/view_nav/selection_rowskernel messages, history restores, the staticroot.xyhandle, and gestures all commit through the same validate → clamp → commit → emit path. Programmatic writes emitsource: "api", history emitssource: "history", so state bridges break echo loops by filtering.root.xy.applyState/state/back/forward) work in kernel-less standalone HTML. Kernel-sideview_state()is an event-fed cache: end-phase view events now always ship (one rAF-coalesced message per gesture); update-phase streams stay gated on listener presence.{"rows": true}marker.Validation (the §9 testing contract)
tests/test_view_state.py— 21 kernel-side tests: §2 boundary rules, patch semantics,view_nav/selection_rowsshapes, theview_state()cache lifecycle, brush-before-select ordering for programmatic geometric selects, the history switch, and theChart.selectattribute/method collision (caught byty, fixed by privatizing the stored switch).tests/test_view_state_client.py— 9 headless-Chromium probes driving the real client: exact-f64 round-trip, clamp equivalence (programmatic vs gesture), history contract (N gestures → N entries, wheel coalescing, back/forward exactness, reset-then-back, linked pushes nothing, per-call opt-out, capacity 64), rows non-durability, axis-band scoping (wheel + drag mutate only the hovered axis; excluded axes get no band), additive hover payload beside the compatible legacy row, kernel-message application withsource: "api", and the custom tooltip mount.tests/reflex_adapter/test_view_state_push.py— 6 tests for the §5.2 push path (message shapes, no version bump, caller-thread validation, unwired behavior, module-level wrappers).test_socket_data_planefailures reproduce on a pristine checkout in that ad-hoc venv — environmental, untouched code paths).scripts/render_smoke_nonumpy.pyandscripts/reflex_lifecycle_smoke.py— clean (the lifecycle smoke exercises the restructuredXYChart.jsxmount).uv run ruff check ./ruff format --check ./pre-commit run --all-files— clean;ty checkclean on touched files.Closes #110, closes #111, closes #120, closes #121.