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.

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

More auto-margin fixes (some of them temporary) #4216

Merged
merged 5 commits into from
Sep 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
52 changes: 37 additions & 15 deletions src/plots/cartesian/axes.js
Original file line number Diff line number Diff line change
Expand Up @@ -1643,6 +1643,7 @@ axes.draw = function(gd, arg, opts) {
* - ax._anchorAxis
* - ax._subplotsWith
* - ax._counterDomainMin, ax._counterDomainMax (optionally, from linkSubplots)
* - ax._tickAngles (on redraw only, old value relinked during supplyDefaults)
* - ax._mainLinePosition (from lsInner)
* - ax._mainMirrorPosition
* - ax._linepositions
Expand Down Expand Up @@ -1684,6 +1685,8 @@ axes.drawOne = function(gd, ax, opts) {
// - stash tickLabels selection, so that drawTitle can use it to scoot title
ax._selections = {};
// stash tick angle (including the computed 'auto' values) per tick-label class
// linkup 'previous' tick angles on redraws
if(ax._tickAngles) ax._prevTickAngles = ax._tickAngles;
ax._tickAngles = {};
// measure [in px] between axis position and outward-most part of bounding box
// (touching either the tick label or ticks)
Expand Down Expand Up @@ -1897,12 +1900,12 @@ axes.drawOne = function(gd, ax, opts) {
if(llbbox.width > 0) {
var rExtra = llbbox.right - (ax._offset + ax._length);
if(rExtra > 0) {
push.x = 1;
push.xr = 1;
push.r = rExtra;
}
var lExtra = ax._offset - llbbox.left;
if(lExtra > 0) {
push.x = 0;
push.xl = 0;
push.l = lExtra;
}
}
Expand All @@ -1917,12 +1920,12 @@ axes.drawOne = function(gd, ax, opts) {
if(llbbox.height > 0) {
var bExtra = llbbox.bottom - (ax._offset + ax._length);
if(bExtra > 0) {
push.y = 0;
push.yb = 0;
push.b = bExtra;
}
var tExtra = ax._offset - llbbox.top;
if(tExtra > 0) {
push.y = 1;
push.yt = 1;
push.t = tExtra;
}
}
Expand Down Expand Up @@ -2400,6 +2403,7 @@ axes.drawZeroLine = function(gd, ax, opts) {
* - {number} tickangle
* - {object (optional)} _selections
* - {object} (optional)} _tickAngles
* - {object} (optional)} _prevTickAngles
* @param {object} opts
* - {array of object} vals (calcTicks output-like)
* - {d3 selection} layer
Expand All @@ -2416,13 +2420,14 @@ axes.drawZeroLine = function(gd, ax, opts) {
axes.drawLabels = function(gd, ax, opts) {
opts = opts || {};

var fullLayout = gd._fullLayout;
var axId = ax._id;
var axLetter = axId.charAt(0);
var cls = opts.cls || axId + 'tick';
var vals = opts.vals;
var labelFns = opts.labelFns;
var tickAngle = opts.secondary ? 0 : ax.tickangle;
var lastAngle = (ax._tickAngles || {})[cls];
var prevAngle = (ax._prevTickAngles || {})[cls];

var tickLabels = opts.layer.selectAll('g.' + cls)
.data(ax.showticklabels ? vals : [], tickDataFn);
Expand Down Expand Up @@ -2507,17 +2512,17 @@ axes.drawLabels = function(gd, ax, opts) {
// do this without waiting, using the last calculated angle to
// minimize flicker, then do it again when we know all labels are
// there, putting back the prescribed angle to check for overlaps.
positionLabels(tickLabels, lastAngle || tickAngle);
positionLabels(tickLabels, (prevAngle + 1) ? prevAngle : tickAngle);

function allLabelsReady() {
return labelsReady.length && Promise.all(labelsReady);
}

var autoangle = null;

function fixLabelOverlaps() {
positionLabels(tickLabels, tickAngle);

var autoangle = null;

// check for auto-angling if x labels overlap
// don't auto-angle at all for log axes with
// base and digit format
Expand Down Expand Up @@ -2584,19 +2589,36 @@ axes.drawLabels = function(gd, ax, opts) {
positionLabels(tickLabels, autoangle);
}
}

if(ax._tickAngles) {
ax._tickAngles[cls] = autoangle === null ?
(isNumeric(tickAngle) ? tickAngle : 0) :
autoangle;
}
}

if(ax._selections) {
ax._selections[cls] = tickLabels;
}

var done = Lib.syncOrAsync([allLabelsReady, fixLabelOverlaps]);
var seq = [allLabelsReady];

// N.B. during auto-margin redraws, if the axis fixed its label overlaps
// by rotating 90 degrees, do not attempt to re-fix its label overlaps
// as this can lead to infinite redraw loops!
if(ax.automargin && fullLayout._redrawFromAutoMarginCount && prevAngle === 90) {
autoangle = 90;
seq.push(function() {
archmoj marked this conversation as resolved.
Show resolved Hide resolved
positionLabels(tickLabels, prevAngle);
});
} else {
seq.push(fixLabelOverlaps);
}

// save current tick angle for future redraws
if(ax._tickAngles) {
seq.push(function() {
ax._tickAngles[cls] = autoangle === null ?
(isNumeric(tickAngle) ? tickAngle : 0) :
autoangle;
});
}

var done = Lib.syncOrAsync(seq);
if(done && done.then) gd._promises.push(done);
return done;
};
Expand Down
14 changes: 13 additions & 1 deletion src/plots/plots.js
Original file line number Diff line number Diff line change
Expand Up @@ -1960,7 +1960,19 @@ plots.doAutoMargin = function(gd) {
} else {
fullLayout._redrawFromAutoMarginCount = 1;
}
return Registry.call('plot', gd);

// Always allow at least one redraw and give each margin-push
// call 3 loops to converge. Of course, for most cases this way too many,
// but let's keep things on the safe side until we fix our
// auto-margin pipeline problems:
// https://github.com/plotly/plotly.js/issues/2704
var maxNumberOfRedraws = 3 * (1 + Object.keys(pushMarginIds).length);

if(fullLayout._redrawFromAutoMarginCount < maxNumberOfRedraws) {
return Registry.call('plot', gd);
} else {
Lib.warn('Too many auto-margin redraws.');
}
}
};

Expand Down
Binary file added test/image/baselines/automargin-small-width.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
21 changes: 21 additions & 0 deletions test/image/mocks/automargin-small-width.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"data": [
{
"type": "bar",
"x": [ "Montreal", "Tofu Bowl", "Tropical Beaches" ],
"y": [ 3, 1, 2 ]
}
],
"layout": {
"margin": {
"l": 20,
"r": 0,
"b": 30,
"t": 15
},
"xaxis": {
"automargin": true
},
"width": 150
}
}