Skip to content

Issue #577: wrap horizontal legends instead of overflowing the image#1007

Open
timmolter wants to merge 1 commit into
developfrom
issue-577-horizontal-legend-wrap
Open

Issue #577: wrap horizontal legends instead of overflowing the image#1007
timmolter wants to merge 1 commit into
developfrom
issue-577-horizontal-legend-wrap

Conversation

@timmolter

Copy link
Copy Markdown
Member

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 for OutsideS, the plot automatically shrinks to make room for the extra rows.

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 reported box matches what is painted.

Changes

  • Legend_: wrapping logic in getBoundsHintHorizontal, plus a shared HorizontalCursor and helpers (computeHorizontalRowHeight, getHorizontalLegendMaxRowWidth, getHorizontalLegendEntryAdvanceWidth, getLegendEntryMarkerWidth).
  • Legend_Marker / Legend_Pie / Legend_Bubble / Legend_HorizontalBar / Legend_OHLC: lay out horizontal entries via the cursor. Legend_OHLC overrides the marker width (always the series-line length).
  • Legend_HeatMap is unaffected — it uses a fully custom layout of color ranges, not series labels.
  • Single-row horizontal legends render identically to before (the new formulas reduce to the old ones for one row), so the issue Mixed series render style legend layout #892 alignment fix is preserved.

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.
  • Full xchart suite 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:

  1. Opt-in flag (e.g. setLegendWrap(true)) to preserve the old single-row overflow for anyone relying on it.
  2. Configurable max width (e.g. setLegendMaxWidth(px)) so callers can wrap at a width other than the chart width.

🤖 Generated with Claude Code

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() {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 timmolter left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. Right-edge truncation still possible (verified, inline comment on getHorizontalLegendMaxRowWidth) — the wrap threshold uses the chart width but OutsideS centers on the plot, whose center is right of the image center. A row packing close to maxRowWidth overflows the right edge by up to ~8px in my probe. Fix: clamp xOffset into [LEGEND_MARGIN, chartWidth - boundsWidth - LEGEND_MARGIN] after the position switch, and strengthen RegressionTestIssue577 to assert position, not just width.

  2. Scatter advance-width mismatch (minor, pre-existing — inline comment on getLegendEntryMarkerWidth)Scatter text is drawn at the line-length offset but advances by BOX_SIZE; carried over from the old code, now also nudges wrap points a few px tight.

  3. 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.

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.

Horizontal legends are truncated when too many series are used

1 participant