Skip to content

Fix overlaying subplot configuration relayouts #2831

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

Merged
merged 6 commits into from
Jul 23, 2018
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
6 changes: 4 additions & 2 deletions src/plot_api/subroutines.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ function lsInner(gd) {
// to put them
var lowerBackgroundIDs = [];
var lowerDomains = [];
subplotSelection.each(function(subplot) {
subplotSelection.each(function(d) {
var subplot = d[0];
var plotinfo = fullLayout._plots[subplot];

if(plotinfo.mainplot) {
Expand Down Expand Up @@ -161,7 +162,8 @@ function lsInner(gd) {
fullLayout._plots[subplot].bg = d3.select(this);
});

subplotSelection.each(function(subplot) {
subplotSelection.each(function(d) {
var subplot = d[0];
var plotinfo = fullLayout._plots[subplot];
var xa = plotinfo.xaxis;
var ya = plotinfo.yaxis;
Expand Down
13 changes: 8 additions & 5 deletions src/plots/cartesian/axes.js
Original file line number Diff line number Diff line change
Expand Up @@ -1500,7 +1500,8 @@ axes.makeClipPaths = function(gd) {
});
};

/** Main multi-axis drawing routine!
/**
* Main multi-axis drawing routine!
*
* @param {DOM element} gd : graph div
* @param {string or array of strings} arg : polymorphic argument
Expand All @@ -1525,8 +1526,9 @@ axes.doTicks = function(gd, arg, skipTitle) {
var fullLayout = gd._fullLayout;

if(arg === 'redraw') {
fullLayout._paper.selectAll('g.subplot').each(function(subplot) {
var plotinfo = fullLayout._plots[subplot];
fullLayout._paper.selectAll('g.subplot').each(function(d) {
var id = d[0];
var plotinfo = fullLayout._plots[id];
var xa = plotinfo.xaxis;
var ya = plotinfo.yaxis;

Expand Down Expand Up @@ -1556,14 +1558,15 @@ axes.doTicks = function(gd, arg, skipTitle) {
}));
};

/** Per axis drawing routine!
/**
* Per-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 {string or array of strings} arg : polymorphic argument
* @param {string or object} arg : polymorphic argument
* @param {boolean} skipTitle : optional flag to skip axis title draw/update
* @return {promise}
*
Expand Down
2 changes: 1 addition & 1 deletion src/plots/cartesian/graph_interact.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ exports.initInteractions = function initInteractions(gd) {
return;
}

if(!fullLayout._has('cartesian') && !fullLayout._has('gl2d') && !fullLayout._has('splom')) return;
if(!fullLayout._has('cartesian') && !fullLayout._has('splom')) return;

var subplots = Object.keys(fullLayout._plots || {}).sort(function(a, b) {
// sort overlays last, then by x axis number, then y axis number
Expand Down
82 changes: 48 additions & 34 deletions src/plots/cartesian/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -358,37 +358,27 @@ exports.drawFramework = function(gd) {
var subplotData = makeSubplotData(gd);

var subplotLayers = fullLayout._cartesianlayer.selectAll('.subplot')
.data(subplotData, Lib.identity);
.data(subplotData, String);

subplotLayers.enter().append('g')
.attr('class', function(name) { return 'subplot ' + name; });
.attr('class', function(d) { return 'subplot ' + d[0]; });

subplotLayers.order();

subplotLayers.exit()
.call(purgeSubplotLayers, fullLayout);

subplotLayers.each(function(name) {
var plotinfo = fullLayout._plots[name];
subplotLayers.each(function(d) {
var id = d[0];
var plotinfo = fullLayout._plots[id];

// keep ref to plot group
plotinfo.plotgroup = d3.select(this);

// initialize list of overlay subplots
plotinfo.overlays = [];

makeSubplotLayer(gd, plotinfo);

// fill in list of overlay subplots
if(plotinfo.mainplot) {
var mainplot = fullLayout._plots[plotinfo.mainplot];
mainplot.overlays.push(plotinfo);
}

// make separate drag layers for each subplot,
// but append them to paper rather than the plot groups,
// so they end up on top of the rest
plotinfo.draglayer = ensureSingle(fullLayout._draggers, 'g', name);
plotinfo.draglayer = ensureSingle(fullLayout._draggers, 'g', id);
});
};

Expand All @@ -400,27 +390,49 @@ exports.rangePlot = function(gd, plotinfo, cdSubplot) {

function makeSubplotData(gd) {
var fullLayout = gd._fullLayout;
var subplotData = [];
var overlays = [];

for(var k in fullLayout._plots) {
var plotinfo = fullLayout._plots[k];
var xa2 = plotinfo.xaxis._mainAxis;
var ya2 = plotinfo.yaxis._mainAxis;
var ids = fullLayout._subplots.cartesian;
var len = ids.length;
var subplotData = new Array(len);
var i, j, id, plotinfo, xa, ya;

for(i = 0; i < len; i++) {
id = ids[i];
plotinfo = fullLayout._plots[id];
xa = plotinfo.xaxis;
ya = plotinfo.yaxis;

var xa2 = xa._mainAxis;
var ya2 = ya._mainAxis;
var mainplot = xa2._id + ya2._id;
var mainplotinfo = fullLayout._plots[mainplot];
plotinfo.overlays = [];

if(mainplot !== k && fullLayout._plots[mainplot]) {
if(mainplot !== id && mainplotinfo) {
// link 'main plot' ref in overlaying plotinfo
plotinfo.mainplot = mainplot;
plotinfo.mainplotinfo = fullLayout._plots[mainplot];
overlays.push(k);
plotinfo.mainplotinfo = mainplotinfo;
// fill in list of overlaying subplots in 'main plot'
mainplotinfo.overlays.push(plotinfo);
} else {
subplotData.push(k);
plotinfo.mainplot = undefined;
plotinfo.mainPlotinfo = undefined;
}
}

// main subplots before overlays
subplotData = subplotData.concat(overlays);
// use info about axis layer and overlaying pattern
// to clean what need to be cleaned up in exit selection
for(i = 0; i < len; i++) {
id = ids[i];
plotinfo = fullLayout._plots[id];
xa = plotinfo.xaxis;
ya = plotinfo.yaxis;

var d = [id, xa.layer, ya.layer, xa.overlaying || '', ya.overlaying || ''];
for(j = 0; j < plotinfo.overlays.length; j++) {
d.push(plotinfo.overlays[j].id);
}
subplotData[i] = d;
}

return subplotData;
}
Expand Down Expand Up @@ -517,7 +529,9 @@ function makeSubplotLayer(gd, plotinfo) {
if(!hasOnlyLargeSploms) {
ensureSingleAndAddDatum(plotinfo.gridlayer, 'g', plotinfo.xaxis._id);
ensureSingleAndAddDatum(plotinfo.gridlayer, 'g', plotinfo.yaxis._id);
plotinfo.gridlayer.selectAll('g').sort(axisIds.idSort);
plotinfo.gridlayer.selectAll('g')
.map(function(d) { return d[0]; })
.sort(axisIds.idSort);
}

plotinfo.xlines
Expand All @@ -534,13 +548,13 @@ function purgeSubplotLayers(layers, fullLayout) {

var overlayIdsToRemove = {};

layers.each(function(subplotId) {
layers.each(function(d) {
var id = d[0];
var plotgroup = d3.select(this);

plotgroup.remove();
removeSubplotExtras(subplotId, fullLayout);

overlayIdsToRemove[subplotId] = true;
removeSubplotExtras(id, fullLayout);
overlayIdsToRemove[id] = true;

// do not remove individual axis <clipPath>s here
// as other subplots may need them
Expand Down
20 changes: 6 additions & 14 deletions src/plots/plots.js
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,8 @@ plots.cleanPlot = function(newFullData, newFullLayout, oldFullData, oldFullLayou
};

plots.linkSubplots = function(newFullData, newFullLayout, oldFullData, oldFullLayout) {
var i, j;

var oldSubplots = oldFullLayout._plots || {};
var newSubplots = newFullLayout._plots = {};
var newSubplotList = newFullLayout._subplots;
Expand All @@ -768,32 +770,22 @@ plots.linkSubplots = function(newFullData, newFullLayout, oldFullData, oldFullLa

var ids = newSubplotList.cartesian.concat(newSubplotList.gl2d || []);

var i, j, id, ax;

for(i = 0; i < ids.length; i++) {
id = ids[i];
var id = ids[i];
var oldSubplot = oldSubplots[id];
var xaxis = axisIDs.getFromId(mockGd, id, 'x');
var yaxis = axisIDs.getFromId(mockGd, id, 'y');
var plotinfo;

// link or create subplot object
if(oldSubplot) {
plotinfo = newSubplots[id] = oldSubplot;

if(plotinfo.xaxis.layer !== xaxis.layer) {
plotinfo.xlines.attr('d', null);
plotinfo.xaxislayer.selectAll('*').remove();
}

if(plotinfo.yaxis.layer !== yaxis.layer) {
plotinfo.ylines.attr('d', null);
plotinfo.yaxislayer.selectAll('*').remove();
}
} else {
plotinfo = newSubplots[id] = {};
plotinfo.id = id;
}

// update x and y axis layout object refs
plotinfo.xaxis = xaxis;
plotinfo.yaxis = yaxis;

Expand Down Expand Up @@ -821,7 +813,7 @@ plots.linkSubplots = function(newFullData, newFullLayout, oldFullData, oldFullLa
// anchored axes to the axes they're anchored to
var axList = axisIDs.list(mockGd, null, true);
for(i = 0; i < axList.length; i++) {
ax = axList[i];
var ax = axList[i];
var mainAx = null;

if(ax.overlaying) {
Expand Down
82 changes: 82 additions & 0 deletions test/jasmine/tests/cartesian_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,88 @@ describe('subplot creation / deletion:', function() {
.then(done);
});

it('should clear obsolete content out of axis layers when changing overlaying configuation', function(done) {
function data() {
return [
{x: [1, 2], y: [1, 2]},
{x: [1, 2], y: [1, 2], xaxis: 'x2', yaxis: 'y2'}
];
}

function fig0() {
return {
data: data(),
layout: {
xaxis2: {side: 'top', overlaying: 'x'},
yaxis2: {side: 'right', overlaying: 'y'}
}
};
}

function fig1() {
return {
data: data(),
layout: {
xaxis2: {side: 'top', domain: [0.37, 1]},
yaxis2: {side: 'right', overlaying: 'y'}
}
};
}

function getParentClassName(query, level) {
level = level || 1;
var cl = gd.querySelector('g.cartesianlayer');
var node = cl.querySelector(query);
while(level--) node = node.parentNode;
return node.getAttribute('class');
}

function _assert(msg, exp) {
expect(getParentClassName('.xtick'))
.toBe(exp.xtickParent, 'xitck parent - ' + msg);
expect(getParentClassName('.x2tick'))
.toBe(exp.x2tickParent, 'x2tick parent - ' + msg);
expect(getParentClassName('.trace' + gd._fullData[0].uid, 2))
.toBe(exp.trace0Parent, 'data[0] parent - ' + msg);
expect(getParentClassName('.trace' + gd._fullData[1].uid, 2))
.toBe(exp.trace1Parent, 'data[1] parent - ' + msg);
}

Plotly.react(gd, fig0())
.then(function() {
_assert('x2/y2 both overlays', {
xtickParent: 'xaxislayer-above',
x2tickParent: 'x2y2-x',
trace0Parent: 'plot',
trace1Parent: 'x2y2'
});
})
.then(function() {
return Plotly.react(gd, fig1());
})
.then(function() {
_assert('x2 free / y2 overlaid', {
xtickParent: 'xaxislayer-above',
x2tickParent: 'xaxislayer-above',
trace0Parent: 'plot',
trace1Parent: 'plot'
});
})
.then(function() {
return Plotly.react(gd, fig0());
})
.then(function() {
_assert('back to x2/y2 both overlays', {
xtickParent: 'xaxislayer-above',
x2tickParent: 'x2y2-x',
trace0Parent: 'plot',
trace1Parent: 'x2y2'
});
})
.catch(failTest)
.then(done);
});

it('clear axis ticks, labels and title when relayout an axis to `*visible:false*', function(done) {
function _assert(xaxis, yaxis) {
var g = d3.select('.subplot.xy');
Expand Down
2 changes: 1 addition & 1 deletion test/jasmine/tests/splom_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ describe('@gl Test splom interactions:', function() {
subplots.each(function(d, i) {
var actual = this.children.length;
var expected = typeof exp.innerSubplotNodeCnt === 'function' ?
exp.innerSubplotNodeCnt(d, i) :
exp.innerSubplotNodeCnt(d[0], i) :
exp.innerSubplotNodeCnt;
if(actual !== expected) {
failedSubplots.push([d, actual, 'vs', expected].join(' '));
Expand Down