From c877c27ac523466007a4df7f992346757a16124f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 15 Sep 2016 17:12:18 -0400 Subject: [PATCH 01/15] move subplot creation from makePlotFramework -> Cartesian.plot --- src/plot_api/plot_api.js | 174 +------------------------------- src/plots/cartesian/index.js | 186 +++++++++++++++++++++++++++++++++++ 2 files changed, 188 insertions(+), 172 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 1b627b49bc1..63534438b54 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -35,8 +35,6 @@ var subroutines = require('./subroutines'); /** * Main plot-creation function * - * Note: will call makePlotFramework if necessary to create the framework - * * @param {string id or DOM element} gd * the id or DOM element of the graph container div * @param {array of objects} data @@ -2720,21 +2718,12 @@ function makePlotFramework(gd) { fullLayout._shapeLowerLayer = layerBelow.append('g') .classed('shapelayer', true); - var subplots = Plotly.Axes.getSubplots(gd); - if(subplots.join('') !== Object.keys(gd._fullLayout._plots || {}).join('')) { - makeSubplots(gd, subplots); - } - - if(fullLayout._has('cartesian')) makeCartesianPlotFramwork(gd, subplots); + // single cartesian layer for the whole plot + fullLayout._cartesianlayer = fullLayout._paper.append('g').classed('cartesianlayer', true); // single ternary layer for the whole plot fullLayout._ternarylayer = fullLayout._paper.append('g').classed('ternarylayer', true); - // shape layers in subplots - var layerSubplot = fullLayout._paper.selectAll('.layer-subplot'); - fullLayout._imageSubplotLayer = layerSubplot.selectAll('.imagelayer'); - fullLayout._shapeSubplotLayer = layerSubplot.selectAll('.shapelayer'); - // upper shape layer // (only for shapes to be drawn above the whole plot, including subplots) var layerAbove = fullLayout._paper.append('g') @@ -2773,162 +2762,3 @@ function makePlotFramework(gd) { return frameWorkDone; } - -// create '_plots' object grouping x/y axes into subplots -// to be better manage subplots -function makeSubplots(gd, subplots) { - var _plots = gd._fullLayout._plots = {}; - var subplot, plotinfo; - - function getAxisFunc(subplot, axLetter) { - return function() { - return Plotly.Axes.getFromId(gd, subplot, axLetter); - }; - } - - for(var i = 0; i < subplots.length; i++) { - subplot = subplots[i]; - plotinfo = _plots[subplot] = {}; - - plotinfo.id = subplot; - - // references to the axis objects controlling this subplot - plotinfo.x = getAxisFunc(subplot, 'x'); - plotinfo.y = getAxisFunc(subplot, 'y'); - - // TODO investigate why replacing calls to .x and .y - // for .xaxis and .yaxis makes the `pseudo_html` - // test image fail - plotinfo.xaxis = plotinfo.x(); - plotinfo.yaxis = plotinfo.y(); - } -} - -function makeCartesianPlotFramwork(gd, subplots) { - var fullLayout = gd._fullLayout; - - // Layers to keep plot types in the right order. - // from back to front: - // 1. heatmaps, 2D histos and contour maps - // 2. bars / 1D histos - // 3. errorbars for bars and scatter - // 4. scatter - // 5. box plots - function plotLayers(svg) { - svg.append('g').classed('imagelayer', true); - svg.append('g').classed('maplayer', true); - svg.append('g').classed('barlayer', true); - svg.append('g').classed('boxlayer', true); - svg.append('g').classed('scatterlayer', true); - } - - // create all the layers in order, so we know they'll stay in order - var overlays = []; - - fullLayout._paper.selectAll('g.subplot').data(subplots) - .enter().append('g') - .classed('subplot', true) - .each(function(subplot) { - var plotinfo = fullLayout._plots[subplot], - plotgroup = plotinfo.plotgroup = d3.select(this).classed(subplot, true), - xa = plotinfo.xaxis, - ya = plotinfo.yaxis; - - // references to any subplots overlaid on this one - plotinfo.overlays = []; - - // is this subplot overlaid on another? - // ax.overlaying is the id of another axis of the same - // dimension that this one overlays to be an overlaid subplot, - // the main plot must exist make sure we're not trying to - // overlay on an axis that's already overlaying another - var xa2 = Plotly.Axes.getFromId(gd, xa.overlaying) || xa; - if(xa2 !== xa && xa2.overlaying) { - xa2 = xa; - xa.overlaying = false; - } - - var ya2 = Plotly.Axes.getFromId(gd, ya.overlaying) || ya; - if(ya2 !== ya && ya2.overlaying) { - ya2 = ya; - ya.overlaying = false; - } - - var mainplot = xa2._id + ya2._id; - if(mainplot !== subplot && subplots.indexOf(mainplot) !== -1) { - plotinfo.mainplot = mainplot; - overlays.push(plotinfo); - - // for now force overlays to overlay completely... so they - // can drag together correctly and share backgrounds. - // Later perhaps we make separate axis domain and - // tick/line domain or something, so they can still share - // the (possibly larger) dragger and background but don't - // have to both be drawn over that whole domain - xa.domain = xa2.domain.slice(); - ya.domain = ya2.domain.slice(); - } - else { - // main subplot - make the components of - // the plot and containers for overlays - plotinfo.bg = plotgroup.append('rect') - .style('stroke-width', 0); - - // back layer for shapes and images to - // be drawn below a subplot - var backlayer = plotgroup.append('g') - .classed('layer-subplot', true); - - plotinfo.shapelayer = backlayer.append('g') - .classed('shapelayer', true); - plotinfo.imagelayer = backlayer.append('g') - .classed('imagelayer', true); - plotinfo.gridlayer = plotgroup.append('g'); - plotinfo.overgrid = plotgroup.append('g'); - plotinfo.zerolinelayer = plotgroup.append('g'); - plotinfo.overzero = plotgroup.append('g'); - plotinfo.plot = plotgroup.append('g').call(plotLayers); - plotinfo.overplot = plotgroup.append('g'); - plotinfo.xlines = plotgroup.append('path'); - plotinfo.ylines = plotgroup.append('path'); - plotinfo.overlines = plotgroup.append('g'); - plotinfo.xaxislayer = plotgroup.append('g'); - plotinfo.yaxislayer = plotgroup.append('g'); - plotinfo.overaxes = plotgroup.append('g'); - - // 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 = fullLayout._draggers.append('g'); - }); - - // now make the components of overlaid subplots - // overlays don't have backgrounds, and append all - // their other components to the corresponding - // extra groups of their main Plots. - overlays.forEach(function(plotinfo) { - var mainplot = fullLayout._plots[plotinfo.mainplot]; - mainplot.overlays.push(plotinfo); - - plotinfo.gridlayer = mainplot.overgrid.append('g'); - plotinfo.zerolinelayer = mainplot.overzero.append('g'); - plotinfo.plot = mainplot.overplot.append('g').call(plotLayers); - plotinfo.xlines = mainplot.overlines.append('path'); - plotinfo.ylines = mainplot.overlines.append('path'); - plotinfo.xaxislayer = mainplot.overaxes.append('g'); - plotinfo.yaxislayer = mainplot.overaxes.append('g'); - }); - - // common attributes for all subplots, overlays or not - subplots.forEach(function(subplot) { - var plotinfo = fullLayout._plots[subplot]; - - plotinfo.xlines - .style('fill', 'none') - .classed('crisp', true); - plotinfo.ylines - .style('fill', 'none') - .classed('crisp', true); - }); -} diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js index aaed864dc92..77a414e4ef5 100644 --- a/src/plots/cartesian/index.js +++ b/src/plots/cartesian/index.js @@ -9,7 +9,9 @@ 'use strict'; +var d3 = require('d3'); var Plots = require('../plots'); +var Axes = require('./axes'); var constants = require('./constants'); @@ -35,6 +37,8 @@ exports.plot = function(gd, traces, transitionOpts, makeOnCompleteCallback) { calcdata = gd.calcdata, modules = fullLayout._modules; + updateSubplots(gd); + if(!Array.isArray(traces)) { // If traces is not provided, then it's a complete replot and missing // traces are removed @@ -146,3 +150,185 @@ exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) } } }; + +function updateSubplots(gd) { + var fullLayout = gd._fullLayout, + subplotData = makeSubplotData(gd); + + var subplotLayers = fullLayout._cartesianlayer.selectAll('.subplot') + .data(subplotData, String); + + subplotLayers.enter().append('g') + .classed('subplot', true); + + subplotLayers.order(); + + subplotLayers.each(function(subplot) { + var plotgroup = d3.select(this), + plotinfo = fullLayout._plots[subplot]; + + // references to any subplots overlaid on this one, + // filled in makeSubplotLayer + plotinfo.overlays = []; + + plotgroup.call(makeSubplotLayer, gd, subplot); + + // 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 = joinLayer(fullLayout._draggers, 'g', subplot); + }); + + // keep reference to shape layers in subplots + var layerSubplot = fullLayout._paper.selectAll('.layer-subplot'); + fullLayout._imageSubplotLayer = layerSubplot.selectAll('.imagelayer'); + fullLayout._shapeSubplotLayer = layerSubplot.selectAll('.shapelayer'); +} + +function makeSubplotData(gd) { + var fullLayout = gd._fullLayout, + subplots = Axes.getSubplots(gd); + + var subplotData = [], + overlays = []; + + for(var i = 0; i < subplots.length; i++) { + var subplot = subplots[i], + plotinfo = fullLayout._plots[subplot]; + + var xa = plotinfo.xaxis, + ya = plotinfo.yaxis; + + // is this subplot overlaid on another? + // ax.overlaying is the id of another axis of the same + // dimension that this one overlays to be an overlaid subplot, + // the main plot must exist make sure we're not trying to + // overlay on an axis that's already overlaying another + var xa2 = Axes.getFromId(gd, xa.overlaying) || xa; + if(xa2 !== xa && xa2.overlaying) { + xa2 = xa; + xa.overlaying = false; + } + + var ya2 = Axes.getFromId(gd, ya.overlaying) || ya; + if(ya2 !== ya && ya2.overlaying) { + ya2 = ya; + ya.overlaying = false; + } + + var mainplot = xa2._id + ya2._id; + if(mainplot !== subplot && subplots.indexOf(mainplot) !== -1) { + plotinfo.mainplot = mainplot; + overlays.push(subplot); + + // for now force overlays to overlay completely... so they + // can drag together correctly and share backgrounds. + // Later perhaps we make separate axis domain and + // tick/line domain or something, so they can still share + // the (possibly larger) dragger and background but don't + // have to both be drawn over that whole domain + xa.domain = xa2.domain.slice(); + ya.domain = ya2.domain.slice(); + } + else { + subplotData.push(subplot); + } + } + + // main subplots before overlays + subplotData = subplotData.concat(overlays); + + return subplotData; +} + +function makeSubplotLayer(plotgroup, gd, subplot) { + var fullLayout = gd._fullLayout, + plotinfo = fullLayout._plots[subplot]; + + // keep reference to plotgroup in _plots object + plotinfo.plotgroup = plotgroup; + + // add class corresponding to the subplot id + plotgroup.classed(subplot, true); + + // Layers to keep plot types in the right order. + // from back to front: + // 1. heatmaps, 2D histos and contour maps + // 2. bars / 1D histos + // 3. errorbars for bars and scatter + // 4. scatter + // 5. box plots + function plotLayers(parent) { + joinLayer(parent, 'g', 'imagelayer'); + joinLayer(parent, 'g', 'maplayer'); + joinLayer(parent, 'g', 'barlayer'); + joinLayer(parent, 'g', 'boxlayer'); + joinLayer(parent, 'g', 'scatterlayer'); + } + + if(!plotinfo.mainplot) { + plotinfo.bg = joinLayer(plotgroup, 'rect', 'bg'); + plotinfo.bg.style('stroke-width', 0); + + var backLayer = joinLayer(plotgroup, 'g', 'layer-subplot'); + plotinfo.shapelayer = joinLayer(backLayer, 'g', 'shapelayer'); + plotinfo.imagelayer = joinLayer(backLayer, 'g', 'imagelayer'); + + plotinfo.gridlayer = joinLayer(plotgroup, 'g', 'gridlayer'); + plotinfo.overgrid = joinLayer(plotgroup, 'g', 'overgrid'); + + plotinfo.zerolinelayer = joinLayer(plotgroup, 'g', 'zerolinelayer'); + plotinfo.overzero = joinLayer(plotgroup, 'g', 'overzero'); + + plotinfo.plot = joinLayer(plotgroup, 'g', 'plot'); + plotinfo.overplot = joinLayer(plotgroup, 'g', 'overplot'); + + plotinfo.xlines = joinLayer(plotgroup, 'path', 'xlines'); + plotinfo.ylines = joinLayer(plotgroup, 'path', 'ylines'); + plotinfo.overlines = joinLayer(plotgroup, 'g', 'overlines'); + + plotinfo.xaxislayer = joinLayer(plotgroup, 'g', 'xaxislayer'); + plotinfo.yaxislayer = joinLayer(plotgroup, 'g', 'yaxislayer'); + plotinfo.overaxes = joinLayer(plotgroup, 'g', 'overaxes'); + } + else { + + // now make the components of overlaid subplots + // overlays don't have backgrounds, and append all + // their other components to the corresponding + // extra groups of their main plots. + + var mainplot = fullLayout._plots[plotinfo.mainplot]; + mainplot.overlays.push(plotinfo); + + plotinfo.gridlayer = joinLayer(mainplot.overgrid, 'g', subplot); + plotinfo.zerolinelayer = joinLayer(mainplot.overzero, 'g', subplot); + + plotinfo.plot = joinLayer(mainplot.overplot, 'g', subplot); + plotinfo.xlines = joinLayer(mainplot.overlines, 'path', subplot); + plotinfo.ylines = joinLayer(mainplot.overlines, 'path', subplot); + plotinfo.xaxislayer = joinLayer(mainplot.overaxes, 'g', subplot); + plotinfo.yaxislayer = joinLayer(mainplot.overaxes, 'g', subplot); + } + + // common attributes for all subplots, overlays or not + plotinfo.plot.call(plotLayers); + + plotinfo.xlines + .style('fill', 'none') + .classed('crisp', true); + + plotinfo.ylines + .style('fill', 'none') + .classed('crisp', true); +} + +function joinLayer(parent, nodeType, className) { + var layer = parent.selectAll('.' + className) + .data([0]); + + layer.enter().append(nodeType) + .classed(className, true); + + return layer; +} From ff26691c3cff92316c0af5daf758ff6b40f1ca4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 15 Sep 2016 17:12:55 -0400 Subject: [PATCH 02/15] rm some early-Plotly.plot logic regarding makePlotFramwork --- src/plot_api/plot_api.js | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 63534438b54..2b51194d154 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -121,28 +121,15 @@ Plotly.plot = function(gd, data, layout, config) { // so we don't try to re-call Plotly.plot from inside // legend and colorbar, if margins changed gd._replotting = true; - var hasData = gd._fullData.length > 0; - var subplots = Plotly.Axes.getSubplots(gd).join(''), - oldSubplots = Object.keys(gd._fullLayout._plots || {}).join(''), - hasSameSubplots = (oldSubplots === subplots); + // make or remake the framework if we need to + if(graphWasEmpty) makePlotFramework(gd); - // Make or remake the framework (ie container and axes) if we need to - // note: if they container already exists and has data, - // the new layout gets ignored (as it should) - // but if there's no data there yet, it's just a placeholder... - // then it should destroy and remake the plot - if(hasData) { - if(gd.framework !== makePlotFramework || graphWasEmpty || !hasSameSubplots) { - gd.framework = makePlotFramework; - makePlotFramework(gd); - } - } - else if(!hasSameSubplots) { + // polar need a different framework + if(gd.framework !== makePlotFramework) { gd.framework = makePlotFramework; makePlotFramework(gd); } - else if(graphWasEmpty) makePlotFramework(gd); // save initial axis range once per graph if(graphWasEmpty) Plotly.Axes.saveRangeInitial(gd); From 40fc7f90d0c8c7e7d90982746dcab4e226e2c4d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 15 Sep 2016 17:15:53 -0400 Subject: [PATCH 03/15] plots: add linkSubplot method - which handles fullLayout._plots creation and reference updates - rm block in Plots.transition that used to handle that --- src/plots/plots.js | 54 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/src/plots/plots.js b/src/plots/plots.js index b4ffa470551..191b94bf7d6 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -414,6 +414,9 @@ plots.supplyDefaults = function(gd) { ax.setScale(); } + // relink / initialize subplot axis objects + plots.linkSubplots(newFullData, newFullLayout, oldFullData, oldFullLayout); + // update object references in calcdata if((gd.calcdata || []).length === newFullData.length) { for(i = 0; i < newFullData.length; i++) { @@ -559,6 +562,41 @@ function relinkPrivateKeys(toContainer, fromContainer) { } } +plots.linkSubplots = function(newFullData, newFullLayout, oldFullData, oldFullLayout) { + var oldSubplots = oldFullLayout._plots || {}, + newSubplots = newFullLayout._plots = {}; + + var mockGd = { + data: newFullData, + _fullLayout: newFullLayout + }; + + var ids = Plotly.Axes.getSubplots(mockGd); + + function getAxisFunc(subplot, axLetter) { + return function() { return Plotly.Axes.getFromId(mockGd, subplot, axLetter); }; + } + + for(var i = 0; i < ids.length; i++) { + var id = ids[i], + oldSubplot = oldSubplots[id], + plotinfo; + + if(oldSubplot) { + plotinfo = newSubplots[id] = oldSubplot; + } + else { + plotinfo = newSubplots[id] = {}; + plotinfo.id = id; + } + + plotinfo.x = getAxisFunc(id, 'x'); + plotinfo.y = getAxisFunc(id, 'y'); + plotinfo.xaxis = plotinfo.x(); + plotinfo.yaxis = plotinfo.y(); + } +}; + plots.supplyDataDefaults = function(dataIn, dataOut, layout) { var modules = layout._modules = [], basePlotModules = layout._basePlotModules = [], @@ -1363,7 +1401,8 @@ plots.transition = function(gd, data, layout, traces, frameOpts, transitionOpts) var transitionedTraces = []; function prepareTransitions() { - var plotinfo, i; + var i; + for(i = 0; i < traceIndices.length; i++) { var traceIdx = traceIndices[i]; var trace = gd._fullData[traceIdx]; @@ -1410,19 +1449,6 @@ plots.transition = function(gd, data, layout, traces, frameOpts, transitionOpts) // transitions: plots.supplyDefaults(gd); - // This step fies the .xaxis and .yaxis references that otherwise - // aren't updated by the supplyDefaults step: - var subplots = Plotly.Axes.getSubplots(gd); - - // Polar does not have _plots: - if(gd._fullLayout._plots) { - for(i = 0; i < subplots.length; i++) { - plotinfo = gd._fullLayout._plots[subplots[i]]; - plotinfo.xaxis = plotinfo.x(); - plotinfo.yaxis = plotinfo.y(); - } - } - plots.doCalcdata(gd); ErrorBars.calc(gd); From 652b90537747c3623f61f218cd608676cac6ebb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 15 Sep 2016 17:18:41 -0400 Subject: [PATCH 04/15] first try at making the new cartesian subplot paradigm work - we must now call drawAxes AFTER drawData (i.e. after updateSubplots) as drawAxes looks for some that updateSubplots creates - move layoutStyles / Fx.init out of makePlotFramework - make makePlotFramework return 'FRAMEWORK' so that restyle polar <-> cartesian still works --- src/plot_api/plot_api.js | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 2b51194d154..5bbf24a7833 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -180,8 +180,10 @@ Plotly.plot = function(gd, data, layout, config) { function marginPushersAgain() { // in case the margins changed, draw margin pushers again var seq = JSON.stringify(fullLayout._size) === oldmargins ? - [] : [marginPushers, subroutines.layoutStyles]; - return Lib.syncOrAsync(seq.concat(Fx.init), gd); + [] : + [marginPushers, subroutines.layoutStyles]; + + return Lib.syncOrAsync(seq, gd); } function positionAndAutorange() { @@ -205,7 +207,6 @@ Plotly.plot = function(gd, data, layout, config) { } } - // calc and autorange for errorbars ErrorBars.calc(gd); @@ -219,17 +220,13 @@ Plotly.plot = function(gd, data, layout, config) { function doAutoRange() { if(gd._transitioning) return; + var axList = Plotly.Axes.list(gd, '', true); for(var i = 0; i < axList.length; i++) { Plotly.Axes.doAutoRange(axList[i]); } } - function drawAxes() { - // draw ticks, titles, and calculate axis scaling (._b, ._m) - return Plotly.Axes.doTicks(gd, 'redraw'); - } - // Now plot the data function drawData() { var calcdata = gd.calcdata, @@ -277,6 +274,15 @@ Plotly.plot = function(gd, data, layout, config) { return Plots.previousPromises(gd); } + // draw ticks, titles, and calculate axis scaling (._b, ._m) + function drawAxes() { + Lib.syncOrAsync([ + subroutines.layoutStyles, + function() { return Plotly.Axes.doTicks(gd, 'redraw'); }, + Fx.init, + ], gd); + } + // An initial paint must be completed before these components can be // correctly sized and the whole plot re-margined. gd._replotting must // be set to false before these will work properly. @@ -302,8 +308,8 @@ Plotly.plot = function(gd, data, layout, config) { marginPushersAgain, positionAndAutorange, subroutines.layoutStyles, - drawAxes, drawData, + drawAxes, finalDraw ], gd, cleanUp); @@ -2736,16 +2742,5 @@ function makePlotFramework(gd) { gd.emit('plotly_framework'); - // position and style the containers, make main title - var frameWorkDone = Lib.syncOrAsync([ - subroutines.layoutStyles, - function goAxes() { return Plotly.Axes.doTicks(gd, 'redraw'); }, - Fx.init - ], gd); - - if(frameWorkDone && frameWorkDone.then) { - gd._promises.push(frameWorkDone); - } - - return frameWorkDone; + return 'FRAMEWORK'; } From e85a6c73d7b8980a2b4360155d0d69629634fd64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 15 Sep 2016 17:55:58 -0400 Subject: [PATCH 05/15] keep ref to subplot image/shape layers in main drawData - so that these ref on fullLayout exist even for non-cartesian trace types. --- src/plot_api/plot_api.js | 5 +++++ src/plots/cartesian/index.js | 5 ----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 5bbf24a7833..da51cd8327f 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -258,6 +258,11 @@ Plotly.plot = function(gd, data, layout, config) { basePlotModules[i].plot(gd); } + // keep reference to shape layers in subplots + var layerSubplot = fullLayout._paper.selectAll('.layer-subplot'); + fullLayout._imageSubplotLayer = layerSubplot.selectAll('.imagelayer'); + fullLayout._shapeSubplotLayer = layerSubplot.selectAll('.shapelayer'); + // styling separate from drawing Plots.style(gd); diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js index 77a414e4ef5..d19d996af83 100644 --- a/src/plots/cartesian/index.js +++ b/src/plots/cartesian/index.js @@ -178,11 +178,6 @@ function updateSubplots(gd) { // so they end up on top of the rest plotinfo.draglayer = joinLayer(fullLayout._draggers, 'g', subplot); }); - - // keep reference to shape layers in subplots - var layerSubplot = fullLayout._paper.selectAll('.layer-subplot'); - fullLayout._imageSubplotLayer = layerSubplot.selectAll('.imagelayer'); - fullLayout._shapeSubplotLayer = layerSubplot.selectAll('.shapelayer'); } function makeSubplotData(gd) { From 43d227adc9537dfd26b27961153b5737b4f70691 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 16 Sep 2016 18:19:13 -0400 Subject: [PATCH 06/15] lint --- src/plot_api/subroutines.js | 2 +- src/plots/cartesian/axes.js | 14 +++++++++----- src/plots/cartesian/dragbox.js | 1 - 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js index 758a778df92..5a59bc642f7 100644 --- a/src/plot_api/subroutines.js +++ b/src/plot_api/subroutines.js @@ -48,6 +48,7 @@ exports.lsInner = function(gd) { var plotinfo = fullLayout._plots[subplot], xa = Plotly.Axes.getFromId(gd, subplot, 'x'), ya = Plotly.Axes.getFromId(gd, subplot, 'y'); + xa.setScale(); // this may already be done... not sure ya.setScale(); @@ -59,7 +60,6 @@ exports.lsInner = function(gd) { .call(Color.fill, fullLayout.plot_bgcolor); } - // Clip so that data only shows up on the plot area. plotinfo.clipId = 'clip' + fullLayout._uid + subplot + 'plot'; diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 6d6d4fbd225..81a7bb5dba5 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -1302,10 +1302,10 @@ axes.findSubplotsWithAxis = function(subplots, ax) { // makeClipPaths: prepare clipPaths for all single axes and all possible xy pairings axes.makeClipPaths = function(gd) { - var layout = gd._fullLayout, - defs = layout._defs, - fullWidth = {_offset: 0, _length: layout.width, _id: ''}, - fullHeight = {_offset: 0, _length: layout.height, _id: ''}, + var fullLayout = gd._fullLayout, + defs = fullLayout._defs, + fullWidth = {_offset: 0, _length: fullLayout.width, _id: ''}, + fullHeight = {_offset: 0, _length: fullLayout.height, _id: ''}, xaList = axes.list(gd, 'x', true), yaList = axes.list(gd, 'y', true), clipList = [], @@ -1322,6 +1322,7 @@ axes.makeClipPaths = function(gd) { var defGroup = defs.selectAll('g.clips') .data([0]); + defGroup.enter().append('g') .classed('clips', true); @@ -1330,11 +1331,14 @@ axes.makeClipPaths = function(gd) { // https://groups.google.com/forum/#!topic/d3-js/6EpAzQ2gU9I var axClips = defGroup.selectAll('.axesclip') .data(clipList, function(d) { return d.x._id + d.y._id; }); + axClips.enter().append('clipPath') .classed('axesclip', true) - .attr('id', function(d) { return 'clip' + layout._uid + d.x._id + d.y._id; }) + .attr('id', function(d) { return 'clip' + fullLayout._uid + d.x._id + d.y._id; }) .append('rect'); + axClips.exit().remove(); + axClips.each(function(d) { d3.select(this).select('rect').attr({ x: d.x._offset || 0, diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js index 85058a998d7..de4a6196f74 100644 --- a/src/plots/cartesian/dragbox.js +++ b/src/plots/cartesian/dragbox.js @@ -695,7 +695,6 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { var plotDx = xa2._offset - fracDx, plotDy = ya2._offset - fracDy; - fullLayout._defs.selectAll('#' + subplot.clipId) .call(Lib.setTranslate, clipDx, clipDy) .call(Lib.setScale, 1 / xScaleFactor, 1 / yScaleFactor); From 937c54b53e6558060215b73c8fea440ee6ad0d47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 16 Sep 2016 18:22:31 -0400 Subject: [PATCH 07/15] add baseModule drawFramework step - after several failed attempts at getting cartesian subplot creation/update/removal into the Cartesian.plot (like the other base plot module), I decide to go with a less ambitious refactoring where a drawFramework update step is added to the main Plotly.plot code path - Main reason: several layout components require the cartesian framework to be present in order to work properly. --- src/plot_api/plot_api.js | 35 +++++++++++++++++++++++++---------- src/plots/cartesian/index.js | 6 ++---- 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index da51cd8327f..fb6a9663b2b 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -154,6 +154,24 @@ Plotly.plot = function(gd, data, layout, config) { var oldmargins = JSON.stringify(fullLayout._size); + // draw framework first so that margin-pushing + // components can position themselves correctly + function drawFramework() { + var basePlotModules = fullLayout._basePlotModules; + + for(var i = 0; i < basePlotModules.length; i++) { + if(basePlotModules[i].drawFramework) { + basePlotModules[i].drawFramework(gd); + } + } + + return Lib.syncOrAsync([ + subroutines.layoutStyles, + drawAxes, + Fx.init + ], gd); + } + // draw anything that can affect margins. // currently this is legend and colorbars function marginPushers() { @@ -227,6 +245,11 @@ Plotly.plot = function(gd, data, layout, config) { } } + // draw ticks, titles, and calculate axis scaling (._b, ._m) + function drawAxes() { + return Plotly.Axes.doTicks(gd, 'redraw'); + } + // Now plot the data function drawData() { var calcdata = gd.calcdata, @@ -279,15 +302,6 @@ Plotly.plot = function(gd, data, layout, config) { return Plots.previousPromises(gd); } - // draw ticks, titles, and calculate axis scaling (._b, ._m) - function drawAxes() { - Lib.syncOrAsync([ - subroutines.layoutStyles, - function() { return Plotly.Axes.doTicks(gd, 'redraw'); }, - Fx.init, - ], gd); - } - // An initial paint must be completed before these components can be // correctly sized and the whole plot re-margined. gd._replotting must // be set to false before these will work properly. @@ -309,12 +323,13 @@ Plotly.plot = function(gd, data, layout, config) { Lib.syncOrAsync([ Plots.previousPromises, + drawFramework, marginPushers, marginPushersAgain, positionAndAutorange, subroutines.layoutStyles, - drawData, drawAxes, + drawData, finalDraw ], gd, cleanUp); diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js index d19d996af83..718d0b23b0f 100644 --- a/src/plots/cartesian/index.js +++ b/src/plots/cartesian/index.js @@ -37,8 +37,6 @@ exports.plot = function(gd, traces, transitionOpts, makeOnCompleteCallback) { calcdata = gd.calcdata, modules = fullLayout._modules; - updateSubplots(gd); - if(!Array.isArray(traces)) { // If traces is not provided, then it's a complete replot and missing // traces are removed @@ -151,7 +149,7 @@ exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) } }; -function updateSubplots(gd) { +exports.drawFramework = function(gd) { var fullLayout = gd._fullLayout, subplotData = makeSubplotData(gd); @@ -178,7 +176,7 @@ function updateSubplots(gd) { // so they end up on top of the rest plotinfo.draglayer = joinLayer(fullLayout._draggers, 'g', subplot); }); -} +}; function makeSubplotData(gd) { var fullLayout = gd._fullLayout, From 78faa3fe4399d13c37207499c451c82e7da701af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 16 Sep 2016 18:22:48 -0400 Subject: [PATCH 08/15] plots: add a cartesian subplot purge step --- src/plots/cartesian/index.js | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js index 718d0b23b0f..7377caf749c 100644 --- a/src/plots/cartesian/index.js +++ b/src/plots/cartesian/index.js @@ -10,9 +10,9 @@ 'use strict'; var d3 = require('d3'); +var Lib = require('../../lib'); var Plots = require('../plots'); var Axes = require('./axes'); - var constants = require('./constants'); exports.name = 'cartesian'; @@ -147,6 +147,16 @@ exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) } } } + + var hadCartesian = (oldFullLayout._has && oldFullLayout._has('cartesian')); + var hasCartesian = (newFullLayout._has && newFullLayout._has('cartesian')); + + if(hadCartesian && !hasCartesian) { + var subplotLayers = oldFullLayout._cartesianlayer.selectAll('.subplot'); + + subplotLayers.call(purgeSubplotLayers, oldFullLayout); + oldFullLayout._defs.selectAll('.axesclip').remove(); + } }; exports.drawFramework = function(gd) { @@ -154,13 +164,16 @@ exports.drawFramework = function(gd) { subplotData = makeSubplotData(gd); var subplotLayers = fullLayout._cartesianlayer.selectAll('.subplot') - .data(subplotData, String); + .data(subplotData, Lib.identity); subplotLayers.enter().append('g') .classed('subplot', true); subplotLayers.order(); + subplotLayers.exit() + .call(purgeSubplotLayers, fullLayout); + subplotLayers.each(function(subplot) { var plotgroup = d3.select(this), plotinfo = fullLayout._plots[subplot]; @@ -316,6 +329,23 @@ function makeSubplotLayer(plotgroup, gd, subplot) { .classed('crisp', true); } +function purgeSubplotLayers(layers, fullLayout) { + if(!layers) return; + + layers.each(function(subplot) { + var plotgroup = d3.select(this), + clipId = 'clip' + fullLayout._uid + subplot + 'plot'; + + plotgroup.remove(); + fullLayout._draggers.selectAll('g.' + subplot).remove(); + fullLayout._defs.select('#' + clipId).remove(); + + // do not remove individual axis s here + // as other subplots may need them + }); +} + + function joinLayer(parent, nodeType, className) { var layer = parent.selectAll('.' + className) .data([0]); From f8017f25af7227e5aff14c3330c03e17ed2b025e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 16 Sep 2016 13:40:01 -0400 Subject: [PATCH 09/15] test: adapt relinkPrivateKeys test --- test/jasmine/tests/plots_test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/jasmine/tests/plots_test.js b/test/jasmine/tests/plots_test.js index 4f9284b3753..0914634191e 100644 --- a/test/jasmine/tests/plots_test.js +++ b/test/jasmine/tests/plots_test.js @@ -21,7 +21,7 @@ describe('Test Plots', function() { }]; var oldFullLayout = { - _plots: { xy: {} }, + _plots: { xy: { plot: {} } }, xaxis: { c2p: function() {} }, yaxis: { _m: 20 }, scene: { _scene: {} }, @@ -54,7 +54,7 @@ describe('Test Plots', function() { expect(gd._fullData[1].z).toBe(newData[1].z); expect(gd._fullData[1]._empties).toBe(oldFullData[1]._empties); expect(gd._fullLayout.scene._scene).toBe(oldFullLayout.scene._scene); - expect(gd._fullLayout._plots).toBe(oldFullLayout._plots); + expect(gd._fullLayout._plots.plot).toBe(oldFullLayout._plots.plot); expect(gd._fullLayout.annotations[0]._min).toBe(oldFullLayout.annotations[0]._min); expect(gd._fullLayout.annotations[1]._max).toBe(oldFullLayout.annotations[1]._max); expect(gd._fullLayout.someFunc).toBe(oldFullLayout.someFunc); From c239602000b937afe00f64fecbfae283f544a908 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 16 Sep 2016 16:35:32 -0400 Subject: [PATCH 10/15] test: adapt legend scroll test - this suite used to rely on className 'bg' to grab the legend background element, but now className 'bg' is also used in . --- test/jasmine/tests/legend_scroll_test.js | 71 ++++++++++++++---------- 1 file changed, 43 insertions(+), 28 deletions(-) diff --git a/test/jasmine/tests/legend_scroll_test.js b/test/jasmine/tests/legend_scroll_test.js index 98b4f0f25ee..b93610ac5f7 100644 --- a/test/jasmine/tests/legend_scroll_test.js +++ b/test/jasmine/tests/legend_scroll_test.js @@ -2,6 +2,7 @@ var Plotly = require('@lib/index'); var Lib = require('@src/lib'); var constants = require('@src/components/legend/constants'); +var d3 = require('d3'); var createGraph = require('../assets/create_graph_div'); var destroyGraph = require('../assets/destroy_graph_div'); var getBBox = require('../assets/get_bbox'); @@ -11,8 +12,6 @@ var mock = require('../../image/mocks/legend_scroll.json'); describe('The legend', function() { 'use strict'; - var gd, legend, bg; - function countLegendGroups(gd) { return gd._fullLayout._toppaper.selectAll('g.legend').size(); } @@ -27,19 +26,36 @@ describe('The legend', function() { return gd._fullLayout.height - gd._fullLayout.margin.t - gd._fullLayout.margin.b; } - function getLegendHeight() { + function getLegendHeight(gd) { + var bg = d3.select('g.legend').select('.bg').node(); return gd._fullLayout.legend.borderwidth + getBBox(bg).height; } + function getLegend() { + return d3.select('g.legend').node(); + } + + function getScrollBox() { + return d3.select('g.legend').select('.scrollbox').node(); + } + + function getScrollBar() { + return d3.select('g.legend').select('.scrollbar').node(); + } + + function getToggle() { + return d3.select('g.legend').select('.legendtoggle').node(); + } + describe('when plotted with many traces', function() { + var gd; + beforeEach(function(done) { gd = createGraph(); var mockCopy = Lib.extendDeep({}, mock); Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(function() { - legend = document.getElementsByClassName('legend')[0]; - bg = document.getElementsByClassName('bg')[0]; done(); }); }); @@ -47,21 +63,22 @@ describe('The legend', function() { afterEach(destroyGraph); it('should not exceed plot height', function() { - var legendHeight = getLegendHeight(); + var legendHeight = getLegendHeight(gd); expect(+legendHeight).toBe(getPlotHeight(gd)); }); it('should insert a scrollbar', function() { - var scrollBar = legend.getElementsByClassName('scrollbar')[0]; + var scrollBar = getScrollBar(); expect(scrollBar).toBeDefined(); expect(scrollBar.getAttribute('x')).not.toBe(null); }); it('should scroll when there\'s a wheel event', function() { - var scrollBox = legend.getElementsByClassName('scrollbox')[0], - legendHeight = getLegendHeight(), + var legend = getLegend(), + scrollBox = getScrollBox(), + legendHeight = getLegendHeight(gd), scrollBoxYMax = gd._fullLayout.legend.height - legendHeight, scrollBarYMax = legendHeight - constants.scrollBarHeight - @@ -80,8 +97,9 @@ describe('The legend', function() { }); it('should keep the scrollbar position after a toggle event', function() { - var scrollBox = legend.getElementsByClassName('scrollbox')[0], - toggle = legend.getElementsByClassName('legendtoggle')[0], + var legend = getLegend(), + scrollBox = getScrollBox(), + toggle = getToggle(), wheelDeltaY = 100; legend.dispatchEvent(scrollTo(wheelDeltaY)); @@ -96,7 +114,7 @@ describe('The legend', function() { it('should be restored and functional after relayout', function() { var wheelDeltaY = 100, - legend = document.getElementsByClassName('legend')[0], + legend = getLegend(), scrollBox, scrollBar, scrollBarX, @@ -111,10 +129,10 @@ describe('The legend', function() { Plotly.relayout(gd, 'showlegend', false); Plotly.relayout(gd, 'showlegend', true); - legend = document.getElementsByClassName('legend')[0]; - scrollBox = legend.getElementsByClassName('scrollbox')[0]; - scrollBar = legend.getElementsByClassName('scrollbar')[0]; - toggle = legend.getElementsByClassName('legendtoggle')[0]; + legend = getLegend(); + scrollBox = getScrollBox(); + scrollBar = getScrollBar(); + toggle = getToggle(); legend.dispatchEvent(scrollTo(wheelDeltaY)); expect(scrollBar.getAttribute('x')).toBe(scrollBarX); @@ -131,7 +149,8 @@ describe('The legend', function() { }); it('should constrain scrolling to the contents', function() { - var scrollBox = legend.getElementsByClassName('scrollbox')[0]; + var legend = getLegend(), + scrollBox = getScrollBox(); legend.dispatchEvent(scrollTo(-100)); expect(scrollBox.getAttribute('transform')).toBe('translate(0, 0)'); @@ -141,8 +160,9 @@ describe('The legend', function() { }); it('should scale the scrollbar movement from top to bottom', function() { - var scrollBar = legend.getElementsByClassName('scrollbar')[0], - legendHeight = getLegendHeight(); + var legend = getLegend(), + scrollBar = getScrollBar(), + legendHeight = getLegendHeight(gd); // The scrollbar is 20px tall and has 4px margins @@ -166,10 +186,10 @@ describe('The legend', function() { }); it('should resize when relayout\'ed with new height', function(done) { - var origLegendHeight = getLegendHeight(); + var origLegendHeight = getLegendHeight(gd); Plotly.relayout(gd, 'height', gd._fullLayout.height / 2).then(function() { - var legendHeight = getLegendHeight(); + var legendHeight = getLegendHeight(gd); // legend still exists and not duplicated expect(countLegendGroups(gd)).toBe(1); @@ -184,7 +204,6 @@ describe('The legend', function() { }); }); - describe('when plotted with few traces', function() { var gd; @@ -219,14 +238,10 @@ describe('The legend', function() { }); it('should resize when traces added', function(done) { - legend = document.getElementsByClassName('legend')[0]; - bg = document.getElementsByClassName('bg')[0]; - var origLegendHeight = getLegendHeight(); + var origLegendHeight = getLegendHeight(gd); Plotly.addTraces(gd, { x: [1, 2, 3], y: [4, 3, 2], name: 'Test2' }).then(function() { - legend = document.getElementsByClassName('legend')[0]; - bg = document.getElementsByClassName('bg')[0]; - var legendHeight = getLegendHeight(); + var legendHeight = getLegendHeight(gd); expect(+legendHeight).toBeCloseTo(+origLegendHeight + 19, 0); From f4aee1e92c07ea7d0583ae12fb34e5408c9a147d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 16 Sep 2016 16:36:34 -0400 Subject: [PATCH 11/15] test: adapt gd._ keys ordering - doCalcdata is now called before drawFramwework, so the ordering for which _ keys are added to the gd differs slightly. --- test/jasmine/tests/plots_test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/jasmine/tests/plots_test.js b/test/jasmine/tests/plots_test.js index 0914634191e..a0908d2777e 100644 --- a/test/jasmine/tests/plots_test.js +++ b/test/jasmine/tests/plots_test.js @@ -302,8 +302,8 @@ describe('Test Plots', function() { it('should unset everything in the gd except _context', function() { var expectedKeys = [ '_ev', 'on', 'once', 'removeListener', 'removeAllListeners', - 'emit', '_context', '_replotPending', '_mouseDownTime', - '_hmpixcount', '_hmlumcount' + 'emit', '_context', '_replotPending', + '_hmpixcount', '_hmlumcount', '_mouseDownTime' ]; Plots.purge(gd); From ab964229be20dab6b82a7c009006f8ab9afbe59c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 16 Sep 2016 17:54:40 -0400 Subject: [PATCH 12/15] test: check clip path & dragger counts in cartesian plots --- test/jasmine/tests/plot_interact_test.js | 48 ++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/test/jasmine/tests/plot_interact_test.js b/test/jasmine/tests/plot_interact_test.js index aa9df30fc62..34032945d7c 100644 --- a/test/jasmine/tests/plot_interact_test.js +++ b/test/jasmine/tests/plot_interact_test.js @@ -37,6 +37,14 @@ describe('Test plot structure', function() { return d3.selectAll('rect.cbbg').size(); } + function countClipPaths() { + return d3.selectAll('defs').selectAll('.axesclip,.plotclip').size(); + } + + function countDraggers() { + return d3.selectAll('g.draglayer').selectAll('g').size(); + } + describe('scatter traces', function() { var mock = require('@mocks/14.json'); var gd; @@ -54,6 +62,14 @@ describe('Test plot structure', function() { expect(countSubplots()).toEqual(1); }); + it('has four clip paths', function() { + expect(countClipPaths()).toEqual(4); + }); + + it('has one dragger group', function() { + expect(countDraggers()).toEqual(1); + }); + it('has one *scatterlayer* node', function() { var nodes = d3.selectAll('g.scatterlayer'); expect(nodes.size()).toEqual(1); @@ -90,11 +106,15 @@ describe('Test plot structure', function() { Plotly.deleteTraces(gd, [0]).then(function() { expect(countScatterTraces()).toEqual(0); expect(countSubplots()).toEqual(1); + expect(countClipPaths()).toEqual(4); + expect(countDraggers()).toEqual(1); return Plotly.relayout(gd, {xaxis: null, yaxis: null}); }).then(function() { expect(countScatterTraces()).toEqual(0); expect(countSubplots()).toEqual(0); + expect(countClipPaths()).toEqual(0); + expect(countDraggers()).toEqual(0); done(); }); @@ -302,6 +322,8 @@ describe('Test plot structure', function() { it('has four *subplot* nodes', function() { expect(countSubplots()).toEqual(4); + expect(countClipPaths()).toEqual(12); + expect(countDraggers()).toEqual(4); }); it('has four heatmap image nodes', function() { @@ -340,6 +362,8 @@ describe('Test plot structure', function() { it('has four *subplot* nodes', function() { expect(countSubplots()).toEqual(4); + expect(countClipPaths()).toEqual(12); + expect(countDraggers()).toEqual(4); }); it('has two heatmap image nodes', function() { @@ -376,6 +400,8 @@ describe('Test plot structure', function() { Plotly.deleteTraces(gd, [0]).then(function() { expect(countSubplots()).toEqual(4); + expect(countClipPaths()).toEqual(12); + expect(countDraggers()).toEqual(4); assertHeatmapNodes(3); assertContourNodes(2); expect(countColorBars()).toEqual(0); @@ -383,6 +409,8 @@ describe('Test plot structure', function() { return Plotly.deleteTraces(gd, [0]); }).then(function() { expect(countSubplots()).toEqual(4); + expect(countClipPaths()).toEqual(12); + expect(countDraggers()).toEqual(4); assertHeatmapNodes(2); assertContourNodes(2); expect(countColorBars()).toEqual(0); @@ -390,6 +418,8 @@ describe('Test plot structure', function() { return Plotly.deleteTraces(gd, [0]); }).then(function() { expect(countSubplots()).toEqual(4); + expect(countClipPaths()).toEqual(12); + expect(countDraggers()).toEqual(4); assertHeatmapNodes(1); assertContourNodes(1); expect(countColorBars()).toEqual(0); @@ -397,6 +427,24 @@ describe('Test plot structure', function() { return Plotly.deleteTraces(gd, [0]); }).then(function() { expect(countSubplots()).toEqual(3); + expect(countClipPaths()).toEqual(11); + expect(countDraggers()).toEqual(3); + assertHeatmapNodes(0); + assertContourNodes(0); + expect(countColorBars()).toEqual(0); + + var update = { + xaxis: null, + yaxis: null, + xaxis2: null, + yaxis2: null + }; + + return Plotly.relayout(gd, update); + }).then(function() { + expect(countSubplots()).toEqual(0); + expect(countClipPaths()).toEqual(0); + expect(countDraggers()).toEqual(0); assertHeatmapNodes(0); assertContourNodes(0); expect(countColorBars()).toEqual(0); From ae3ab5554219f5ad93f77d8f83355d79f34fe79a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 23 Sep 2016 16:42:21 -0400 Subject: [PATCH 13/15] replace plotinfo.x() -> plotinfo.xaxis --- src/components/errorbars/plot.js | 5 +++-- src/plots/cartesian/axes.js | 6 +++--- src/plots/cartesian/dragbox.js | 28 +++++++++++++------------- src/plots/cartesian/graph_interact.js | 4 ++-- src/plots/cartesian/select.js | 4 ++-- src/plots/cartesian/transition_axes.js | 10 ++++----- src/plots/plots.js | 10 ++------- src/traces/bar/plot.js | 4 ++-- src/traces/bar/set_positions.js | 8 ++++---- src/traces/box/plot.js | 4 ++-- src/traces/box/set_positions.js | 4 ++-- src/traces/contour/plot.js | 12 +++++------ src/traces/heatmap/plot.js | 4 ++-- src/traces/scatter/plot.js | 8 ++++---- src/traces/scatterternary/plot.js | 4 ++-- test/jasmine/tests/bar_test.js | 4 ++-- 16 files changed, 57 insertions(+), 62 deletions(-) diff --git a/src/components/errorbars/plot.js b/src/components/errorbars/plot.js index 07fdeafc3ec..4126c81936b 100644 --- a/src/components/errorbars/plot.js +++ b/src/components/errorbars/plot.js @@ -16,8 +16,9 @@ var subTypes = require('../../traces/scatter/subtypes'); module.exports = function plot(traces, plotinfo, transitionOpts) { var isNew; - var xa = plotinfo.x(), - ya = plotinfo.y(); + + var xa = plotinfo.xaxis, + ya = plotinfo.yaxis; var hasAnimation = transitionOpts && transitionOpts.duration > 0; diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 81a7bb5dba5..56cc0eb9ea9 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -1373,8 +1373,8 @@ axes.doTicks = function(gd, axid, skipTitle) { if(axid === 'redraw') { fullLayout._paper.selectAll('g.subplot').each(function(subplot) { var plotinfo = fullLayout._plots[subplot], - xa = plotinfo.x(), - ya = plotinfo.y(); + xa = plotinfo.xaxis, + ya = plotinfo.yaxis; plotinfo.xaxislayer .selectAll('.' + xa._id + 'tick').remove(); @@ -1837,7 +1837,7 @@ axes.doTicks = function(gd, axid, skipTitle) { // [bottom or left, top or right, free, main] linepositions = ax._linepositions[subplot] || [], - counteraxis = plotinfo[counterLetter](), + counteraxis = plotinfo[counterLetter + 'axis'], mainSubplot = counteraxis._id === ax.anchor, ticksides = [false, false, false], tickpath = ''; diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js index de4a6196f74..d2f5271ba3d 100644 --- a/src/plots/cartesian/dragbox.js +++ b/src/plots/cartesian/dragbox.js @@ -48,8 +48,8 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { var fullLayout = gd._fullLayout, // if we're dragging two axes at once, also drag overlays subplots = [plotinfo].concat((ns && ew) ? plotinfo.overlays : []), - xa = [plotinfo.x()], - ya = [plotinfo.y()], + xa = [plotinfo.xaxis], + ya = [plotinfo.yaxis], pw = xa[0]._length, ph = ya[0]._length, MINDRAG = constants.MINDRAG, @@ -57,8 +57,8 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { isMainDrag = (ns + ew === 'nsew'); for(var i = 1; i < subplots.length; i++) { - var subplotXa = subplots[i].x(), - subplotYa = subplots[i].y(); + var subplotXa = subplots[i].xaxis, + subplotYa = subplots[i].yaxis; if(xa.indexOf(subplotXa) === -1) xa.push(subplotXa); if(ya.indexOf(subplotYa) === -1) ya.push(subplotYa); } @@ -146,8 +146,8 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { dragElement.init(dragOptions); var zoomlayer = gd._fullLayout._zoomlayer, - xs = plotinfo.x()._offset, - ys = plotinfo.y()._offset, + xs = plotinfo.xaxis._offset, + ys = plotinfo.yaxis._offset, x0, y0, box, @@ -159,14 +159,14 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { corners; function recomputeAxisLists() { - xa = [plotinfo.x()]; - ya = [plotinfo.y()]; + xa = [plotinfo.xaxis]; + ya = [plotinfo.yaxis]; pw = xa[0]._length; ph = ya[0]._length; for(var i = 1; i < subplots.length; i++) { - var subplotXa = subplots[i].x(), - subplotYa = subplots[i].y(); + var subplotXa = subplots[i].xaxis, + subplotYa = subplots[i].yaxis; if(xa.indexOf(subplotXa) === -1) xa.push(subplotXa); if(ya.indexOf(subplotYa) === -1) ya.push(subplotYa); } @@ -174,8 +174,8 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { xActive = isDirectionActive(xa, ew); yActive = isDirectionActive(ya, ns); cursor = getDragCursor(yActive + xActive, fullLayout.dragmode); - xs = plotinfo.x()._offset; - ys = plotinfo.y()._offset; + xs = plotinfo.xaxis._offset; + ys = plotinfo.yaxis._offset; dragOptions.xa = xa; dragOptions.ya = ya; } @@ -656,8 +656,8 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { for(var i = 0; i < subplots.length; i++) { var subplot = plotinfos[subplots[i]], - xa2 = subplot.x(), - ya2 = subplot.y(), + xa2 = subplot.xaxis, + ya2 = subplot.yaxis, editX = ew && !xa2.fixedrange, editY = ns && !ya2.fixedrange; diff --git a/src/plots/cartesian/graph_interact.js b/src/plots/cartesian/graph_interact.js index bed9ad0b723..bf2d373779f 100644 --- a/src/plots/cartesian/graph_interact.js +++ b/src/plots/cartesian/graph_interact.js @@ -109,8 +109,8 @@ fx.init = function(gd) { if(!fullLayout._has('cartesian')) return; - var xa = plotinfo.x(), - ya = plotinfo.y(), + var xa = plotinfo.xaxis, + ya = plotinfo.yaxis, // the y position of the main x axis line y0 = (xa._linepositions[subplot] || [])[3], diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js index 9c36daf30a2..9afa8ec8c4e 100644 --- a/src/plots/cartesian/select.js +++ b/src/plots/cartesian/select.js @@ -24,8 +24,8 @@ function getAxId(ax) { return ax._id; } module.exports = function prepSelect(e, startX, startY, dragOptions, mode) { var plot = dragOptions.gd._fullLayout._zoomlayer, dragBBox = dragOptions.element.getBoundingClientRect(), - xs = dragOptions.plotinfo.x()._offset, - ys = dragOptions.plotinfo.y()._offset, + xs = dragOptions.plotinfo.xaxis._offset, + ys = dragOptions.plotinfo.yaxis._offset, x0 = startX - dragBBox.left, y0 = startY - dragBBox.top, x1 = x0, diff --git a/src/plots/cartesian/transition_axes.js b/src/plots/cartesian/transition_axes.js index e1ffe9a2167..96462dd4ee0 100644 --- a/src/plots/cartesian/transition_axes.js +++ b/src/plots/cartesian/transition_axes.js @@ -134,8 +134,8 @@ module.exports = function transitionAxes(gd, newLayout, transitionOpts, makeOnCo } function unsetSubplotTransform(subplot) { - var xa2 = subplot.x(); - var ya2 = subplot.y(); + var xa2 = subplot.xaxis; + var ya2 = subplot.yaxis; fullLayout._defs.selectAll('#' + subplot.clipId) .call(Lib.setTranslate, 0, 0) @@ -194,11 +194,11 @@ module.exports = function transitionAxes(gd, newLayout, transitionOpts, makeOnCo viewBox[3] = subplot.yaxis._length; } - ticksAndAnnotations(subplot.x(), subplot.y()); + ticksAndAnnotations(subplot.xaxis, subplot.yaxis); - var xa2 = subplot.x(); - var ya2 = subplot.y(); + var xa2 = subplot.xaxis; + var ya2 = subplot.yaxis; var editX = !!xUpdate; var editY = !!yUpdate; diff --git a/src/plots/plots.js b/src/plots/plots.js index 9ba716986f9..cba332d8198 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -573,10 +573,6 @@ plots.linkSubplots = function(newFullData, newFullLayout, oldFullData, oldFullLa var ids = Plotly.Axes.getSubplots(mockGd); - function getAxisFunc(subplot, axLetter) { - return function() { return Plotly.Axes.getFromId(mockGd, subplot, axLetter); }; - } - for(var i = 0; i < ids.length; i++) { var id = ids[i], oldSubplot = oldSubplots[id], @@ -590,10 +586,8 @@ plots.linkSubplots = function(newFullData, newFullLayout, oldFullData, oldFullLa plotinfo.id = id; } - plotinfo.x = getAxisFunc(id, 'x'); - plotinfo.y = getAxisFunc(id, 'y'); - plotinfo.xaxis = plotinfo.x(); - plotinfo.yaxis = plotinfo.y(); + plotinfo.xaxis = Plotly.Axes.getFromId(mockGd, id, 'x'); + plotinfo.yaxis = Plotly.Axes.getFromId(mockGd, id, 'y'); } }; diff --git a/src/traces/bar/plot.js b/src/traces/bar/plot.js index dd6504cca61..0ea14bf3ee8 100644 --- a/src/traces/bar/plot.js +++ b/src/traces/bar/plot.js @@ -20,8 +20,8 @@ var arraysToCalcdata = require('./arrays_to_calcdata'); module.exports = function plot(gd, plotinfo, cdbar) { - var xa = plotinfo.x(), - ya = plotinfo.y(), + var xa = plotinfo.xaxis, + ya = plotinfo.yaxis, fullLayout = gd._fullLayout; var bartraces = plotinfo.plot.select('.barlayer') diff --git a/src/traces/bar/set_positions.js b/src/traces/bar/set_positions.js index b6d09a714a9..3fea9a1d58c 100644 --- a/src/traces/bar/set_positions.js +++ b/src/traces/bar/set_positions.js @@ -24,16 +24,16 @@ var Lib = require('../../lib'); module.exports = function setPositions(gd, plotinfo) { var fullLayout = gd._fullLayout, - xa = plotinfo.x(), - ya = plotinfo.y(), + xa = plotinfo.xaxis, + ya = plotinfo.yaxis, i, j; ['v', 'h'].forEach(function(dir) { var bl = [], pLetter = {v: 'x', h: 'y'}[dir], sLetter = {v: 'y', h: 'x'}[dir], - pa = plotinfo[pLetter](), - sa = plotinfo[sLetter](); + pa = plotinfo[pLetter + 'axis'], + sa = plotinfo[sLetter + 'axis']; gd._fullData.forEach(function(trace, i) { if(trace.visible === true && diff --git a/src/traces/box/plot.js b/src/traces/box/plot.js index 5d251686f9c..3cad8ab2e5b 100644 --- a/src/traces/box/plot.js +++ b/src/traces/box/plot.js @@ -37,8 +37,8 @@ var JITTERCOUNT = 5, // points either side of this to include module.exports = function plot(gd, plotinfo, cdbox) { var fullLayout = gd._fullLayout, - xa = plotinfo.x(), - ya = plotinfo.y(), + xa = plotinfo.xaxis, + ya = plotinfo.yaxis, posAxis, valAxis; var boxtraces = plotinfo.plot.select('.boxlayer') diff --git a/src/traces/box/set_positions.js b/src/traces/box/set_positions.js index cb803850add..038e6e5533e 100644 --- a/src/traces/box/set_positions.js +++ b/src/traces/box/set_positions.js @@ -15,8 +15,8 @@ var Lib = require('../../lib'); module.exports = function setPositions(gd, plotinfo) { var fullLayout = gd._fullLayout, - xa = plotinfo.x(), - ya = plotinfo.y(), + xa = plotinfo.xaxis, + ya = plotinfo.yaxis, orientations = ['v', 'h']; var posAxis, i, j, k; diff --git a/src/traces/contour/plot.js b/src/traces/contour/plot.js index 2122f6e5c0e..21076c87139 100644 --- a/src/traces/contour/plot.js +++ b/src/traces/contour/plot.js @@ -56,8 +56,8 @@ function plotOne(gd, plotinfo, cd) { y = cd[0].y, contours = trace.contours, uid = trace.uid, - xa = plotinfo.x(), - ya = plotinfo.y(), + xa = plotinfo.xaxis, + ya = plotinfo.yaxis, fullLayout = gd._fullLayout, id = 'contour' + uid, pathinfo = emptyPathinfo(contours, plotinfo, cd[0]); @@ -120,8 +120,8 @@ function emptyPathinfo(contours, plotinfo, cd0) { // all closed paths paths: [], // store axes so we can convert to px - xaxis: plotinfo.x(), - yaxis: plotinfo.y(), + xaxis: plotinfo.xaxis, + yaxis: plotinfo.yaxis, // full data arrays to use for interpolation x: cd0.x, y: cd0.y, @@ -662,8 +662,8 @@ function clipGaps(plotGroup, plotinfo, cd0, perimeter) { starts: [], edgepaths: [], paths: [], - xaxis: plotinfo.x(), - yaxis: plotinfo.y(), + xaxis: plotinfo.xaxis, + yaxis: plotinfo.yaxis, x: cd0.x, y: cd0.y, // 0 = no data, 1 = data diff --git a/src/traces/heatmap/plot.js b/src/traces/heatmap/plot.js index d8632d945dc..e1d6694ddd1 100644 --- a/src/traces/heatmap/plot.js +++ b/src/traces/heatmap/plot.js @@ -30,8 +30,8 @@ module.exports = function(gd, plotinfo, cdheatmaps) { function plotOne(gd, plotinfo, cd) { var trace = cd[0].trace, uid = trace.uid, - xa = plotinfo.x(), - ya = plotinfo.y(), + xa = plotinfo.xaxis, + ya = plotinfo.yaxis, fullLayout = gd._fullLayout, id = 'hm' + uid; diff --git a/src/traces/scatter/plot.js b/src/traces/scatter/plot.js index 763a42cff7f..c52fc921692 100644 --- a/src/traces/scatter/plot.js +++ b/src/traces/scatter/plot.js @@ -156,8 +156,8 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition return hasTransition ? selection.transition() : selection; } - var xa = plotinfo.x(), - ya = plotinfo.y(); + var xa = plotinfo.xaxis, + ya = plotinfo.yaxis; var trace = cdscatter[0].trace, line = trace.line, @@ -479,8 +479,8 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition } function selectMarkers(gd, idx, plotinfo, cdscatter, cdscatterAll) { - var xa = plotinfo.x(), - ya = plotinfo.y(), + var xa = plotinfo.xaxis, + ya = plotinfo.yaxis, xr = d3.extent(xa.range.map(xa.l2c)), yr = d3.extent(ya.range.map(ya.l2c)); diff --git a/src/traces/scatterternary/plot.js b/src/traces/scatterternary/plot.js index ca866ce7038..742eced81e3 100644 --- a/src/traces/scatterternary/plot.js +++ b/src/traces/scatterternary/plot.js @@ -20,8 +20,8 @@ module.exports = function plot(ternary, data) { // mimic cartesian plotinfo var plotinfo = { - x: function() { return ternary.xaxis; }, - y: function() { return ternary.yaxis; }, + xaxis: ternary.xaxis, + yaxis: ternary.yaxis, plot: plotContainer }; diff --git a/test/jasmine/tests/bar_test.js b/test/jasmine/tests/bar_test.js index be35de30694..b1363b111ad 100644 --- a/test/jasmine/tests/bar_test.js +++ b/test/jasmine/tests/bar_test.js @@ -93,8 +93,8 @@ describe('heatmap calc / setPositions', function() { }); var plotinfo = { - x: function() { return gd._fullLayout.xaxis; }, - y: function() { return gd._fullLayout.yaxis; } + xaxis: gd._fullLayout.xaxis, + yaxis: gd._fullLayout.yaxis }; Bar.setPositions(gd, plotinfo); From a708381b4b21bb5a071b444d7d55b879d2561cb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 5 Oct 2016 15:27:49 -0400 Subject: [PATCH 14/15] make makeSubplotLayer agnostic to fullLayout - to allow calling outside the drawFramework step --- src/plots/cartesian/index.js | 59 ++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js index 7377caf749c..77aa2c6d2e8 100644 --- a/src/plots/cartesian/index.js +++ b/src/plots/cartesian/index.js @@ -167,27 +167,34 @@ exports.drawFramework = function(gd) { .data(subplotData, Lib.identity); subplotLayers.enter().append('g') - .classed('subplot', true); + .attr('class', function(name) { return 'subplot ' + name; }); subplotLayers.order(); subplotLayers.exit() .call(purgeSubplotLayers, fullLayout); - subplotLayers.each(function(subplot) { - var plotgroup = d3.select(this), - plotinfo = fullLayout._plots[subplot]; + subplotLayers.each(function(name) { + var plotinfo = fullLayout._plots[name]; + + // keep ref to plot group + plotinfo.plotgroup = d3.select(this); - // references to any subplots overlaid on this one, - // filled in makeSubplotLayer + // initialize list of overlay subplots plotinfo.overlays = []; - plotgroup.call(makeSubplotLayer, gd, subplot); + makeSubplotLayer(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 = joinLayer(fullLayout._draggers, 'g', subplot); + plotinfo.draglayer = joinLayer(fullLayout._draggers, 'g', name); }); }; @@ -225,6 +232,7 @@ function makeSubplotData(gd) { var mainplot = xa2._id + ya2._id; if(mainplot !== subplot && subplots.indexOf(mainplot) !== -1) { plotinfo.mainplot = mainplot; + plotinfo.mainplotinfo = fullLayout._plots[mainplot]; overlays.push(subplot); // for now force overlays to overlay completely... so they @@ -247,15 +255,9 @@ function makeSubplotData(gd) { return subplotData; } -function makeSubplotLayer(plotgroup, gd, subplot) { - var fullLayout = gd._fullLayout, - plotinfo = fullLayout._plots[subplot]; - - // keep reference to plotgroup in _plots object - plotinfo.plotgroup = plotgroup; - - // add class corresponding to the subplot id - plotgroup.classed(subplot, true); +function makeSubplotLayer(plotinfo) { + var plotgroup = plotinfo.plotgroup, + id = plotinfo.id; // Layers to keep plot types in the right order. // from back to front: @@ -264,7 +266,7 @@ function makeSubplotLayer(plotgroup, gd, subplot) { // 3. errorbars for bars and scatter // 4. scatter // 5. box plots - function plotLayers(parent) { + function joinPlotLayers(parent) { joinLayer(parent, 'g', 'imagelayer'); joinLayer(parent, 'g', 'maplayer'); joinLayer(parent, 'g', 'barlayer'); @@ -298,27 +300,25 @@ function makeSubplotLayer(plotgroup, gd, subplot) { plotinfo.overaxes = joinLayer(plotgroup, 'g', 'overaxes'); } else { + var mainplotinfo = plotinfo.mainplotinfo; // now make the components of overlaid subplots // overlays don't have backgrounds, and append all // their other components to the corresponding // extra groups of their main plots. - var mainplot = fullLayout._plots[plotinfo.mainplot]; - mainplot.overlays.push(plotinfo); + plotinfo.gridlayer = joinLayer(mainplotinfo.overgrid, 'g', id); + plotinfo.zerolinelayer = joinLayer(mainplotinfo.overzero, 'g', id); - plotinfo.gridlayer = joinLayer(mainplot.overgrid, 'g', subplot); - plotinfo.zerolinelayer = joinLayer(mainplot.overzero, 'g', subplot); - - plotinfo.plot = joinLayer(mainplot.overplot, 'g', subplot); - plotinfo.xlines = joinLayer(mainplot.overlines, 'path', subplot); - plotinfo.ylines = joinLayer(mainplot.overlines, 'path', subplot); - plotinfo.xaxislayer = joinLayer(mainplot.overaxes, 'g', subplot); - plotinfo.yaxislayer = joinLayer(mainplot.overaxes, 'g', subplot); + plotinfo.plot = joinLayer(mainplotinfo.overplot, 'g', id); + plotinfo.xlines = joinLayer(mainplotinfo.overlines, 'path', id); + plotinfo.ylines = joinLayer(mainplotinfo.overlines, 'path', id); + plotinfo.xaxislayer = joinLayer(mainplotinfo.overaxes, 'g', id); + plotinfo.yaxislayer = joinLayer(mainplotinfo.overaxes, 'g', id); } // common attributes for all subplots, overlays or not - plotinfo.plot.call(plotLayers); + plotinfo.plot.call(joinPlotLayers); plotinfo.xlines .style('fill', 'none') @@ -345,7 +345,6 @@ function purgeSubplotLayers(layers, fullLayout) { }); } - function joinLayer(parent, nodeType, className) { var layer = parent.selectAll('.' + className) .data([0]); From e78b0994c2a829f59f6612e70e6f9c0461eca058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 5 Oct 2016 22:29:39 -0400 Subject: [PATCH 15/15] rm useless return in makePlotFramework --- src/plot_api/plot_api.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 986b9cce4cb..44598d4a576 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -2794,6 +2794,4 @@ function makePlotFramework(gd) { fullLayout._hoverlayer = fullLayout._toppaper.append('g').classed('hoverlayer', true); gd.emit('plotly_framework'); - - return 'FRAMEWORK'; }