Skip to content

Commit 1b21ccc

Browse files
committed
fix: Avoid title and legend collisions
This checks if there is a collision between the title and legend and adjusts the legend down if necessary to avoid the collision. Because this resulted in a lot of SVG changes (in part because of the render order), the double space (` `) in the `path` tag was also fixed.
1 parent 155df06 commit 1b21ccc

31 files changed

+1114
-545
lines changed

alias_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ func TestMergeFontStyles(t *testing.T) {
262262
assert.InDelta(t, tt.expected.FontSize, result.FontSize, matrix.DefaultEpsilon)
263263
assert.Equal(t, tt.expected.FontColor, result.FontColor)
264264

265-
// Handle font comparison carefully since they're pointers
265+
// Handle font comparison carefully since their pointers
266266
if tt.expected.Font != nil && result.Font != nil {
267267
// Both should be non-nil, check if they match expected source
268268
if tt.primary.Font != nil {

axis_test.go

Lines changed: 12 additions & 12 deletions
Large diffs are not rendered by default.

bar_chart_test.go

Lines changed: 20 additions & 20 deletions
Large diffs are not rendered by default.

candlestick_chart_test.go

Lines changed: 21 additions & 21 deletions
Large diffs are not rendered by default.

candlestick_patterns_test.go

Lines changed: 22 additions & 22 deletions
Large diffs are not rendered by default.

chart_option_test.go

Lines changed: 15 additions & 15 deletions
Large diffs are not rendered by default.

chartdraw/vector_renderer.go

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -389,8 +389,17 @@ func (c *canvas) Path(parts []string, style Style) {
389389
bb := c.bb
390390
defer c.bb.Reset()
391391

392-
bb.WriteString(`<path `)
393-
c.writeStrokeDashArray(bb, style)
392+
bb.WriteString(`<path`)
393+
if len(style.StrokeDashArray) > 0 {
394+
bb.WriteString(" stroke-dasharray=\"")
395+
for i, v := range style.StrokeDashArray {
396+
if i > 0 {
397+
bb.WriteString(", ")
398+
}
399+
_, _ = fmt.Fprintf(bb, "%0.1f", v)
400+
}
401+
bb.WriteString("\"")
402+
}
394403
bb.WriteString(` d="`)
395404
for i, p := range parts {
396405
if i > 0 {
@@ -419,7 +428,7 @@ func (c *canvas) Text(x, y int, body string, style Style) {
419428
bb.WriteString(`" `)
420429
styleAsSVG(bb, style, c.dpi, true)
421430
if c.textTheta != nil {
422-
fmt.Fprintf(bb, ` transform="rotate(%0.2f,%d,%d)"`, RadiansToDegrees(*c.textTheta), x, y)
431+
_, _ = fmt.Fprintf(bb, ` transform="rotate(%0.2f,%d,%d)"`, RadiansToDegrees(*c.textTheta), x, y)
423432
}
424433
bb.WriteRune('>')
425434
bb.WriteString(body)
@@ -449,20 +458,6 @@ func (c *canvas) End() {
449458
_, _ = c.w.Write([]byte("</svg>"))
450459
}
451460

452-
// writeStrokeDashArray writes the stroke-dasharray property of a style.
453-
func (c *canvas) writeStrokeDashArray(bb *bytes.Buffer, s Style) {
454-
if len(s.StrokeDashArray) > 0 {
455-
bb.WriteString("stroke-dasharray=\"")
456-
for i, v := range s.StrokeDashArray {
457-
if i > 0 {
458-
bb.WriteString(", ")
459-
}
460-
fmt.Fprintf(bb, "%0.1f", v)
461-
}
462-
bb.WriteString("\"")
463-
}
464-
}
465-
466461
// styleAsSVG returns the style as a svg style or class string.
467462
func styleAsSVG(bb *bytes.Buffer, s Style, dpi float64, applyText bool) {
468463
sw := s.StrokeWidth

charts.go

Lines changed: 75 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package charts
33
import (
44
"errors"
55
"math"
6+
"strconv"
67
"strings"
78

89
"github.com/go-analyze/charts/chartdraw"
@@ -140,37 +141,97 @@ func defaultRender(p *Painter, opt defaultRenderOption) (*defaultRenderResult, e
140141

141142
const legendTitlePadding = 15
142143
var legendTopSpacing int
143-
legendResult, err := newLegendPainter(p, *opt.legend).Render()
144+
var titleBox, legendResult Box
145+
var err error
146+
147+
// make a local copy of legend options before we modify position during collision handling
148+
legendOpt := *opt.legend
149+
150+
// helper to check if legend can be repositioned to avoid title collision
151+
// repositioning is allowed when no explicit numeric offset is set
152+
// OverlayChart only controls legend/chart overlap, not legend/title overlap
153+
legendCanReposition := func() bool {
154+
return (legendOpt.Offset.Top == "" || legendOpt.Offset.Top == PositionTop ||
155+
legendOpt.Offset.Top == PositionBottom) &&
156+
(legendOpt.Offset.Left == "" || legendOpt.Offset.Left == PositionLeft ||
157+
legendOpt.Offset.Left == PositionCenter || legendOpt.Offset.Left == PositionRight)
158+
}
159+
160+
// determine positioning
161+
titleAtBottom := opt.title.Offset.Top == PositionBottom
162+
legendAtBottom := legendOpt.Offset.Top == PositionBottom
163+
164+
// calculate legend box for both-at-bottom check and collision detection
165+
legendPainter := newLegendPainter(p, legendOpt)
166+
legendResult, err = legendPainter.calculateBox()
167+
if err != nil {
168+
return nil, err
169+
}
170+
171+
// when both title and legend are at bottom (without overlay), render title on a reduced canvas
172+
// so title appears above where legend will render (title above legend in reading order)
173+
titleCanvas := p
174+
adjustedForBottom := false
175+
if titleAtBottom && legendAtBottom && !legendResult.IsZero() &&
176+
!flagIs(true, legendOpt.OverlayChart) {
177+
titleCanvas = p.Child(PainterPaddingOption(Box{
178+
Bottom: legendResult.Height() + legendTitlePadding,
179+
IsSet: true,
180+
}))
181+
adjustedForBottom = true
182+
}
183+
184+
titleBox, err = newTitlePainter(titleCanvas, opt.title).Render()
144185
if err != nil {
145186
return nil, err
146187
}
147-
if !legendResult.IsZero() && !flagIs(true, opt.legend.Vertical) && !flagIs(true, opt.legend.OverlayChart) {
148-
legendHeight := legendResult.Height()
188+
189+
// check for collision and reposition if needed
190+
// skip if both-at-bottom was handled via canvas adjustment
191+
if !adjustedForBottom &&
192+
!titleBox.IsZero() && !legendResult.IsZero() &&
193+
legendCanReposition() && titleBox.Overlaps(legendResult) {
194+
legendOpt.Offset.Top = strconv.Itoa(titleBox.Bottom)
195+
legendPainter = newLegendPainter(p, legendOpt)
196+
}
197+
198+
// render legend with final position
199+
legendResult, err = legendPainter.Render()
200+
if err != nil {
201+
return nil, err
202+
}
203+
204+
// reserve space for the legend if not in overlay mode
205+
// - horizontal legends reserve space (top or bottom)
206+
// - vertical legends always overlay from the side
207+
// - skip for adjustedForBottom since title reservation handles combined space
208+
if !legendResult.IsZero() && !flagIs(true, legendOpt.OverlayChart) && !adjustedForBottom &&
209+
!flagIs(true, legendOpt.Vertical) {
149210
if legendResult.Bottom < p.Height()/2 {
150-
// horizontal legend at the top, set the spacing based on the height
151-
legendTopSpacing = legendHeight + legendTitlePadding
211+
// horizontal legend at top - reserve top space
212+
legendTopSpacing = chartdraw.MaxInt(legendResult.Height(), legendResult.Bottom) + legendTitlePadding
152213
} else {
153214
// horizontal legend at the bottom, raise the chart above it
154215
p = p.Child(PainterPaddingOption(Box{
155-
Bottom: legendHeight + legendTitlePadding,
216+
Bottom: legendResult.Height() + legendTitlePadding,
156217
IsSet: true,
157218
}))
158219
}
159220
}
160221

161-
titleBox, err := newTitlePainter(p, opt.title).Render()
162-
if err != nil {
163-
return nil, err
164-
}
222+
// apply title and legend spacing to chart area
165223
if !titleBox.IsZero() {
166224
titlePadBox := Box{IsSet: true}
167225
if titleBox.Bottom < p.Height()/2 {
168226
titlePadBox.Top = chartdraw.MaxInt(legendTopSpacing, titleBox.Bottom+legendTitlePadding)
169-
} else { // else, title is at the bottom, raise the chart to be above the title
170-
titlePadBox.Top = legendTopSpacing // the legend may still need space on the top, set to the legend space
171-
titlePadBox.Bottom = titleBox.Height()
227+
} else { // title is at the bottom, raise the chart to be above the title
228+
titlePadBox.Top = legendTopSpacing
229+
if adjustedForBottom {
230+
titlePadBox.Bottom = p.Height() - titleBox.Top
231+
} else {
232+
titlePadBox.Bottom = titleBox.Height()
233+
}
172234
}
173-
174235
p = p.Child(PainterPaddingOption(titlePadBox))
175236
} else if legendTopSpacing > 0 { // apply chart spacing below legend
176237
p = p.Child(PainterPaddingOption(Box{

charts_test.go

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,194 @@ import (
55
"testing"
66

77
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
89
)
910

1011
func TestGetNullValue(t *testing.T) {
1112
t.Parallel()
1213

1314
assert.InDelta(t, math.MaxFloat64, GetNullValue(), 0.0)
1415
}
16+
17+
func TestLegendRepositioning(t *testing.T) {
18+
t.Parallel()
19+
20+
tests := []struct {
21+
name string
22+
titleOpt TitleOption
23+
legendOpt LegendOption
24+
expectLegendRepositioned bool
25+
}{
26+
{
27+
name: "horizontal_center_with_center_title_collision",
28+
titleOpt: TitleOption{
29+
Text: "Test Title",
30+
Offset: OffsetCenter,
31+
},
32+
legendOpt: LegendOption{
33+
SeriesNames: []string{"Series A", "Series B", "Series C", "Series D", "Series E"},
34+
},
35+
expectLegendRepositioned: true,
36+
},
37+
{
38+
name: "horizontal_center_no_title",
39+
titleOpt: TitleOption{
40+
Text: "",
41+
},
42+
legendOpt: LegendOption{
43+
SeriesNames: []string{"Series A", "Series B"},
44+
},
45+
expectLegendRepositioned: false,
46+
},
47+
{
48+
name: "horizontal_bottom_no_collision",
49+
titleOpt: TitleOption{
50+
Text: "Test Title",
51+
Offset: OffsetCenter,
52+
},
53+
legendOpt: LegendOption{
54+
SeriesNames: []string{"A", "B"},
55+
Offset: OffsetStr{
56+
Top: PositionBottom,
57+
},
58+
},
59+
expectLegendRepositioned: false,
60+
},
61+
{
62+
name: "vertical_legend_with_collision",
63+
titleOpt: TitleOption{
64+
Text: "Test Title",
65+
Offset: OffsetLeft,
66+
},
67+
legendOpt: LegendOption{
68+
SeriesNames: []string{"Series A", "Series B"},
69+
Vertical: Ptr(true),
70+
},
71+
expectLegendRepositioned: true,
72+
},
73+
{
74+
name: "vertical_legend_no_collision_right_position",
75+
titleOpt: TitleOption{
76+
Text: "Test Title",
77+
Offset: OffsetLeft,
78+
},
79+
legendOpt: LegendOption{
80+
SeriesNames: []string{"Series A", "Series B"},
81+
Vertical: Ptr(true),
82+
Offset: OffsetRight,
83+
},
84+
expectLegendRepositioned: false,
85+
},
86+
{
87+
name: "explicit_numeric_offset_blocks_repositioning",
88+
titleOpt: TitleOption{
89+
Text: "Test Title",
90+
Offset: OffsetCenter,
91+
},
92+
legendOpt: LegendOption{
93+
SeriesNames: []string{"A", "B", "C", "D", "E"},
94+
Offset: OffsetStr{
95+
Top: "10",
96+
},
97+
},
98+
expectLegendRepositioned: false,
99+
},
100+
{
101+
name: "explicit_numeric_left_offset_blocks_repositioning",
102+
titleOpt: TitleOption{
103+
Text: "Test Title",
104+
Offset: OffsetCenter,
105+
},
106+
legendOpt: LegendOption{
107+
SeriesNames: []string{"A", "B", "C", "D", "E"},
108+
Offset: OffsetStr{
109+
Left: "50",
110+
},
111+
},
112+
expectLegendRepositioned: false,
113+
},
114+
{
115+
name: "overlay_chart_blocks_repositioning",
116+
titleOpt: TitleOption{
117+
Text: "Test Title",
118+
Offset: OffsetCenter,
119+
},
120+
legendOpt: LegendOption{
121+
SeriesNames: []string{"A", "B", "C", "D", "E"},
122+
OverlayChart: Ptr(true),
123+
},
124+
expectLegendRepositioned: false,
125+
},
126+
{
127+
name: "title_and_legend_both_bottom",
128+
titleOpt: TitleOption{
129+
Text: "Test Title",
130+
Offset: OffsetStr{
131+
Top: PositionBottom,
132+
},
133+
},
134+
legendOpt: LegendOption{
135+
SeriesNames: []string{"A", "B"},
136+
Offset: OffsetStr{
137+
Top: PositionBottom,
138+
},
139+
},
140+
expectLegendRepositioned: false,
141+
},
142+
}
143+
144+
for _, tt := range tests {
145+
t.Run(tt.name, func(t *testing.T) {
146+
// create chart with line series to trigger defaultRender
147+
opt := ChartOption{
148+
Title: tt.titleOpt,
149+
Legend: tt.legendOpt,
150+
SeriesList: NewSeriesListGeneric([][]float64{{1, 2, 3}}, ChartTypeLine),
151+
Width: 600,
152+
Height: 400,
153+
}
154+
155+
// save original offset
156+
originalTopOffset := tt.legendOpt.Offset.Top
157+
158+
p, err := Render(opt)
159+
require.NoError(t, err)
160+
require.NotNil(t, p)
161+
162+
// check if legend was repositioned by verifying original option wasn't mutated
163+
// (we use a copy internally, so original should always remain unchanged)
164+
assert.Equal(t, originalTopOffset, tt.legendOpt.Offset.Top,
165+
"original legend option should not be mutated")
166+
})
167+
}
168+
}
169+
170+
func TestLegendOptionNotMutated(t *testing.T) {
171+
t.Parallel()
172+
173+
// verify that rendering does not mutate the original legend option
174+
legendOpt := LegendOption{
175+
SeriesNames: []string{"A", "B", "C", "D", "E"},
176+
Offset: OffsetStr{
177+
Top: PositionTop,
178+
},
179+
}
180+
titleOpt := TitleOption{
181+
Text: "Wide Title That Should Cause Collision",
182+
Offset: OffsetCenter,
183+
}
184+
185+
opt := ChartOption{
186+
Title: titleOpt,
187+
Legend: legendOpt,
188+
SeriesList: NewSeriesListGeneric([][]float64{{1, 2, 3}}, ChartTypeLine),
189+
Width: 600,
190+
Height: 400,
191+
}
192+
193+
_, err := Render(opt)
194+
require.NoError(t, err)
195+
196+
// original should not have been modified
197+
assert.Equal(t, PositionTop, legendOpt.Offset.Top)
198+
}

0 commit comments

Comments
 (0)