Skip to content

Replace _has<PlotType> variables with '_has()' fullLayout method #491

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 27 commits into from
May 11, 2016
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
88f552a
mv gd._modules -> fullLayout._modules
etpinard Apr 27, 2016
525e3c7
lint
etpinard Apr 27, 2016
876ce4b
attach has (plot type) method to fullLayout,
etpinard Apr 27, 2016
f45ffec
add tempory block _has() -> _hasCartesian etc.
etpinard Apr 27, 2016
4e0c6da
rm _has??? attributes
etpinard Apr 27, 2016
93493a6
make 'pie' a layout category,
etpinard Apr 27, 2016
e63ba4a
check for hasCartesian && !hasPie for 'hovermode',
etpinard Apr 28, 2016
b655cb0
Revert "check for hasCartesian && !hasPie for 'hovermode',"
etpinard May 3, 2016
f476e05
split pie trace base plot module from cartesian:
etpinard May 3, 2016
1c41c2e
rm (now useless) trace module layoutCategories
etpinard May 3, 2016
313150e
add fill unique lib function,
etpinard May 6, 2016
392619c
fill fullLayout._basePlotModules list in default trace loop,
etpinard May 6, 2016
3787d47
loop over basePlotModules instead of subplotRegistry in:
etpinard May 6, 2016
68b12c4
make sure to add cartesian base plot module to orphan axis graphs:
etpinard May 6, 2016
a41427c
move 3d axes init step from makePlotFrameork -> Gl3d.plot
etpinard May 6, 2016
a48d071
check for non-cartesian/pie/ternary base plot module in restyle,
etpinard May 6, 2016
a78521c
handle a few polar-only cases:
etpinard May 6, 2016
aa2ec8b
sub has??? -> has('???')
etpinard May 6, 2016
77fb7e2
loop over (trace) modules in trace supplyLayoutDefaults:
etpinard May 6, 2016
a141d78
robustify pie clean step:
etpinard May 6, 2016
dc297ca
put ref of hasPlotType method on Plots:
etpinard May 6, 2016
e3f72f0
move block of _has?? = _has('') after all defaults are filled:
etpinard May 6, 2016
c59a960
update tests
etpinard May 6, 2016
9f94494
add info about data/layout and fullData/fullLayout
etpinard May 6, 2016
9837d8b
Merge branch 'master' into has-plot-type
etpinard May 6, 2016
0c7b2f8
rename Lib.fillUnique -> Lib.pushUnique
etpinard May 9, 2016
5e6759b
rm unnecessary boolean type coercion
etpinard May 9, 2016
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
4 changes: 2 additions & 2 deletions src/components/modebar/buttons.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ function handleCartesian(gd, ev) {

Plotly.relayout(gd, aobj).then(function() {
if(astr === 'dragmode') {
if(fullLayout._hasCartesian) {
if(fullLayout._has('cartesian')) {
setCursor(
fullLayout._paper.select('.nsewdrag'),
DRAGCURSORS[val]
Expand Down Expand Up @@ -500,7 +500,7 @@ function toggleHover(gd) {
var fullLayout = gd._fullLayout;

var onHoverVal;
if(fullLayout._hasCartesian) {
if(fullLayout._has('cartesian')) {
onHoverVal = fullLayout._isHoriz ? 'y' : 'x';
}
else onHoverVal = 'closest';
Expand Down
12 changes: 6 additions & 6 deletions src/components/modebar/manage.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,12 @@ function getButtonGroups(gd, buttonsToRemove, buttonsToAdd) {
var fullLayout = gd._fullLayout,
fullData = gd._fullData;

var hasCartesian = !!fullLayout._hasCartesian,
hasGL3D = !!fullLayout._hasGL3D,
hasGeo = !!fullLayout._hasGeo,
hasPie = !!fullLayout._hasPie,
hasGL2D = !!fullLayout._hasGL2D,
hasTernary = !!fullLayout._hasTernary;
var hasCartesian = !!fullLayout._has('cartesian'),
Copy link
Contributor

Choose a reason for hiding this comment

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

The !! shouldn't be necessary here anymore, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct. nice catch.

hasGL3D = !!fullLayout._has('gl3d'),
hasGeo = !!fullLayout._has('geo'),
hasPie = !!fullLayout._has('pie'),
hasGL2D = !!fullLayout._has('gl2d'),
hasTernary = !!fullLayout._has('ternary');

var groups = [];

Expand Down
2 changes: 1 addition & 1 deletion src/components/rangeslider/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ function draw(gd) {
var height = (fullLayout.height - fullLayout.margin.b - fullLayout.margin.t) * options.thickness,
offsetShift = Math.floor(options.borderwidth / 2);

if(sliderContainer[0].length === 0 && !fullLayout._hasGL2D) createSlider(gd);
if(sliderContainer[0].length === 0 && !fullLayout._has('gl2d')) createSlider(gd);

// Need to default to 0 for when making gl plots
var bb = fullLayout.xaxis._boundingBox ?
Expand Down
17 changes: 17 additions & 0 deletions src/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,23 @@ lib.noneOrAll = function(containerIn, containerOut, attrList) {
}
};

/**
* Fill with unique items
*
* @param {array} array
* array to be filled
* @param {any} item
* item to be or not to be inserted
* @return {array}
* ref to array (now possibly containing one more item)
*
*/
lib.fillUnique = function(array, item) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe inconsequential, but what do you think of lib.addUnique instead?

In my head, fillUnique implies filling an entire array of some length.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or perhaps pushUnique

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1 pushUnique. Thanks

if(item && array.indexOf(item) === -1) array.push(item);

return array;
};

lib.mergeArray = function(traceAttr, cd, cdAttr) {
if(Array.isArray(traceAttr)) {
var imax = Math.min(traceAttr.length, cd.length);
Expand Down
39 changes: 16 additions & 23 deletions src/plot_api/plot_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ Plotly.plot = function(gd, data, layout, config) {
if(!recalc) return;

var subplots = Plots.getSubplotIds(fullLayout, 'cartesian'),
modules = gd._modules;
modules = fullLayout._modules;

// position and range calculations for traces that
// depend on each other ie bars (stacked or grouped)
Expand Down Expand Up @@ -250,11 +250,12 @@ Plotly.plot = function(gd, data, layout, config) {

// Now plot the data
function drawData() {
var calcdata = gd.calcdata;
var calcdata = gd.calcdata,
i;

// in case of traces that were heatmaps or contour maps
// previously, remove them and their colorbars explicitly
for(var i = 0; i < calcdata.length; i++) {
for(i = 0; i < calcdata.length; i++) {
var trace = calcdata[i][0].trace,
isVisible = (trace.visible === true),
uid = trace.uid;
Expand All @@ -272,18 +273,11 @@ Plotly.plot = function(gd, data, layout, config) {
}
}

var plotRegistry = Plots.subplotsRegistry;

if(fullLayout._hasGL3D) plotRegistry.gl3d.plot(gd);
if(fullLayout._hasGeo) plotRegistry.geo.plot(gd);
if(fullLayout._hasGL2D) plotRegistry.gl2d.plot(gd);
if(fullLayout._hasCartesian || fullLayout._hasPie) {
plotRegistry.cartesian.plot(gd);
// loop over the base plot modules present on graph
var basePlotModules = fullLayout._basePlotModules;
for(i = 0; i < basePlotModules.length; i++) {
basePlotModules[i].plot(gd);
}
if(fullLayout._hasTernary) plotRegistry.ternary.plot(gd);

// clean up old scenes that no longer have associated data
// will this be a performance hit?

// styling separate from drawing
Plots.style(gd);
Expand Down Expand Up @@ -1660,10 +1654,12 @@ Plotly.restyle = function restyle(gd, astr, val, traces) {
axlist,
flagAxForDelete = {};

// for now, if we detect gl or geo stuff, just re-do the plot
if(fullLayout._hasGL3D || fullLayout._hasGeo || fullLayout._hasGL2D) {
doplot = true;
}
// At the moment, only cartesian, pie and ternary plot types can afford
// to not go through a full replot
var doPlotWhiteList = ['cartesian', 'pie', 'ternary'];
fullLayout._basePlotModules.forEach(function(_module) {
if(doPlotWhiteList.indexOf(_module.name) === -1) doplot = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In restyle, which is more general than

if(fullLayout._hasGL3D || fullLayout._hasGeo || fullLayout._hasGL2D) doplot = true;

in an effort to support custom basePlotModules

});

// make a new empty vals array for undoit
function a0() { return traces.map(function() { return undefined; }); }
Expand Down Expand Up @@ -2308,7 +2304,7 @@ Plotly.relayout = function relayout(gd, astr, val) {
if(p.parts[0].indexOf('scene') === 0) doplot = true;
else if(p.parts[0].indexOf('geo') === 0) doplot = true;
else if(p.parts[0].indexOf('ternary') === 0) doplot = true;
else if(fullLayout._hasGL2D &&
else if(fullLayout._has('gl2d') &&
(ai.indexOf('axis') !== -1 || p.parts[0] === 'plot_bgcolor')
) doplot = true;
else if(ai === 'hiddenlabels') docalc = true;
Expand Down Expand Up @@ -2581,9 +2577,6 @@ function makePlotFramework(gd) {
var gd3 = d3.select(gd),
fullLayout = gd._fullLayout;

// TODO - find a better place for 3D to initialize axes
if(fullLayout._hasGL3D) Plots.subplotsRegistry.gl3d.initAxes(gd);

// Plot container
fullLayout._container = gd3.selectAll('.plot-container').data([0]);
fullLayout._container.enter().insert('div', ':first-child')
Expand Down Expand Up @@ -2658,7 +2651,7 @@ function makePlotFramework(gd) {
makeSubplots(gd, subplots);
}

if(fullLayout._hasCartesian) makeCartesianPlotFramwork(gd, subplots);
if(fullLayout._has('cartesian')) makeCartesianPlotFramwork(gd, subplots);

// single ternary layer for the whole plot
fullLayout._ternarylayer = fullLayout._paper.append('g').classed('ternarylayer', true);
Expand Down
2 changes: 2 additions & 0 deletions src/plot_api/plot_schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,8 @@ function handleSubplotObjs(layoutAttributes) {
var subplotRegistry = subplotsRegistry[subplotType],
isSubplotObj;

if(!subplotRegistry.attrRegex) return;

if(subplotType === 'cartesian' || subplotType === 'gl2d') {
isSubplotObj = (
subplotRegistry.attrRegex.x.test(k) ||
Expand Down
4 changes: 2 additions & 2 deletions src/plots/cartesian/axes.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ axes.getFromTrace = axisIds.getFromTrace;
// find the list of possible axes to reference with an xref or yref attribute
// and coerce it to that list
axes.coerceRef = function(containerIn, containerOut, gd, axLetter) {
var axlist = gd._fullLayout._hasGL2D ? [] : axes.listIds(gd, axLetter),
var axlist = gd._fullLayout._has('gl2d') ? [] : axes.listIds(gd, axLetter),
refAttr = axLetter + 'ref',
attrDef = {};

Expand Down Expand Up @@ -1797,7 +1797,7 @@ axes.doTicks = function(gd, axid, skipTitle) {
var alldone = axes.getSubplots(gd, ax).map(function(subplot) {
var plotinfo = fullLayout._plots[subplot];

if(!fullLayout._hasCartesian) return;
if(!fullLayout._has('cartesian')) return;

var container = plotinfo[axletter + 'axislayer'],

Expand Down
6 changes: 3 additions & 3 deletions src/plots/cartesian/graph_interact.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ fx.supplyLayoutDefaults = function(layoutIn, layoutOut, fullData) {
coerce('dragmode');

var hovermodeDflt;
if(layoutOut._hasCartesian) {
if(layoutOut._has('cartesian')) {
// flag for 'horizontal' plots:
// determines the state of the mode bar 'compare' hovermode button
var isHoriz = layoutOut._isHoriz = fx.isHoriz(fullData);
Expand Down Expand Up @@ -89,7 +89,7 @@ fx.isHoriz = function(fullData) {
fx.init = function(gd) {
var fullLayout = gd._fullLayout;

if(!fullLayout._hasCartesian || gd._context.staticPlot) return;
if(!fullLayout._has('cartesian') || gd._context.staticPlot) return;

var subplots = Object.keys(fullLayout._plots || {}).sort(function(a,b) {
// sort overlays last, then by x axis number, then y axis number
Expand All @@ -107,7 +107,7 @@ fx.init = function(gd) {
subplots.forEach(function(subplot) {
var plotinfo = fullLayout._plots[subplot];

if(!fullLayout._hasCartesian) return;
if(!fullLayout._has('cartesian')) return;

var xa = plotinfo.x(),
ya = plotinfo.y(),
Expand Down
19 changes: 1 addition & 18 deletions src/plots/cartesian/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ exports.plot = function(gd) {
var fullLayout = gd._fullLayout,
subplots = Plots.getSubplotIds(fullLayout, 'cartesian'),
calcdata = gd.calcdata,
modules = gd._modules;
modules = fullLayout._modules;

function getCdSubplot(calcdata, subplot) {
var cdSubplot = [];
Expand Down Expand Up @@ -78,28 +78,11 @@ exports.plot = function(gd) {
// skip over non-cartesian trace modules
if(_module.basePlotModule.name !== 'cartesian') continue;

// skip over pies, there are drawn below
if(_module.name === 'pie') continue;

// plot all traces of this type on this subplot at once
var cdModule = getCdModule(cdSubplot, _module);
_module.plot(gd, subplotInfo, cdModule);

Lib.markTime('done ' + (cdModule[0] && cdModule[0][0].trace.type));
}
}

// now draw stuff not on subplots (ie, only pies at the moment)
if(fullLayout._hasPie) {
var Pie = Plots.getModule('pie');
var cdPie = getCdModule(calcdata, Pie);

if(cdPie.length) Pie.plot(gd, cdPie);
}
};

exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) {
if(oldFullLayout._hasPie && !newFullLayout._hasPie) {
oldFullLayout._pielayer.selectAll('g.trace').remove();
}
};
4 changes: 2 additions & 2 deletions src/plots/cartesian/layout_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
// if gl3d or geo is present on graph. This is retain backward compatible.
//
// TODO drop this in version 2.0
var ignoreOrphan = (layoutOut._hasGL3D || layoutOut._hasGeo);
var ignoreOrphan = (layoutOut._has('gl3d') || layoutOut._has('geo'));

if(!ignoreOrphan) {
for(i = 0; i < layoutKeys.length; i++) {
Expand All @@ -95,7 +95,7 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
// make sure that plots with orphan cartesian axes
// are considered 'cartesian'
if(xaListCartesian.length && yaListCartesian.length) {
layoutOut._hasCartesian = true;
Lib.fillUnique(layoutOut._basePlotModules, Plots.subplotsRegistry.cartesian);
}

function axSort(a, b) {
Expand Down
2 changes: 0 additions & 2 deletions src/plots/geo/layout/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ var supplyGeoAxisLayoutDefaults = require('./axis_defaults');


module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
if(!layoutOut._hasGeo) return;

handleSubplotDefaults(layoutIn, layoutOut, fullData, {
type: 'geo',
attributes: layoutAttributes,
Expand Down
28 changes: 12 additions & 16 deletions src/plots/gl3d/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,13 @@ exports.plot = function plotGl3d(gd) {
for(var i = 0; i < sceneIds.length; i++) {
var sceneId = sceneIds[i],
fullSceneData = Plots.getSubplotData(fullData, 'gl3d', sceneId),
scene = fullLayout[sceneId]._scene; // ref. to corresp. Scene instance
sceneLayout = fullLayout[sceneId],
scene = sceneLayout._scene;

// If Scene is not instantiated, create one!
if(scene === undefined) {
initAxes(gd, sceneLayout);

scene = new Scene({
id: sceneId,
graphDiv: gd,
Expand All @@ -60,10 +63,11 @@ exports.plot = function plotGl3d(gd) {
fullLayout
);

fullLayout[sceneId]._scene = scene; // set ref to Scene instance
// set ref to Scene instance
sceneLayout._scene = scene;
}

scene.plot(fullSceneData, fullLayout, gd.layout); // takes care of business
scene.plot(fullSceneData, fullLayout, gd.layout);
}
};

Expand Down Expand Up @@ -91,18 +95,10 @@ exports.cleanId = function cleanId(id) {

exports.setConvert = require('./set_convert');

exports.initAxes = function(gd) {
var fullLayout = gd._fullLayout;
var sceneIds = Plots.getSubplotIds(fullLayout, 'gl3d');

for(var i = 0; i < sceneIds.length; ++i) {
var sceneId = sceneIds[i];
var sceneLayout = fullLayout[sceneId];
function initAxes(gd, sceneLayout) {
for(var j = 0; j < 3; ++j) {
var axisName = axesNames[j];

for(var j = 0; j < 3; ++j) {
var axisName = axesNames[j];
var ax = sceneLayout[axisName];
ax._gd = gd;
}
sceneLayout[axisName]._gd = gd;
}
};
}
12 changes: 5 additions & 7 deletions src/plots/gl3d/layout/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,12 @@ var supplyGl3dAxisLayoutDefaults = require('./axis_defaults');


module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
if(!layoutOut._hasGL3D) return;

var hasNon3D = (
layoutOut._hasCartesian ||
layoutOut._hasGeo ||
layoutOut._hasGL2D ||
layoutOut._hasPie ||
layoutOut._hasTernary
layoutOut._has('cartesian') ||
layoutOut._has('geo') ||
layoutOut._has('gl2d') ||
layoutOut._has('pie') ||
layoutOut._has('ternary')
);

// some layout-wide attribute are used in all scenes
Expand Down
25 changes: 1 addition & 24 deletions src/plots/layout_attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,30 +168,7 @@ module.exports = {
role: 'info',
description: 'Determines whether or not a legend is drawn.'
},
_hasCartesian: {
valType: 'boolean',
dflt: false
},
_hasGL3D: {
valType: 'boolean',
dflt: false
},
_hasGeo: {
valType: 'boolean',
dflt: false
},
_hasPie: {
valType: 'boolean',
dflt: false
},
_hasGL2D: {
valType: 'boolean',
dflt: false
},
_hasTernary: {
valType: 'boolean',
dflt: false
},

_composedModules: {
'*': 'Fx'
},
Expand Down
Loading