Skip to content

Add hard viewport bounds that clamp pan and zoom#108

Merged
Alek99 merged 4 commits into
mainfrom
codex/add-hard-viewport-bounds-for-pan-and-zoom
Jul 20, 2026
Merged

Add hard viewport bounds that clamp pan and zoom#108
Alek99 merged 4 commits into
mainfrom
codex/add-hard-viewport-bounds-for-pan-and-zoom

Conversation

@FarhanAliRaza

@FarhanAliRaza FarhanAliRaza commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Motivation

  • Provide a way to keep interactive navigation (pan/zoom) inside meaningful data extents separate from the initial domain so users cannot pan/zoom the camera completely away from the chart.
  • Keep exploratory interaction available while preventing empty/partially-visible charts and preserve existing semantics for reversed and log axes.

Description

  • Add a bounds field to Axis and expose bounds=(lo, hi) or bounds="data" on xy.x_axis() and xy.y_axis() so navigation limits are declaratively configurable.
  • Validate bounds (including log-axis positivity) on the Python side and serialize spec["bounds"] separately from domain, resolving "data" to the data range at payload build time.
  • Clamp the initial viewport, drag panning, wheel/box/button zooms, animated view transitions, and linked-view updates on the client by adding _clampAxisRange and _clampView helpers and integrating them where views are set/updated; clamping works in scale coordinates to preserve reversed and log semantics.
  • Add tests/test_viewport_bounds.py covering payload serialization, "data" resolution, validation, and presence of client clamping hooks; regenerate the committed browser bundles so the shipped static client contains the clamp logic.

Testing

  • Ran unit tests: PYTHONPATH=python python -m pytest tests/test_viewport_bounds.py tests/test_type_surface.py tests/test_figure.py -q and PYTHONPATH=python python -m pytest tests/test_viewport_bounds.py -q, both succeeded (final run: 166 passed, 1 skipped; viewport-bounds tests passed).
  • Ran formatting and lint checks: ruff format --check / ruff check on the modified files and fixed import ordering; all checks passed.
  • Rebuilt the static client: node js/build.mjs --check and verified the generated bundles; check passed.
  • Ran git diff --check to ensure no whitespace/check issues; no problems found.

Closes #82

@codspeed-hq

codspeed-hq Bot commented Jul 20, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 94 untouched benchmarks
⏩ 1 skipped benchmark1


Comparing codex/add-hard-viewport-bounds-for-pan-and-zoom (1580601) with main (ad979e8)

Open in CodSpeed

Footnotes

  1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports.

@FarhanAliRaza FarhanAliRaza added valid Issue is validated. This pr actually fixes the issue. Needs Code Review. and removed codex labels Jul 20, 2026

@Alek99 Alek99 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review findings:

  • [P1] js/src/50_chartview.js:216: the initial viewport is clamped only in the browser, after density/decimation payloads and static-export ranges have been built. For domain=(0, 1_000_000), bounds=(0, 1), a 0..1 scatter is still binned/exported over 0..1_000_000. Clamp before payload generation, or reject incompatible domain/bounds.
  • [P2] python/xy/_figure.py:1003: bounds changes renderer semantics but ships under protocol 3, so older clients silently ignore it and allow unrestricted navigation. Bump the protocol and regenerate bundles.

@Alek99
Alek99 merged commit 3c5054f into main Jul 20, 2026
25 checks passed
Alek99 added a commit that referenced this pull request Jul 21, 2026
The move in this PR promoted docs/engineering/ to spec/ and CLAUDE.md
declares that tree the source of truth. An audit of all 24 documents
against the code found the tree had drifted at PR #96: five behavior
PRs (#103, #105, #106, #108, #115) had landed with no spec update, and
~100 claims contradicted shipped code.

Accuracy: correct claims that no longer match the implementation,
including the Rust ABI version (v31 -> v36), the "zero-crate cdylib"
claim (png is a real dependency), the pyramid ABI function names, the
panic-shield status, the R2/VAO claim in the renderer doc, the
WASM/Web-Worker architecture in dossier §3/§8 (superseded by §32's
C-ABI cdylib), the count-only tier rule, the matplotlib shim overhead
figures (the cited guardrail says +60% at 10k, not +9%), and the
Plotly Scattergl 10M figure that appeared as two different numbers
from the same run.

Coverage: add spec/api/export.md, spec/api/interaction.md and
spec/design/wire-protocol.md for three surfaces that had no coverage
at all, plus the config.py threshold table, full JS and Rust module
inventories, and the hexbin centers-only wire contract that three
renderers must expand identically.

Structure: group 15 top-level files into api/, design/, matplotlib/,
benchmarks/ and process/, and rewrite spec/README.md as a real index
(it previously listed only the two SVG assets, leaving 21 of 22 docs
unreachable). All 150 consumer references repointed, including the CI
--emit-md target and the compat-matrix generator.

Guard: verify_sdist.py pinned only 5 top-level spec files, so the
sdist could silently lose spec/design/ or spec/assets/ and still
pass; it now asserts markdown under every subdirectory and both SVGs.
Alek99 added a commit that referenced this pull request Jul 21, 2026
…roup the tree (#122)

* Move engineering docs to root spec directory

* Resync spec/ with the implementation and group the tree

The move in this PR promoted docs/engineering/ to spec/ and CLAUDE.md
declares that tree the source of truth. An audit of all 24 documents
against the code found the tree had drifted at PR #96: five behavior
PRs (#103, #105, #106, #108, #115) had landed with no spec update, and
~100 claims contradicted shipped code.

Accuracy: correct claims that no longer match the implementation,
including the Rust ABI version (v31 -> v36), the "zero-crate cdylib"
claim (png is a real dependency), the pyramid ABI function names, the
panic-shield status, the R2/VAO claim in the renderer doc, the
WASM/Web-Worker architecture in dossier §3/§8 (superseded by §32's
C-ABI cdylib), the count-only tier rule, the matplotlib shim overhead
figures (the cited guardrail says +60% at 10k, not +9%), and the
Plotly Scattergl 10M figure that appeared as two different numbers
from the same run.

Coverage: add spec/api/export.md, spec/api/interaction.md and
spec/design/wire-protocol.md for three surfaces that had no coverage
at all, plus the config.py threshold table, full JS and Rust module
inventories, and the hexbin centers-only wire contract that three
renderers must expand identically.

Structure: group 15 top-level files into api/, design/, matplotlib/,
benchmarks/ and process/, and rewrite spec/README.md as a real index
(it previously listed only the two SVG assets, leaving 21 of 22 docs
unreachable). All 150 consumer references repointed, including the CI
--emit-md target and the compat-matrix generator.

Guard: verify_sdist.py pinned only 5 top-level spec files, so the
sdist could silently lose spec/design/ or spec/assets/ and still
pass; it now asserts markdown under every subdirectory and both SVGs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

valid Issue is validated. This pr actually fixes the issue. Needs Code Review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add hard viewport bounds that clamp pan and zoom

2 participants