Issue #577: wrap horizontal legends instead of overflowing the image#1007
Issue #577: wrap horizontal legends instead of overflowing the image#1007timmolter wants to merge 1 commit into
Conversation
A horizontal legend (LegendLayout.Horizontal) laid every entry out on a single row. With many series, or long series names, that row grew wider than the image, so the centered OutsideS legend spilled past both edges and was truncated. Entries now flow left-to-right and wrap onto additional rows once a row fills up, keyed off the chart width so the legend box always stays within the image. Sizing (Legend_.getBoundsHintHorizontal) and painting (each Legend_* subclass) walk the same series in the same order using the same per-entry advance width, so they wrap at identical points and the plot reserves the correct vertical space below it. - Legend_: wrap logic in getBoundsHintHorizontal, shared HorizontalCursor and helpers (row height, max row width, per-entry advance width). - Legend_Marker/Pie/Bubble/HorizontalBar/OHLC: lay out horizontal entries via the cursor. OHLC overrides the marker width (series-line length). - HeatMap legend is unaffected (custom layout, not series labels). - Add RegressionTestIssue577 and TestForIssue577 demo. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| * chart width (minus margin and padding) so the centered OutsideS legend box never extends past | ||
| * the image edge (issue #577). | ||
| */ | ||
| double getHorizontalLegendMaxRowWidth() { |
There was a problem hiding this comment.
Bug (verified): a near-max-width row can still be truncated on the right edge.
The wrap threshold is keyed off the chart width, but OutsideS centers the legend box on the plot (xOffset = plotX + (plotW - boundsW)/2). Since the plot center sits right of the image center (the y-axis consumes left space), a row that packs close to maxRowWidth overflows the right edge.
Reproduced empirically (800px-wide chart, 14 series, sweeping name lengths):
nameLen=10 width=768.7 xOffset=38.8 rightEdge=807.5 → 7.5px truncated
nameLen=32 width=763.7 xOffset=41.3 rightEdge=805.0 → 5.0px truncated
nameLen=22 width=758.5 xOffset=43.9 rightEdge=802.4 → 2.4px truncated
The wrap threshold can't simply use the plot width instead — legend bounds are consumed during axis preparation, before the plot width is final (circular dependency). The clean fix is to clamp the computed xOffset in paint() so the box always stays within the image:
// after the switch statement, for horizontal layouts
xOffset = Math.min(xOffset, chart.getWidth() - bounds.getWidth() - LEGEND_MARGIN);
xOffset = Math.max(xOffset, LEGEND_MARGIN);This keeps the plot-centered position in the common case and only nudges the box left when it would otherwise be cut off. The regression test should then also assert position (xOffset >= 0 && xOffset + width <= chartWidth), not just width.
| * The width of the legend graphic (line or box) preceding an entry's text. Subclasses whose | ||
| * graphic isn't sized by render type (e.g. OHLC) override this. | ||
| */ | ||
| int getLegendEntryMarkerWidth(S series) { |
There was a problem hiding this comment.
Minor (pre-existing, carried over): Scatter entries under-advance by legendSeriesLineLength - BOX_SIZE (4px at defaults).
Legend_Marker.doPaint() draws both Line and Scatter entries' text at startx + getLegendSeriesLineLength() + padding (the marker is centered at lineLength / 2), but this method returns BOX_SIZE for Scatter because it only special-cases Line. So a Scatter entry actually occupies lineLength + padding + textWidth while its advance width claims BOX_SIZE + padding + textWidth.
The old single-row code had the same inconsistency (it also only checked == Line), so this PR faithfully preserves existing spacing. But now that the advance width also decides wrap points, a Scatter-heavy legend can pack a row a few px too tight and let the last entry's text touch the box border.
Suggested follow-up (would slightly change existing horizontal spacing for scatter charts, hence not snuck into this PR):
return (series.getLegendRenderType() == LegendRenderType.Line
|| series.getLegendRenderType() == LegendRenderType.Scatter)
? chart.getStyler().getLegendSeriesLineLength()
: BOX_SIZE;
timmolter
left a comment
There was a problem hiding this comment.
Reviewed the diff (self-review pass). CI is green and the core design is sound: sizing (getBoundsHintHorizontal) and painting (HorizontalCursor in each subclass) walk the same series with the same per-entry advance width, so wrap points agree; single-row output reduces to the previous formulas byte-for-byte, preserving the #892 alignment fix; the OutsideS plot correctly reserves the taller box's height.
Findings, ranked:
-
Right-edge truncation still possible (verified, inline comment on
getHorizontalLegendMaxRowWidth) — the wrap threshold uses the chart width butOutsideScenters on the plot, whose center is right of the image center. A row packing close tomaxRowWidthoverflows the right edge by up to ~8px in my probe. Fix: clampxOffsetinto[LEGEND_MARGIN, chartWidth - boundsWidth - LEGEND_MARGIN]after the position switch, and strengthenRegressionTestIssue577to assert position, not just width. -
Scatter advance-width mismatch (minor, pre-existing — inline comment on
getLegendEntryMarkerWidth) —Scattertext is drawn at the line-length offset but advances byBOX_SIZE; carried over from the old code, now also nudges wrap points a few px tight. -
Nit, no action needed: the bounds pass accumulates row width as a plain sum while the painter computes
x - leftOrigin; floating-point non-associativity could theoretically flip a>comparison at an exact boundary, but the divergence is ~1 ULP and harmless in practice.
Item 1 should be fixed before merge; item 2 is fine as a follow-up.
Fixes #577.
Problem
With
LegendPosition.OutsideS+LegendLayout.Horizontal, all legend entries were laid out on a single row. With many series (or long series names) that row grew wider than the image, so the centered legend spilled past both edges and was truncated.Fix
Legend entries now flow left-to-right and wrap onto additional rows once a row fills up. The wrap threshold is keyed off the chart width (minus margin/padding) so the legend box always stays within the image. Because the plot reserves
legend.getBounds().getHeight()below itself forOutsideS, the plot automatically shrinks to make room for the extra rows.Sizing (
Legend_.getBoundsHintHorizontal) and painting (eachLegend_*subclass) walk the same series in the same order using the same per-entry advance width, so they wrap at identical points and the reported box matches what is painted.Changes
Legend_: wrapping logic ingetBoundsHintHorizontal, plus a sharedHorizontalCursorand helpers (computeHorizontalRowHeight,getHorizontalLegendMaxRowWidth,getHorizontalLegendEntryAdvanceWidth,getLegendEntryMarkerWidth).Legend_Marker/Legend_Pie/Legend_Bubble/Legend_HorizontalBar/Legend_OHLC: lay out horizontal entries via the cursor.Legend_OHLCoverrides the marker width (always the series-line length).Legend_HeatMapis unaffected — it uses a fully custom layout of color ranges, not series labels.Tests / demo
RegressionTestIssue577— asserts the wrapped legend fits within the image width and is taller than a single row (would fail pre-fix, where the box was far wider than the chart).TestForIssue577— standalone reproducer/demo.xchartsuite green (139 tests).Result
Before: the horizontal legend ran off both edges of the image. After: it wraps and stays centered within the image.
Follow-up options (not in this PR)
Auto-wrapping at the chart width is applied by default. Two alternatives were considered and can be added later if desired:
setLegendWrap(true)) to preserve the old single-row overflow for anyone relying on it.setLegendMaxWidth(px)) so callers can wrap at a width other than the chart width.🤖 Generated with Claude Code