Skip to content

fix(svelteplot): auto margins immune to ancestor CSS transform scale#571

Open
ljodea wants to merge 1 commit into
mainfrom
fix/issue-179-css-transform-margins
Open

fix(svelteplot): auto margins immune to ancestor CSS transform scale#571
ljodea wants to merge 1 commit into
mainfrom
fix/issue-179-css-transform-margins

Conversation

@ljodea

@ljodea ljodea commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes #179 by measuring axis tick labels in SVG user space instead of viewport pixels from getBoundingClientRect().

When a parent applies transform: scale(), inflated margin measurements could feed a reactive loop between X/Y axis $effects until Svelte throws effect_update_depth_exceeded. This PR replaces the viewport measurement with measureSvgTextLayoutExtent(), which:

  • reads getBBox() on tick <text> elements (aggregating <tspan> children when needed)
  • applies only the text element's local SVG transform for rotated X ticks
  • returns 0 on detached/hidden elements instead of propagating bad geometry

Test plan

  • Unit tests (measureSvgTextLayoutExtent.test.ts) — T1–T4: unscaled vs inflated gbcr, rotation, error handling
  • jsdom integration test — mocked 1.3× gbcr proves margins use bbox (~21) not inflated (~25)
  • Browser regression (Vitest + Playwright) — B1 no crash, B4 scale-invariance ±1px, B3 rotated ticks
  • Full jsdom suite: 840 tests pass
  • Browser suite: 4 tests pass

CI

Adds pnpm --filter svelteplot test:browser to .github/workflows/test.yml with Playwright chromium install.

Codex review

Independent diff review: PASS — no [P1] or [P2] findings.

…S scale

Replace getBoundingClientRect() in axis auto-margin effects with
measureSvgTextLayoutExtent(), which reads getBBox() and applies the
tick text element's local SVG transform. This keeps margins in plot
layout units when ancestor elements apply transform: scale().

Adds jsdom integration tests with mocked geometry inflation, browser
regression tests (Vitest + Playwright) for scale-invariance, and CI
wiring for test:browser.

Fixes #179
@netlify

netlify Bot commented Jun 26, 2026

Copy link
Copy Markdown

Deploy Preview for svelteplot ready!

Name Link
🔨 Latest commit 7dc8bfb
🔍 Latest deploy log https://app.netlify.com/projects/svelteplot/deploys/6a3df9fe7c720600085d43d9
😎 Deploy Preview https://deploy-preview-571--svelteplot.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Infinite reactive loop with CSS transforms at scale >> 1

1 participant