Skip to content

Combine geo and ternary genaral update pattern #1291

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.

Sign up for GitHub

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 8 commits into from
Jan 16, 2017
Merged

Combine geo and ternary genaral update pattern #1291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b3e25ac
fix jsdoc for Plots.getSubplotData
etpinard Jan 10, 2017
447abec
shorten parts to cartesian modules in geo.js
etpinard Jan 10, 2017
fe8a3f9
simplify subplot creation if-statement
etpinard Jan 10, 2017
92119bb
rm (now useless) logic for ternary make plot framework
etpinard Jan 10, 2017
a7cca2e
move getSubplotCalcdata method into Plots
etpinard Jan 10, 2017
fc0af0c
add generalUpdatePerTraceModule method in Plots
etpinard Jan 10, 2017
bfb05d4
make ternary use getSubplotCalcData & generalUpdatePerTraceModule
etpinard Jan 10, 2017
83c2722
replace unnecessary `=== undefined` check in Plots
etpinard Jan 16, 2017
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
60 changes: 4 additions & 56 deletions src/plots/geo/geo.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ var d3 = require('d3');

var Color = require('../../components/color');
var Drawing = require('../../components/drawing');
var Axes = require('../../plots/cartesian/axes');
var Fx = require('../../plots/cartesian/graph_interact');
var Plots = require('../plots');
var Axes = require('../cartesian/axes');
var Fx = require('../cartesian/graph_interact');

var addProjectionsToD3 = require('./projections');
var createGeoScale = require('./set_scale');
Expand Down Expand Up @@ -170,66 +171,13 @@ proto.plot = function(geoCalcData, fullLayout, promises) {
};

proto.onceTopojsonIsLoaded = function(geoCalcData, geoLayout) {
var i;

this.drawLayout(geoLayout);

var traceHashOld = this.traceHash;
var traceHash = {};

for(i = 0; i < geoCalcData.length; i++) {
var calcData = geoCalcData[i],
trace = calcData[0].trace;

traceHash[trace.type] = traceHash[trace.type] || [];
traceHash[trace.type].push(calcData);
}

var moduleNamesOld = Object.keys(traceHashOld);
var moduleNames = Object.keys(traceHash);

// when a trace gets deleted, make sure that its module's
// plot method is called so that it is properly
// removed from the DOM.
for(i = 0; i < moduleNamesOld.length; i++) {
var moduleName = moduleNamesOld[i];

if(moduleNames.indexOf(moduleName) === -1) {
var fakeCalcTrace = traceHashOld[moduleName][0],
fakeTrace = fakeCalcTrace[0].trace;

fakeTrace.visible = false;
traceHash[moduleName] = [fakeCalcTrace];
}
}

moduleNames = Object.keys(traceHash);

for(i = 0; i < moduleNames.length; i++) {
var moduleCalcData = traceHash[moduleNames[i]],
_module = moduleCalcData[0][0].trace._module;

_module.plot(this, filterVisible(moduleCalcData), geoLayout);
}

this.traceHash = traceHash;
Plots.generalUpdatePerTraceModule(this, geoCalcData, geoLayout);
Copy link
Contributor

Choose a reason for hiding this comment

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

Lots of trace-specific code moved elsewhere. I like it already. 👏


this.render();
};

function filterVisible(calcDataIn) {
var calcDataOut = [];

for(var i = 0; i < calcDataIn.length; i++) {
var calcTrace = calcDataIn[i],
trace = calcTrace[0].trace;

if(trace.visible === true) calcDataOut.push(calcTrace);
}

return calcDataOut;
}

