Skip to content

Commit

Permalink
Merge pull request #3446 from plotly/horiz-legend-wrap-all-lines-fix
Browse files Browse the repository at this point in the history
Fix wrapped horizontal legends height computations
  • Loading branch information
etpinard committed Jan 21, 2019
2 parents 13daed2 + 0ed6d4b commit 0251a0c
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 6 deletions.
16 changes: 10 additions & 6 deletions src/components/legend/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,6 @@ function computeLegendDimensions(gd, groups, traces) {
var offsetX = 0;
var fullTracesWidth = 0;
var traceGap = opts.tracegroupgap || 5;
var oneRowLegend;

// calculate largest width for traces and use for width of all legend items
traces.each(function(d) {
Expand All @@ -639,15 +638,16 @@ function computeLegendDimensions(gd, groups, traces) {
});

// check if legend fits in one row
oneRowLegend = fullLayout._size.w > borderwidth + fullTracesWidth - traceGap;
var oneRowLegend = fullLayout._size.w > borderwidth + fullTracesWidth - traceGap;

This comment has been minimized.

Copy link
@Braintelligence

Braintelligence Jan 23, 2019

Could this be related to #3175 (comment) ? Does the detection if we're looking at one row fail in Firefox mobile developer view?

This comment has been minimized.

Copy link
@Braintelligence

Braintelligence Jan 23, 2019

Yes, I've added console.log(oneRowLegend) below this line and it returns true in mobile developer view in Firefox but returns false first and then true in a small window of the same size where it looks correct. This is the same case for Chrome and Edge.

If I set layout.autosize = false I always get true but if I set layout.autosize = true and do NOT use Firefox mobile developer view I always get false and then true afterwards resulting in a correct rendering of the legend.

EDIT2: Getting first false then true shows that the algorithm correctly sees that it needs to "break" the legend once so the second line actually fulfills being the last single row. We just need to reach this behaviour in firefox mobile developer view as well!

@etpinard I hope this helps you pinpointing what's going on here :(

EDIT: I have an idea! Could it be that you guys are working with the window.width here? This could lead to problems with Firefox mobile developer view since the window.width includes the grey area that is not interesting iirc.


traces.each(function(d) {
var legendItem = d[0];
var traceWidth = oneRowLegend ? 40 + d[0].width : maxTraceWidth;

if((borderwidth + offsetX + traceGap + traceWidth) > fullLayout._size.w) {
offsetX = 0;
rowHeight = rowHeight + maxTraceHeight;
opts._height = opts._height + maxTraceHeight;
rowHeight += maxTraceHeight;
opts._height += maxTraceHeight;
// reset for next row
maxTraceHeight = 0;
}
Expand All @@ -657,16 +657,20 @@ function computeLegendDimensions(gd, groups, traces) {
(5 + borderwidth + legendItem.height / 2) + rowHeight);

opts._width += traceGap + traceWidth;
opts._height = Math.max(opts._height, legendItem.height);

// keep track of tallest trace in group
offsetX += traceGap + traceWidth;
maxTraceHeight = Math.max(legendItem.height, maxTraceHeight);
});

if(oneRowLegend) {
opts._height = maxTraceHeight;
} else {
opts._height += maxTraceHeight;
}

opts._width += borderwidth * 2;
opts._height += 10 + borderwidth * 2;

}

// make sure we're only getting full pixels
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
58 changes: 58 additions & 0 deletions test/image/mocks/legend_horizontal_wrap-alll-lines.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
{
"data": [
{
"x": [ 1, 2, 3, 4 ],
"y": [ 3, 5, 1, 7 ],
"name": "Break this and last<br>trace will display properly"
},
{
"x": [ 1, 2, 3, 4 ],
"y": [ 3, 5, 1, 7 ],
"name": "You need to break this trace twice if first one has no break"
},
{
"x": [ 1, 2, 3, 4 ],
"y": [ 3, 5, 1, 7 ],
"name": "You need to break this trace twice if first one has no break"
},
{
"x": [ 1, 2, 3, 4 ],
"y": [ 3, 5, 1, 7 ],
"name": "You need to break this trace twice if first one has no break"
},
{
"x": [ 1, 2, 3, 4 ],
"y": [ 3, 5, 1, 7 ],
"name": "You need to break this trace twice if first one has no break"
},
{
"x": [ 1, 2, 3, 4 ],
"y": [ 3, 5, 1, 7 ],
"name": "You need to break this trace twice if first one has no break"
},
{
"x": [ 1, 2, 3, 4 ],
"y": [ 3, 5, 1, 7 ],
"name": "This<br> contains<br>a break"
}
],
"layout": {
"legend": {
"orientation": "h",
"xanchor": "center",
"yanchor": "top",
"x": 0.5,
"y": -0.1,
"traceorder": "normal"
},
"paper_bgcolor": "rgba(255,0,0,0.5)",
"margin": {
"l": 20,
"r": 20,
"t": 20,
"b": 20
},
"width": 360,
"height": 600
}
}

0 comments on commit 0251a0c

Please sign in to comment.