Skip to content

automatic margins for long labels #2243

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 22 commits into from
Mar 2, 2018
Merged

automatic margins for long labels #2243

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
6797c9d
first cut at automatic margins for long labels
nicolaskruchten Jan 10, 2018
f83fb55
:hocho: test code
nicolaskruchten Jan 10, 2018
eda678d
proper fix for top/right axes
nicolaskruchten Jan 22, 2018
e86cfea
subplot-compatible generalization
nicolaskruchten Jan 22, 2018
8a5de93
Merge branch 'master' into label_automargin
nicolaskruchten Jan 29, 2018
0ef6009
creating default-false parameter
nicolaskruchten Jan 29, 2018
412941c
option guard
nicolaskruchten Jan 29, 2018
bc0cf06
removing package-lock.json
nicolaskruchten Jan 30, 2018
b2d49d5
Merge branch 'master' into label_automargin
nicolaskruchten Jan 30, 2018
25a36db
adding baseline, fixing coerce check
nicolaskruchten Jan 31, 2018
d89f23e
moving coercion around
nicolaskruchten Feb 1, 2018
8d0a5eb
Merge branch 'master' into label_automargin
nicolaskruchten Feb 1, 2018
67b04e9
ticklabelsautomargin -> automargin
nicolaskruchten Feb 1, 2018
05b909a
Merge branch 'master' into label_automargin
nicolaskruchten Feb 27, 2018
a05bd81
first relayout test
nicolaskruchten Feb 28, 2018
eeb1f65
make relayout test relative, add guards
nicolaskruchten Mar 1, 2018
c1f7df4
more guards
nicolaskruchten Mar 1, 2018
48da987
Merge branch 'master' into label_automargin
nicolaskruchten Mar 1, 2018
504d133
axes push according to the domain of their anchor
nicolaskruchten Mar 1, 2018
ae6ec95
formatting of mock
nicolaskruchten Mar 1, 2018
20bdefa
exclude polar and ternary from automargins
nicolaskruchten Mar 2, 2018
1df34bd
add editType to clear automargins from pushmargins
nicolaskruchten Mar 2, 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
20 changes: 19 additions & 1 deletion src/plots/cartesian/axes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

var d3 = require('d3');
var isNumeric = require('fast-isnumeric');
var Plots = require('../../plots/plots');

var Registry = require('../../registry');
var Lib = require('../../lib');
Expand Down Expand Up @@ -2220,10 +2221,27 @@ axes.doTicks = function(gd, axid, skipTitle) {
}
}

function doAutoMargins() {
if(!ax.ticklabelsautomargin) { return; }
var marginPush = ax.titlefont.size +
(axLetter === 'x' ? ax._boundingBox.height : ax._boundingBox.width);

if(!ax._marginPush || ax._marginPush < marginPush) {
ax._marginPush = marginPush;
var pushParams = {
x: ax.side[0] === 'r' ? ax.domain[1] : ax.domain[0],
y: ax.side[0] === 't' ? ax.domain[1] : ax.domain[0],
r: 0, l: 0, t: 0, b: 0};
pushParams[ax.side[0]] = marginPush;
Plots.autoMargin(gd, ax._name, pushParams);
}
}