proto.updateFx = function(hovermode) {
this.showHover = (hovermode !== false);

Expand Down
17 changes: 2 additions & 15 deletions src/plots/geo/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ exports.plot = function plotGeo(gd) {

for(var i = 0; i < geoIds.length; i++) {
var geoId = geoIds[i],
geoCalcData = getSubplotCalcData(calcData, geoId),
geoCalcData = Plots.getSubplotCalcData(calcData, 'geo', geoId),
geo = fullLayout[geoId]._subplot;

// If geo is not instantiated, create one!
if(geo === undefined) {
if(!geo) {
geo = new Geo({
id: geoId,
graphDiv: gd,
Expand Down Expand Up @@ -102,16 +102,3 @@ exports.toSVG = function(gd) {
.appendChild(geoFramework.node());
}
};

function getSubplotCalcData(calcData, id) {
var subplotCalcData = [];

for(var i = 0; i < calcData.length; i++) {
var calcTrace = calcData[i],
trace = calcTrace[0].trace;

if(trace.geo === id) subplotCalcData.push(calcTrace);
}

return subplotCalcData;
}
15 changes: 1 addition & 14 deletions src/plots/mapbox/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ exports.plot = function plotMapbox(gd) {

for(var i = 0; i < mapboxIds.length; i++) {
var id = mapboxIds[i],
subplotCalcData = getSubplotCalcData(calcData, id),
subplotCalcData = Plots.getSubplotCalcData(calcData, 'mapbox', id),
opts = fullLayout[id],
mapbox = opts._subplot;

Expand Down Expand Up @@ -118,19 +118,6 @@ exports.toSVG = function(gd) {
}
};

function getSubplotCalcData(calcData, id) {
var subplotCalcData = [];

for(var i = 0; i < calcData.length; i++) {
var calcTrace = calcData[i],
trace = calcTrace[0].trace;

if(trace.subplot === id) subplotCalcData.push(calcTrace);
}

return subplotCalcData;
}

function findAccessToken(gd, mapboxIds) {
var fullLayout = gd._fullLayout,
context = gd._context;
Expand Down
99 changes: 94 additions & 5 deletions src/plots/plots.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ plots.hasSimpleAPICommandBindings = commandModule.hasSimpleAPICommandBindings;
plots.findSubplotIds = function findSubplotIds(data, type) {
var subplotIds = [];

if(plots.subplotsRegistry[type] === undefined) return subplotIds;
if(!plots.subplotsRegistry[type]) return subplotIds;

var attr = plots.subplotsRegistry[type].attr;

Expand All @@ -90,7 +90,7 @@ plots.findSubplotIds = function findSubplotIds(data, type) {
plots.getSubplotIds = function getSubplotIds(layout, type) {
var _module = plots.subplotsRegistry[type];

if(_module === undefined) return [];
if(!_module) return [];

// layout must be 'fullLayout' here
if(type === 'cartesian' && (!layout._has || !layout._has('cartesian'))) return [];
Expand Down Expand Up @@ -124,14 +124,14 @@ plots.getSubplotIds = function getSubplotIds(layout, type) {
* Get the data trace(s) associated with a given subplot.
*
* @param {array} data plotly full data array.
* @param {object} layout plotly full layout object.
* @param {string} subplotId subplot ids to look for.
* @param {string} type subplot type to look for.
* @param {string} subplotId subplot id to look for.
*
* @return {array} list of trace objects.
*
*/
plots.getSubplotData = function getSubplotData(data, type, subplotId) {
if(plots.subplotsRegistry[type] === undefined) return [];
if(!plots.subplotsRegistry[type]) return [];

var attr = plots.subplotsRegistry[type].attr,
subplotData = [],
Expand All @@ -157,6 +157,31 @@ plots.getSubplotData = function getSubplotData(data, type, subplotId) {
return subplotData;
};

/**
* Get calcdata traces(s) associated with a given subplot
*
* @param {array} calcData (as in gd.calcdata)
* @param {string} type subplot type
* @param {string} subplotId subplot id to look for
*
* @return {array} array of calcdata traces
*/
plots.getSubplotCalcData = function(calcData, type, subplotId) {
if(!plots.subplotsRegistry[type]) return [];

var attr = plots.subplotsRegistry[type].attr;
var subplotCalcData = [];

for(var i = 0; i < calcData.length; i++) {
var calcTrace = calcData[i],
trace = calcTrace[0].trace;

if(trace[attr] === subplotId) subplotCalcData.push(calcTrace);
}

return subplotCalcData;
};

// in some cases the browser doesn't seem to know how big
// the text is at first, so it needs to draw it,
// then wait a little, then draw it again
Expand Down Expand Up @@ -2027,3 +2052,67 @@ plots.doCalcdata = function(gd, traces) {
calcdata[i] = cd;
}
};

plots.generalUpdatePerTraceModule = function(subplot, subplotCalcData, subplotLayout) {
var traceHashOld = subplot.traceHash,
Copy link
Contributor

Choose a reason for hiding this comment

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

I always feel like the mutate-existing-data approach contributes a bit to fragility, but I understand why these things are as they are.

traceHash = {},
i;

function filterVisible(calcDataIn) {
var calcDataOut = [];

for(var i = 0; i < calcDataIn.length; i++) {
var calcTrace = calcDataIn[i],
trace = calcTrace[0].trace;

if(trace.visible === true) calcDataOut.push(calcTrace);
}

return calcDataOut;
}

// build up moduleName -> calcData hash
for(i = 0; i < subplotCalcData.length; i++) {
var calcTraces = subplotCalcData[i],
trace = calcTraces[0].trace;

// skip over visible === false traces
// as they don't have `_module` ref
if(trace.visible) {
traceHash[trace.type] = traceHash[trace.type] || [];
traceHash[trace.type].push(calcTraces);
}
}

var moduleNamesOld = Object.keys(traceHashOld);
var moduleNames = Object.keys(traceHash);

// when a trace gets deleted, make sure that its module's
// plot method is called so that it is properly
// removed from the DOM.
Copy link
Contributor

Choose a reason for hiding this comment

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

👏 Yes.

for(i = 0; i < moduleNamesOld.length; i++) {
var moduleName = moduleNamesOld[i];

if(moduleNames.indexOf(moduleName) === -1) {
var fakeCalcTrace = traceHashOld[moduleName][0],
fakeTrace = fakeCalcTrace[0].trace;

fakeTrace.visible = false;
traceHash[moduleName] = [fakeCalcTrace];
}
}

// update list of module names to include 'fake' traces added above
moduleNames = Object.keys(traceHash);

// call module plot method
for(i = 0; i < moduleNames.length; i++) {
var moduleCalcData = traceHash[moduleNames[i]],
_module = moduleCalcData[0][0].trace._module;

_module.plot(subplot, filterVisible(moduleCalcData), subplotLayout);
}

// update moduleName -> calcData hash
subplot.traceHash = traceHash;
};
8 changes: 4 additions & 4 deletions src/plots/ternary/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,16 @@ exports.supplyLayoutDefaults = require('./layout/defaults');

exports.plot = function plotTernary(gd) {
var fullLayout = gd._fullLayout,
fullData = gd._fullData,
calcData = gd.calcdata,
ternaryIds = Plots.getSubplotIds(fullLayout, 'ternary');

for(var i = 0; i < ternaryIds.length; i++) {
var ternaryId = ternaryIds[i],
fullTernaryData = Plots.getSubplotData(fullData, 'ternary', ternaryId),
ternaryCalcData = Plots.getSubplotCalcData(calcData, 'ternary', ternaryId),
ternary = fullLayout[ternaryId]._subplot;

// If ternary is not instantiated, create one!
if(ternary === undefined) {
if(!ternary) {
ternary = new Ternary({
id: ternaryId,
graphDiv: gd,
Expand All @@ -53,7 +53,7 @@ exports.plot = function plotTernary(gd) {
fullLayout[ternaryId]._subplot = ternary;
}

ternary.plot(fullTernaryData, fullLayout, gd._promises);
ternary.plot(ternaryCalcData, fullLayout, gd._promises);
}
};

Expand Down
52 changes: 4 additions & 48 deletions src/plots/ternary/ternary.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ var Color = require('../../components/color');
var Drawing = require('../../components/drawing');
var setConvert = require('../cartesian/set_convert');
var extendFlat = require('../../lib/extend').extendFlat;
var Plots = require('../plots');
var Axes = require('../cartesian/axes');
var dragElement = require('../../components/dragelement');
var Titles = require('../../components/titles');
Expand All @@ -44,59 +45,14 @@ proto.init = function(fullLayout) {
this.traceHash = {};
};

proto.plot = function(ternaryData, fullLayout) {
proto.plot = function(ternaryCalcData, fullLayout) {
var _this = this,
ternaryLayout = fullLayout[_this.id],
graphSize = fullLayout._size,
i;

if(Lib.getPlotDiv(_this.plotContainer.node()) !== _this.graphDiv) {
// someone deleted the framework - remake it
// TODO: this is getting deleted in (cartesian) makePlotFramework
// turn that into idiomatic d3 (enter/exit, the piece I didn't know
// before was ordering selections) so we don't need this.
_this.init(_this.graphDiv._fullLayout);
_this.makeFramework();
}
graphSize = fullLayout._size;

_this.adjustLayout(ternaryLayout, graphSize);

var traceHashOld = _this.traceHash;
var traceHash = {};

for(i = 0; i < ternaryData.length; i++) {
var trace = ternaryData[i];

traceHash[trace.type] = traceHash[trace.type] || [];
traceHash[trace.type].push(trace);
}

var moduleNamesOld = Object.keys(traceHashOld);
var moduleNames = Object.keys(traceHash);

// when a trace gets deleted, make sure that its module's
// plot method is called so that it is properly
// removed from the DOM.
for(i = 0; i < moduleNamesOld.length; i++) {
var moduleName = moduleNamesOld[i];

if(moduleNames.indexOf(moduleName) === -1) {
var fakeModule = traceHashOld[moduleName][0];
fakeModule.visible = false;
traceHash[moduleName] = [fakeModule];
}
}

moduleNames = Object.keys(traceHash);

for(i = 0; i < moduleNames.length; i++) {
var moduleData = traceHash[moduleNames[i]];
var _module = moduleData[0]._module;

_module.plot(_this, Lib.filterVisible(moduleData), ternaryLayout);
}

_this.traceHash = traceHash;
Plots.generalUpdatePerTraceModule(_this, ternaryCalcData, ternaryLayout);

_this.layers.plotbg.select('path').call(Color.fill, ternaryLayout.bgcolor);
};
Expand Down
Loading