Skip to content
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

[WIP] Vertically center multi-line labels on Y axis #2424

Merged
merged 11 commits into from
Mar 5, 2018
Merged

[WIP] Vertically center multi-line labels on Y axis #2424

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bb90e1d
Add test illustrating #1137.
darabos Feb 28, 2018
b682fe3
Vertically center multi-line labels on Y axis.
darabos Feb 28, 2018
8f8876c
Fix multi-line alignment in different angles and sides.
darabos Mar 1, 2018
e96f7e1
More complete bar_multiline_labels test with the change disabled.
darabos Mar 5, 2018
3adcdc9
Correct alignment on all sides and angles, reliable line count.
darabos Mar 5, 2018
b7a67ce
Fix eslint mistakes.
darabos Mar 5, 2018
4431506
Update affected prior baselines.
darabos Mar 5, 2018
261cb9d
Merge branch 'master' of https://github.com/plotly/plotly.js into dar…
darabos Mar 5, 2018
f4b2a85
Regenerate gl2d_date_axes.png after merge to resolve conflict.
darabos Mar 5, 2018
f9e1a57
Large anchor positioning cutoff for X. Use 30/60 degrees. Fix label i…
darabos Mar 5, 2018
99292ee
Update baseline.
darabos Mar 5, 2018
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
28 changes: 28 additions & 0 deletions src/plots/cartesian/axes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ var MINUS_SIGN = constants.MINUS_SIGN;
var BADNUM = constants.BADNUM;

var MID_SHIFT = require('../../constants/alignment').MID_SHIFT;
var LINE_SPACING = require('../../constants/alignment').LINE_SPACING;

var axes = module.exports = {};

Expand Down Expand Up @@ -2036,6 +2037,26 @@ axes.doTicks = function(gd, axid, skipTitle) {
});
}

// How much to shift a multi-line label to center it vertically.
function getAnchorHeight(lineCount, lineHeight, angle) {
var h = (lineCount - 1) * lineHeight;
if(axLetter === 'x') {
if(angle < -20 || 20 < angle) {
return -0.5 * h;
} else if(axside === 'top') {
return -h;
}
} else {
angle *= axside === 'left' ? 1 : -1;
if(angle < -20) {
return -h;
} else if(angle < 20) {
return -0.5 * h;
}
}
return 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Very nice, I don't think this is overly long at all, it has pretty much exactly the complexity required.

The new mock is fantastic, clearly covers all the cases we care about. The only question in my mind is the cutoff angle - why did you choose 20 degrees? It looks a little funny to me in the 45-degree cases where the x and y axis labels are essentially symmetric (look at x5/y5, and rotate the image 90 degrees CCW), but the x axis labels are vertically centered while the y axis labels are aligned to the first line. Symmetry would say the logic for an x axis at angle angle should match the logic for a y axis at angle 90 - angle.

Thinking also about cases where this could cause problems: the main issue I see is if you have a hard edge on the plot (either axis.showline or something like bars or a heatmap flush with the edge) particularly when you're not showing ticks. In order for multiline labels to not overlap the plot in that case, we need to make sure the angle between the vertical text and the axis is fairly small before we vertically center the text. So that would argue for a fairly small cutoff in the y case (could be 20 as you have it, or maybe 30?) and a correspondingly large cutoff in the x case (70 or 60).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had no particular reason for 20 degrees. I think I ended up with the asymmetric setup because it counteracts an existing asymmetry a bit. But that's not a good reason. 🙂 I've switched to your recommendation. (30° cutoff for y, 60° cutoff for x.)

You can see the asymmetry I mean if you look in the top right on the current baseline. Both place the anchor at the start of the third line, but the gap between the tick and the text is greater on the top than on the right. Also the gap for x 2 is greater than for y 1.

I suppose this is unrelated to this pull request though, and does not really bother my eye.

}