var done = Lib.syncOrAsync([
allLabelsReady,
fixLabelOverlaps,
calcBoundingBox
calcBoundingBox,
doAutoMargins
]);
if(done && done.then) gd._promises.push(done);
return done;
Expand Down
10 changes: 10 additions & 0 deletions src/plots/cartesian/layout_attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,16 @@ module.exports = {
editType: 'ticks',
description: 'Determines whether or not the tick labels are drawn.'
},
ticklabelsautomargin: {
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be too verbose. Would (x|y)axis.automargin suffice? Or maybe tickautomargin? Or perhaps we're looking at this the wrong way, something like:

var layout = {
  automargin: true || false,
  automarginmode: 'all' || 'ticks' || 'title' || 'xaxis.title' || 'legend' || ...
  // or any combination of these using '+',  
  // a 'flaglist' attribute like scatter 'mode'
}

might scale better.

@alexcjohnson @cldougl @chriddyp any thoughts on this?

Copy link
Member

Choose a reason for hiding this comment

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

+1 for automargin / automarginmode or at least (x|y)axis.automargin - this is more clear if attributes other than ticks/tick labels contributes to this (now or in the future).

valType: 'boolean',
dflt: false,
role: 'style',
editType: 'ticks',
description: [
'Determines whether long tick labels automatically grow the figure',
'margins.'
].join(' ')
},
showspikes: {
valType: 'boolean',
dflt: false,
Expand Down
2 changes: 2 additions & 0 deletions src/plots/cartesian/tick_label_defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ module.exports = function handleTickLabelDefaults(containerIn, containerOut, coe
}

if(axType !== 'category' && !options.noHover) coerce('hoverformat');

if(axType === 'cartesian') coerce('ticklabelsautomargin');
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh this won't work. axType is either 'linear', 'date', 'log' or 'category'.

I think the easiest way would to add an options.automargin flag set to true when called from plots/cartesian/axis_defaults.js.

Copy link
Contributor

@etpinard etpinard Jan 31, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Oh and does coerce('ticklabelsautomargin') have an effect when showticklabels is false? More precisely, do the axis title font settings and/or ticklen affect the auto-margin computations?

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'm not sure what coerce actually does, to be honest, so I'm not sure if it will have an impact. What exactly does it do? Is it necessary here?

To answer your question more generally, the auto-margin calculation explicitly takes the axis title font size into account, and for the rest relies on the pre-existing bounding-box calculation.

Copy link
Contributor

@etpinard etpinard Jan 31, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure what coerce actually does,

In brief, coerce fills in fullLayout with a sanitise version of its user layout value. Almost everywhere downstream of the Plots.supplyDefaults uses fullLayout and fullData value (i.e. not gd.layout and gd.data) including your new logic in axes.js.

So, yes it is necessary 😄

the auto-margin calculation explicitly takes the axis title font size into account,

Ok great. So, auto-margin doesn't just depend on tick settings like the attribute name ticklabelsautomargin is suggesting. We'll need to think about how to name this new attribute some more cc #2243 (comment)

for the rest relies on the pre-existing bounding-box calculation.

Ok, so this should depend on ticklen (we should double-check though). Knowing which attributes impact the auto-margin computation is important to make sure relayout calls like Plotly.relayout(gd, 'xaxis.ticklen', /**/) update the (auto) margin when ticklabelsautomargin is turned on.

};

/*
Expand Down
37 changes: 37 additions & 0 deletions test/image/mocks/long_axis_labels.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
{
"data": [{
"type": "scatter",
"name": "loooooong x",
"x": [
"short label 1", "loooooong label 1",
"short label 2", "loooooong label 2",
"short label 3", "loooooong label 3",
"short label 4", "loooooongloooooongloooooong label 4",
"short label 5", "loooooong label 5"
],
"y": [
"short label 1", "loooooong label 1",
"short label 2", "loooooong label 2",
"short label 3", "loooooong label 3",
"short label 4", "loooooong label 4",
"short label 5", "loooooong label 5"
]
},
{
"yaxis": "y2",
"type": "scatter",
"name": "loooooong y",
"x":["looooooooooooonger"],
"y":["loooooooo"]
}
],
"layout": {
"xaxis": {"title": "X Axis Title", "titlefont": {"size": 80}, "ticklabelsautomargin": true},
"yaxis": {"title": "Y Axis Title", "titlefont": {"size": 40}, "ticklabelsautomargin": true},
"yaxis2": {
"title": "Y2 Axis Title",
"overlaying": "y",
"side": "right", "titlefont": {"size": 40}, "ticklabelsautomargin": true
}
}
}