Skip to content

Implement layout.legend.orientation (closes #53) #535

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.

Sign up for GitHub

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
merged 19 commits into from
May 17, 2016
Merged

Implement layout.legend.orientation (closes #53) #535

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
ff16df1
Move setup of legend trace toggle into a function
n-riesco Apr 28, 2016
ddc6d71
Simplify signature of drawTexts in legend
n-riesco Apr 28, 2016
f40dbd4
Legend: add attribute `orientation`
n-riesco May 12, 2016
92ff540
Legend: add helper.isVertical(legendLayout)
n-riesco May 12, 2016
869538c
Legend: refactor computation of text dimensions
n-riesco May 12, 2016
b2538e1
Legend: set tspan.line's x in the callback
n-riesco May 13, 2016
9dc2609
Legend: set tspan.line's x before expandMargin
n-riesco May 13, 2016
f68344b
Legend: compute size only after drawing texts
n-riesco May 13, 2016
c0dfc8a
Legend: move placing of groups and traces together
n-riesco May 13, 2016
e5049c9
Legend: implement layout.legend.orientation
n-riesco May 13, 2016
23e3972
Legend: add mocks with horizontal legends
n-riesco May 13, 2016
25c2970
Legend: Use D3 to invoke functions
n-riesco May 17, 2016
7d502fa
Legend: Use Lib.setTranslate
n-riesco May 17, 2016
107883a
Legend: replace .each() by a explicit loop
n-riesco May 17, 2016
65aa6c0
Legend: Remove legendItem from function signatures
n-riesco May 17, 2016
8c6d47a
Legend: set default position of horizontal legends
n-riesco May 17, 2016
bc20390
Legend: ensure all the legends sides are visible
n-riesco May 17, 2016
c1632d4
Legend: update baseline of legend_horizontal.json
n-riesco May 17, 2016
313976d
Legend: test defaults for horizontal legends
n-riesco May 17, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 25 additions & 5 deletions src/components/legend/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@ module.exports = function legendDefaults(layoutIn, layoutOut, fullData) {
containerOut = layoutOut.legend = {};

var visibleTraces = 0,
defaultOrder = 'normal';
defaultOrder = 'normal',
defaultX,
defaultY,
defaultXAnchor,
defaultYAnchor;

for(var i = 0; i < fullData.length; i++) {
var trace = fullData[i];
Expand Down Expand Up @@ -57,14 +61,30 @@ module.exports = function legendDefaults(layoutIn, layoutOut, fullData) {
coerce('bordercolor');
coerce('borderwidth');
Lib.coerceFont(coerce, 'font', layoutOut.font);

coerce('orientation');
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add some smart default logic so that if a user set orientation to 'h', the default 'x' would be 0 (instead of 1.02).

Moreover, maybe we should make the default 'y' value 0 instead of 1 ? But that one is less obvious to me.

@mdtusz @chriddyp @jackparmer should the default horizontal legend start at the bottom-left or at the top-left corner of the plot area?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd think bottom left but if we do this, we will need to take into account the case where range sliders are present. The legend is currently rendered below it so it gets hidden if it is placed in the same place.

if(containerOut.orientation === 'h') {
var xaxis = layoutIn.xaxis;
if(xaxis && xaxis.rangeslider && xaxis.rangeslider.visible) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, that works. layout.xaxis is coerced before layout.legend. 👍

defaultX = 0;
defaultXAnchor = 'left';
defaultY = 1.1;
defaultYAnchor = 'bottom';
}
else {
Copy link
Contributor

Choose a reason for hiding this comment

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

@n-riesco can you add a few jasmine test cases in legend_test.js to test out this logic?

defaultX = 0;
defaultXAnchor = 'left';
defaultY = -0.1;
defaultYAnchor = 'top';
}
}

coerce('traceorder', defaultOrder);
if(helpers.isGrouped(layoutOut.legend)) coerce('tracegroupgap');

coerce('x');
coerce('xanchor');
coerce('y');
coerce('yanchor');
coerce('x', defaultX);
coerce('xanchor', defaultXAnchor);
coerce('y', defaultY);
coerce('yanchor', defaultYAnchor);
Lib.noneOrAll(containerIn, containerOut, ['x', 'y']);
};
Binary file modified test/image/baselines/legend_horizontal.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
6 changes: 1 addition & 5 deletions test/image/mocks/legend_horizontal.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,6 @@
},
"legend": {
"orientation": "h",
"x": 0,
"xanchor": "left",
"y": -0.1,
"yanchor": "top",
"traceorder": "normal",
"font": {
"family": "",
Expand All @@ -187,7 +183,7 @@
},
"bgcolor": "#fff",
"bordercolor": "#444",
"borderwidth": 0
"borderwidth": 1
},
"margin": {
"l": 80,
Expand Down