-
Notifications
You must be signed in to change notification settings - Fork 0
Horizontal legend spacing #1
base: master
Are you sure you want to change the base?
Conversation
Please remove the |
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.
How does this respond to legend groups (ie traceorder: "grouped"
with an appropriate legendgroup
trace)?
@@ -590,14 +591,14 @@ function computeLegendDimensions(gd, groups, traces) { | |||
maxTraceWidth = 0, | |||
offsetX = 0; | |||
|
|||
// calculate largest width for traces and use for width of all legend items | |||
// calculate largest width for traces and use for width of all legend items (if horizontalspacing mode is 'column') | |||
traces.each(function(d) { |
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.
maxTraceWidth
isn't used unless isHorizontalColumn
is true
, so don't compute it if not.
traces.each(function(d) { | ||
maxTraceWidth = Math.max(40 + d[0].width, maxTraceWidth); | ||
}); | ||
|
||
traces.each(function(d) { | ||
var legendItem = d[0], | ||
traceWidth = maxTraceWidth, | ||
traceWidth = isHorizontalColumn ? maxTraceWidth : 40 + d[0].width, |
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.
Where is the 40 coming from? Leave a comment.
|
||
exports.isHorizontalColumn = function isHorizontalColumn(legendLayout) { | ||
return legendLayout.horizontalspacing === 'column'; | ||
} |
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.
Missing newline
describe('isHorizontalColumn', function() { | ||
var isHorizontalColumn = helpers.isHorizontalColumn; | ||
|
||
it('should return true when option horizontalspacing is "column"', function() { |
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.
Break this into two tests: should return true when option horizontalspacing is "column"
, and should return false when option horizontalspacing is "wrapped"
@@ -628,3 +637,54 @@ describe('legend restyle update', function() { | |||
}); | |||
}); | |||
}); | |||
|
|||
describe('legend horizontal spacing', function() { |
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.
Good tests!
e5836b4
to
8296b90
Compare
* Fix bug in scattergl plot * Added tests for scattergl plot update
No description provided.