Skip to content
Merged
14 changes: 7 additions & 7 deletions src/plot_api/plot_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -1689,12 +1689,9 @@ exports.relayout = function relayout(gd, astr, val) {
// subroutine of its own so that finalDraw always gets
// executed after drawData
seq.push(
// TODO
// can target specific axes,
// do not have to redraw all axes here
// See:
// https://github.com/plotly/plotly.js/issues/2547
subroutines.doTicksRelayout,
function(gd) {
return subroutines.doTicksRelayout(gd, specs.rangesAltered);
},
subroutines.drawData,
subroutines.finalDraw
);
Expand Down Expand Up @@ -2058,6 +2055,7 @@ function _relayout(gd, aobj) {

return {
flags: flags,
rangesAltered: rangesAltered,
undoit: undoit,
redoit: redoit,
eventData: Lib.extendDeep({}, redoit)
Expand Down Expand Up @@ -2171,7 +2169,9 @@ exports.update = function update(gd, traceUpdate, layoutUpdate, _traces) {
if(relayoutFlags.layoutstyle) seq.push(subroutines.layoutStyles);
if(relayoutFlags.axrange) {
seq.push(
subroutines.doTicksRelayout,
function(gd) {
return subroutines.doTicksRelayout(gd, relayoutSpecs.rangesAltered);
Copy link
Contributor Author

@etpinard etpinard May 11, 2018

Choose a reason for hiding this comment

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

N.B. @alexcjohnson I didn't implement per-axis "axrange" edit for Plotly.react. My main concern was improvement perf on dragend axis range relayout calls. I didn't think adding an exception to the react diffing algo was worth it for this case.

},
subroutines.drawData,
subroutines.finalDraw
);
Expand Down
8 changes: 6 additions & 2 deletions src/plot_api/subroutines.js
Original file line number Diff line number Diff line change
Expand Up @@ -448,8 +448,12 @@ exports.doLegend = function(gd) {
return Plots.previousPromises(gd);
};

exports.doTicksRelayout = function(gd) {
Axes.doTicks(gd, 'redraw');
exports.doTicksRelayout = function(gd, rangesAltered) {
if(rangesAltered) {
Axes.doTicks(gd, Object.keys(rangesAltered), true);
} else {
Axes.doTicks(gd, 'redraw');
}

if(gd._fullLayout._hasOnlyLargeSploms) {
clearGlCanvases(gd);
Expand Down
80 changes: 52 additions & 28 deletions src/plots/cartesian/axes.js
Original file line number Diff line number Diff line change
Expand Up @@ -1498,28 +1498,50 @@ axes.makeClipPaths = function(gd) {
});
};

// doTicks: draw ticks, grids, and tick labels
// axid: 'x', 'y', 'x2' etc,
// blank to do all,
// 'redraw' to force full redraw, and reset:
// ax._r (stored range for use by zoom/pan)
// ax._rl (stored linearized range for use by zoom/pan)
// or can pass in an axis object directly
axes.doTicks = function(gd, axid, skipTitle) {
/** Main axis drawing routine!
*
* This routine draws axis ticks and much more (... grids, labels, title etc.)
* Supports multiple argument signatures.
* N.B. this thing is async in general (because of MathJax rendering)
*
* @param {DOM element} gd : graph div
* @param {object or string or array of strings} arg : polymorphic argument
* @param {boolean} skipTitle : optional flag to skip axis title draw/update
* @return {promise}
*
* Signature 1: Axes.doTicks(gd, ax)
* where ax is an axis object as in fullLayout
*
* Signature 2: Axes.doTicks(gd, axId)
* where axId is a axis id string
*
* Signature 3: Axes.doTicks(gd, 'redraw')
* use this to clear and redraw all axes on graph
*
* Signature 4: Axes.doTicks(gd, '')
* use this to draw all axes on graph w/o the selectAll().remove()
* of the 'redraw' signature
*
* Signature 5: Axes.doTicks(gd, [axId, axId2, ...])
* where the items are axis id string,
* use this to update multiple axes in one call
Copy link
Collaborator

Choose a reason for hiding this comment

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

This was already a bit out of control, do we really want to add another signature?

Can we perhaps break it into 2 separate functions, one for signatures 1 & 2 and a second for 3, 4, 5? That's how it functions anyway - 3,4,5 just map into a bunch of calls to signature 2 (ignoring skipTitle) and then return.

Copy link
Contributor Author

@etpinard etpinard May 11, 2018

Choose a reason for hiding this comment

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

Sure that would be nice. But since doTicks is one of our most referenced function in existing issues, I'm a little reluctant to do so.

But if you insist, than I'm thinking of making Axes.doTicks the multi-cartesian-axis version (that accepts '', 'redraw' and arrays of axis ids), and adding Axes.doTicksSingle (that accepts one axis id string or an axis object).

On master, we have:

image

*
* N.B signatures 3, 4 and 5 reset:
* - ax._r (stored range for use by zoom/pan)
* - ax._rl (stored linearized range for use by zoom/pan)
*/
axes.doTicks = function(gd, arg, skipTitle) {
var fullLayout = gd._fullLayout;
var ax;
var independent = false;
var ax;

// allow passing an independent axis object instead of id
if(typeof axid === 'object') {
ax = axid;
axid = ax._id;
if(Lib.isPlainObject(arg)) {
ax = arg;
independent = true;
}
else {
ax = axes.getFromId(gd, axid);
} else if(!arg || arg === 'redraw' || Array.isArray(arg)) {
var axList = (!arg || arg === 'redraw') ? axes.listIds(gd) : arg;

if(axid === 'redraw') {
if(arg === 'redraw') {
fullLayout._paper.selectAll('g.subplot').each(function(subplot) {
var plotinfo = fullLayout._plots[subplot];
var xa = plotinfo.xaxis;
Expand All @@ -1534,22 +1556,24 @@ axes.doTicks = function(gd, axid, skipTitle) {
});
}

if(!axid || axid === 'redraw') {
return Lib.syncOrAsync(axes.list(gd, '', true).map(function(ax) {
return function() {
if(!ax._id) return;
var axDone = axes.doTicks(gd, ax._id);
ax._r = ax.range.slice();
ax._rl = Lib.simpleMap(ax._r, ax.r2l);
return axDone;
};
}));
}
return Lib.syncOrAsync(axList.map(function(a) {
return function() {
if(!a) return;
var axDone = axes.doTicks(gd, a);
var ax = axes.getFromId(gd, a);
ax._r = ax.range.slice();
ax._rl = Lib.simpleMap(ax._r, ax.r2l);
return axDone;
};
}));
} else {
ax = axes.getFromId(gd, arg);
}

// set scaling to pixels
ax.setScale();

var axid = ax._id;
var axLetter = axid.charAt(0);
var counterLetter = axes.counterLetter(axid);
var vals = ax._vals = axes.calcTicks(ax);
Expand Down