Skip to content

fix(engine): compile leaf layers in the primary layout's chart area#89

Merged
rileyhilliard merged 1 commit into
mainfrom
fix/layer-leaf-chart-area
Jul 4, 2026
Merged

fix(engine): compile leaf layers in the primary layout's chart area#89
rileyhilliard merged 1 commit into
mainfrom
fix/layer-leaf-chart-area

Conversation

@rileyhilliard

@rileyhilliard rileyhilliard commented Jul 4, 2026

Copy link
Copy Markdown
Member

Problem

compileLayer compiled each leaf spec raw, without the layer-level chrome, legend, and theme that buildPrimarySpec merges into the primary spec. Each leaf computed its own chart area (no title/subtitle reservation, default legend position, default theme fonts) and positioned its marks there, while axes, gridlines, and annotations rendered in the primary's area.

On a layered scatter with a title + top legend (the AISD D/F scatter on labs), every bubble drew ~130px up and ~128px left of where the axes said it should be, compressing the whole cloud into the upper-left of the plot.

Fix

  • New internal CompileOptions.frozenChartArea: when set, compileChart uses it instead of the computed area.
  • compileLayer passes the primary layout's area to every leaf compile, so all layers share one coordinate space.
  • Leaves also inherit the layer-level theme/darkMode so mark styling matches the primary.

Testing

  • New regression test: leaf point marks and the rule diagonal must land at axis-implied pixel positions (was drifting by 100+px, now <1.5px).
  • Full unit suite (2466), visual baselines (20), and layout invariants (48) all pass.

Summary by CodeRabbit

  • Bug Fixes
    • Leaf-layer marks now align correctly with the main chart’s coordinate space, preventing drift when charts include titles, subtitles, or legends.
    • Additional layers now reuse the same chart area as the primary layout, improving consistency across layered visualizations.
    • Theme settings are now applied more consistently to leaf layers, helping preserve expected styling in layered charts.

compileLayer compiled each leaf spec raw, without the layer-level chrome,
legend, and theme that buildPrimarySpec merges into the primary spec. Each
leaf therefore computed its own chart area: no title/subtitle reservation,
a default legend position instead of the layer's, and default theme fonts.
Leaf marks landed in that private coordinate space while axes, gridlines,
and annotations rendered in the primary's, so on a layered scatter with a
title and top legend every bubble drew up-and-left of where the axis said
it should be (about 130px vertical and 128px horizontal drift at 766px).

Freeze the primary layout's area into CompileOptions.frozenChartArea for
leaf compiles so every layer's scales and marks share the primary's
coordinate space, and inherit the layer-level theme/darkMode so leaf mark
styling matches the primary.
@rileyhilliard rileyhilliard merged commit 117d09f into main Jul 4, 2026
1 of 2 checks passed
@rileyhilliard rileyhilliard deleted the fix/layer-leaf-chart-area branch July 4, 2026 12:43
@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 80ce1cde-ac92-4563-b715-ebeadab2d89e

📥 Commits

Reviewing files that changed from the base of the PR and between 06a67c1 and 9db8a96.

📒 Files selected for processing (4)
  • packages/core/src/types/layout.ts
  • packages/engine/src/__tests__/compile-layer.test.ts
  • packages/engine/src/compile.ts
  • packages/engine/src/compile/layer.ts

📝 Walkthrough

Walkthrough

Adds an optional frozenChartArea field to CompileOptions, used by compileChart to override the computed chart area. compileLayer now passes the primary layout's area as a frozen chart area when compiling additional leaf layers, and inherits theme/darkMode from the parent spec. A regression test validates leaf mark alignment.

Changes

Frozen Chart Area for Leaf Layer Alignment

Layer / File(s) Summary
Type contract and chart area override
packages/core/src/types/layout.ts, packages/engine/src/compile.ts
CompileOptions gains an optional frozenChartArea?: Rect field; compileChart uses it in place of the computed dims.chartArea when provided.
Leaf layer compilation and validation
packages/engine/src/compile/layer.ts, packages/engine/src/__tests__/compile-layer.test.ts
compileLayer compiles additional leaf layers with frozenChartArea set to the primary layout's area and a themed leaf spec inheriting theme/darkMode; a new test verifies point and rule marks align to the primary coordinate space.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant compileLayer
  participant compileChart

  Caller->>compileLayer: compile(spec, options)
  compileLayer->>compileLayer: compute primaryLayout
  compileLayer->>compileLayer: build themedLeaf (inherit theme/darkMode)
  compileLayer->>compileLayer: build leafOptions with frozenChartArea = primaryLayout.area
  compileLayer->>compileChart: compileChart(themedLeaf, leafOptions)
  compileChart->>compileChart: chartArea = frozenChartArea ?? dims.chartArea
  compileChart-->>compileLayer: leaf marks positioned in primary coordinate space
  compileLayer-->>Caller: compiled layered chart
Loading

Compact metadata

  • Key components modified: CompileOptions, compileChart, compileLayer
  • Impact: Leaf layer marks align to the primary chart layout's coordinate space instead of computing an independent chart area
  • Test coverage: New regression test added for compileLayer

Related PRs: None specified

Suggested labels: enhancement, engine

Suggested reviewers: None specified

Poem
A rabbit measured axes twice,
then froze the frame to keep it nice—
leaf marks now sit where they belong,
aligned with axes, straight and strong.
Hop, compile, and check the grid—
no more drifting marks amid! 🐇📐

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/layer-leaf-chart-area

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

1 participant