fix(print-layout): match proportional legend sizes to map - #1779
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change propagates captured map pixel ratios through print layout and atlas export flows. Legend symbols now use fitted map scale and pixel ratio. Proportional legend rows derive dimensions from rendered radii and receive entry-specific overflow scaling. Tests cover fitted scaling and oversized symbols. ChangesPixel-ratio-aware print layout
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant captureMapImage
participant PrintLayoutDialog
participant print_layout
captureMapImage->>PrintLayoutDialog: return CapturedMap.pixelRatio
PrintLayoutDialog->>print_layout: pass mapPixelRatio to layout and atlas options
print_layout->>PrintLayoutDialog: render scaled legend symbols and atlas layouts
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. 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. Comment |
🔍 Cloudflare PR preview
|
🔍 GitHub Pages PR preview
Note GitHub Pages built this preview successfully, but its serving edge returned HTTP 403 when checked. The links may still be propagating. |
Code reviewBugs
Security
Performance
Quality
CLAUDE.md
|
- Cap the legend swatch so one outsized proportional symbol cannot blank the whole box. Sizing rows 1:1 with the map made rowH independent of the page, so a single very large symbol could push a row past the caller's maxHeight and trip drawLegend's `fitRows === 0` early return, dropping normally-sized entries along with it. A row is now held to a quarter of the post-chrome budget, leaving room for a heading, two class rows and the "+N more" note. - Shrink every sized symbol by the same factor when the cap bites, so a ramp keeps its internal ratios and nothing overflows its swatch box. - Hoist chromeH (and the title metrics it needs) above the sizing math and drop the duplicate in the truncation block. - Add a regression test for the outsized-symbol case, and keep the existing fit-scale test's symbol under the cap so it still asserts the 1:1 map-fit scaling on its own.
|
Posted successfully. That's the only substantive finding worth flagging — everything else in the diff checks out cleanly against manual arithmetic and the new tests. Code reviewBugs
Security
Performance
Quality
CLAUDE.md
Everything else — the |
- Scope the outsized-symbol shrink to the entry that overflowed, instead of applying one legend-wide factor. The swatch column and row height stay legend-wide (they have to be — every row shares them), but the shrink is now keyed by entryId, mirroring MapLegendPanel's per-entry maxRowSize. A legend holding two proportional layers no longer shrinks the well-behaved one just because its neighbour has an outlier; that layer still renders 1:1 with the map, which is the point of the PR. This also makes the "entryId keeps proportional scaling scoped to one legend entry" comment true again. - Add a test for the two-layer case: the modest layer's marker box stays at its 1:1 size while the outlier layer is capped.
Code reviewI traced the full scaling chain ( Bugs
Security
Performance
Quality
CLAUDE.md
No inline comments posted — nothing rose to a level worth flagging on specific lines. |
Summary
Verification
ruchers_018.csvdataset andnb_ruchesproportional sizingnode --import tsx --test tests/print-layout.test.ts --test-name-pattern='drawLayout legend rendering'npm run buildpre-commit run --files ...Refs #1711 (comment)
Summary by CodeRabbit
Bug Fixes
Tests