function positionLabels(s, angle) {
s.each(function(d) {
var anchor = labelanchor(angle, d);
Expand All @@ -2046,6 +2067,13 @@ axes.doTicks = function(gd, axid, skipTitle) {
(' rotate(' + angle + ',' + labelx(d) + ',' +
(labely(d) - d.fontSize / 2) + ')') :
'');
var anchorHeight = getAnchorHeight(
svgTextUtils.lineCount(thisLabel),
LINE_SPACING * d.fontSize,
isNumeric(angle) ? +angle : 0);
if(anchorHeight) {
transform += ' translate(0, ' + anchorHeight + ')';
}
if(mathjaxGroup.empty()) {
thisLabel.select('text').attr({
transform: transform,
Expand Down
Binary file added test/image/baselines/bar_multiline_labels.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.
161 changes: 161 additions & 0 deletions test/image/mocks/bar_multiline_labels.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
{
"data":[
{
"type": "bar",
"x":["x 1","multiple<br>lines","one<br >two<BR>three"],
"y":["x 2","multiple<br>lines","one<br >two<BR>three"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

"y 1"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"y 1"

Oops, fixed.

},
{
"type": "bar",
"x":["x 2","multiple<br>lines","one<br>two<br>three"],
"y":["y 2","multiple<br>lines","one<br>two<br>three"],
"xaxis": "x2",
"yaxis": "y2"
},
{
"type": "bar",
"x":["x 3","multiple<br>lines","one<br>two<br>three"],
"y":["y 3","multiple<br>lines","one<br>two<br>three"],
"xaxis": "x3",
"yaxis": "y3"
},
{
"type": "bar",
"x":["x 4","multiple<br>lines","one<br>two<br>three"],
"y":["y 4","multiple<br>lines","one<br>two<br>three"],
"xaxis": "x4",
"yaxis": "y4"
},
{
"type": "bar",
"orientation": "h",
"x":["x 5","multiple<br>lines","one<br>two<br>three"],
"y":["y 5","multiple<br>lines","one<br>two<br>three"],
"xaxis": "x5",
"yaxis": "y5"
},
{
"type": "bar",
"orientation": "h",
"x":["x 6","multiple<br>lines","one<br>two<br>three"],
"y":["y 6","multiple<br>lines","one<br>two<br>three"],
"xaxis": "x6",
"yaxis": "y6"
},
{
"type": "scatter",
"x":["x 7","multiple<br>lines","one<br>two<br>three"],
"y":["y 7","multiple<br>lines","one<br>two<br>three"],
"xaxis": "x7",
"yaxis": "y7"
},
{
"type": "scatter",
"x":["x 8","multiple<br>lines","one<br>two<br>three"],
"y":["y 8","multiple<br>lines","one<br>two<br>three"],
"xaxis": "x8",
"yaxis": "y8"
}
],
"layout":{
"showlegend": false,
"xaxis": {
"ticks": "outside",
"domain": [0, 0.4]
},
"yaxis": {
"ticks": "outside",
"domain": [0, 0.4]
},
"xaxis2": {
"ticks": "outside",
"domain": [0, 0.4],
"side": "top",
"anchor": "y2",
"tickangle": 90
},
"yaxis2": {
"ticks": "outside",
"domain": [0, 0.4],
"side": "right",
"tickangle": 90
},
"xaxis3": {
"ticks": "outside",
"domain": [0.6, 1],
"tickangle": 90
},
"yaxis3": {
"ticks": "outside",
"domain": [0, 0.4],
"anchor": "x3",
"tickangle": 90
},
"xaxis4": {
"ticks": "outside",
"domain": [0.6, 1],
"side": "top"
},
"yaxis4": {
"ticks": "outside",
"domain": [0, 0.4],
"side": "right",
"anchor": "x4"
},
"xaxis5": {
"ticks": "outside",
"domain": [0, 0.4],
"anchor": "y5",
"tickangle": 45
},
"yaxis5": {
"ticks": "outside",
"domain": [0.6, 1],
"anchor": "x5",
"tickangle": 45
},
"xaxis6": {
"ticks": "outside",
"domain": [0, 0.4],
"side": "top",
"anchor": "y6",
"tickangle": -90
},
"yaxis6": {
"ticks": "outside",
"domain": [0.6, 1],
"anchor": "x6",
"side": "right",
"tickangle": -90
},
"xaxis7": {
"ticks": "outside",
"domain": [0.6, 1],
"anchor": "y7",
"tickangle": -90
},
"yaxis7": {
"ticks": "outside",
"domain": [0.6, 1],
"anchor": "x7",
"tickangle": -90
},
"xaxis8": {
"ticks": "outside",
"domain": [0.6, 1],
"side": "top",
"anchor": "y8",
"tickangle": -45
},
"yaxis8": {
"ticks": "outside",
"domain": [0.6, 1],
"side": "right",
"anchor": "x8",
"tickangle": 45
},
"legend": "none",
"height":800,
"width":800
}
}