Add opt-in placement="block" to <Legend> (fixes #7)#21
Open
KyleKreuter wants to merge 1 commit into
Open
Conversation
|
✅ Comp AI code review complete — no issues found. Reviewed commit Posted by Comp AI Code Reviews. |
Author
There was a problem hiding this comment.
No blocking issues found across the changed files.
Posted by Comp AI Code Reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


What
Adds an opt-in
placementprop to<Legend>. The default"overlay"is unchanged;placement="block"renders the same legend in normal flow above the plot, so it reserves its own height and can't overlap the chart at any width.Closes #7.
Why
<Legend>is an absolute overlay pinned to the top of the plot, so it reserves no layout space. Past ~3 entries (or in a narrow card)flex-wrappushes rows onto the plot, and the wrap count changes with width — a chart that looks fine at one size overlaps at another.BlockLegendalready exists as a separate, config-fed component; this is the lighterLegend placement="block"the issue floated (direction 2), keeping the same composed API, chart context, andisClickable/align/ hover-spotlight behavior.How
legend.tsx— addplacement?: "overlay" | "block"(default"overlay"); the wrapper isabsolute inset-x-0 top-0for overlay, in-flow (w-full pb-2) for block.cartesian-root.tsx/polar-root.tsx— a block-placement legend routes to a flow slot: the root becomes aflex flex-col, the legend sits above aflex-1plot area that holds the canvas/svg/overlay chrome. Pure flexbox reservation — no measurement, no feedback loop.Non-breaking: with no block legend the flow slot is empty and the layout is identical to before. No changes to the controllers or the shared context. Most of the root diff is one level of added indentation from the new wrapper.
Tests
tests/legend-placement.test.tsxasserts the overlay legend renders inside the plot (absolute), and the block legend renders outside/above it. Full suite: 30 passing. Registry regenerated withnpm run build:registry(r/core.json).Summary by Comp AI
No blocking issues found.
Written for commit
8cd45ee. New commits will trigger a re-review. Generated by Comp AI.