-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix #1913 by pushing overflowed legend items to a new line #3628
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
antoinerg
merged 14 commits into
plotly:master
from
nathanjenx:fix1913-horizontal-legened-groups
Mar 20, 2019
Merged
Changes from 5 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
b89f1ee
Fix #1913 by adding pushing overflowed legend items to a new line whi…
nathanjenx d152792
Add test image for legendgroup horizontal wrapping
nathanjenx b9542c4
Remove traceGroupGap from widths
nathanjenx f279235
Generate test images for legend group wrapping
nathanjenx 51178ec
Enforce minimum of 5px gap between traces when legend horizontal
nathanjenx 76df791
Fix traceGroup vriable and baseline images
nathanjenx 737150b
Ensure width of legend is checked before adding a new width
nathanjenx 3edc9eb
Remove extra height from horizontal grouped legends
nathanjenx 68848e4
Only add traceGap on new rows
nathanjenx f6d430b
Include intial 10px padding on legend
nathanjenx 9dfdedd
Add opts.tracegroupgap to height not traceGap
nathanjenx 0806a9a
Ensure opt.tracegroupgap is added to legend rather than traceGap, ref…
nathanjenx 131145d
test that changing `legend.tracegroupgap` updates the margin size
antoinerg a5fa247
legend: minor style improvements to the code
antoinerg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
"data": [{ | ||
"x": [1, 2, 3, 4], | ||
"y": [63.69, 62.55, 61.64, 61.39], | ||
"legendgroup": 1, | ||
"name": "Trace A-1" | ||
}, { | ||
"x": [1, 2, 3, 4], | ||
"y": [58.24, 54.93, 42.11, 50.75], | ||
"legendgroup": 1, | ||
"name":"Trace A-2" | ||
}, { | ||
"x": [1, 2, 3, 4], | ||
"y": [51.49, 49.59, 37.12, 31.45], | ||
"legendgroup":2, | ||
"name":"Trace B-1" | ||
}, { | ||
"x": [1, 2, 3, 4], | ||
"y": [49.09, 58.54, 53.91, 43.12], | ||
"legendgroup":2, | ||
"name":"Trace B-2" | ||
}, { | ||
"x": [1, 2, 3, 4], | ||
"y": [70.53, 72.51, 72.28, 78.65], | ||
"name":"Trace C-1" | ||
}, { | ||
"x": [1, 2, 3, 4], | ||
"y": [62.69, 59.09, 63.82, 62], | ||
"legendgroup":3, | ||
"name":"Trace D-1" | ||
}, { | ||
"x": [1, 2, 3, 4], | ||
"y": [76.27, 71.43, 59.83, 64.34], | ||
"legendgroup":3, | ||
"name":"Trace D-2" | ||
}, { | ||
"x": [1, 2, 3, 4], | ||
"y": [71.15, 81.82, 88.46, 74.29], | ||
"name":"Trace E-1" | ||
}], | ||
"layout": { | ||
"width": 600, | ||
"legend": { | ||
"orientation": "h" | ||
}, | ||
"xaxis": { | ||
"type": "linear", | ||
"range": [0.7840236686390533, 4.215976331360947], | ||
"autorange": true | ||
}, | ||
"yaxis": { | ||
"type": "linear", | ||
"range": [27.274108280254776, 92.63589171974522], | ||
"autorange": true | ||
} | ||
} | ||
} |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NON-BLOCKING
We could probably clean up these few lines a little:
forEach
Lib.aggNums
instead ofMath.max.apply
andgroup.reduce
for(var i = 0, n = groupData.length; i < n; i++)
loopsfor(var i = 0; i < groupData.length; i++)
, i.e. no need to definen
(I think).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in a5fa247 except for
group.reduce
.Can I really replace
group.reduce
withLib.aggNums
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe with something like:
similar to
Lib.mean
, but you're right, that's probably an overkill here as all theb[0].height
should be defined. 💃