refactor make_trace_groups to more general lib function

instead of cartesian-specific

Author: alexcjohnson

src/lib/index.js

@@ -118,6 +118,8 @@ lib.clearThrottle = throttleModule.clear;
 
 lib.getGraphDiv = require('./get_graph_div');
 
+lib.makeTraceGroups = require('./make_trace_groups');
+
 lib._ = require('./localize');
 
 lib.notifier = require('./notifier');

src/plots/cartesian/make_trace_groups.js renamed to src/lib/make_trace_groups.js

@@ -9,28 +9,18 @@
 
 'use strict';
 
-var d3 = require('d3');
-
 /**
- * helper for cartesian trace types to manage trace groups and plot traces
- * into them
+ * General helper to manage trace groups based on calcdata
  *
- * @param {div} gd: plot div
- * @param {object} plotinfo: the cartesian subplot info object
- * @param {array} cdModule: array of calcdata items for this
- *     module and subplot combination
  * @param {d3.selection} traceLayer: a selection containing a single group
  *     to draw these traces into
+ * @param {array} cdModule: array of calcdata items for this
+ *     module and subplot combination. Assumes the calcdata item for each
+ *     trace is an array with the fullData trace attached to the first item.
  * @param {string} cls: the class attribute to give each trace group
  *     so you can give multiple classes separated by spaces
- * @param {function} plotOneFn: a function that will plot one trace
- *     takes arguments:
- *         gd
- *         plotinfo
- *         cd: calcdata array for this one trace
- *         plotGroup: d3 selection of the single group to draw into
  */
-module.exports = function makeTraceGroups(gd, plotinfo, cdModule, traceLayer, cls, plotOneFn) {
+module.exports = function makeTraceGroups(traceLayer, cdModule, cls) {
     var traces = traceLayer.selectAll('g.' + cls.replace(/\s/g, '.'))
         .data(cdModule, function(cd) { return cd[0].trace.uid; });
 
@@ -39,10 +29,7 @@ module.exports = function makeTraceGroups(gd, plotinfo, cdModule, traceLayer, cl
     traces.enter().append('g')
         .attr('class', cls);
 
-    traces.each(function(cd) {
-        plotOneFn(gd, plotinfo, cd, d3.select(this));
-    })
-    .order();
+    traces.order();
 
     return traces;
 };

src/traces/bar/plot.js

@@ -19,7 +19,6 @@ var svgTextUtils = require('../../lib/svg_text_utils');
 var Color = require('../../components/color');
 var Drawing = require('../../components/drawing');
 var Registry = require('../../registry');
-var makeTraceGroups = require('../../plots/cartesian/make_trace_groups');
 
 var attributes = require('./attributes'),
     attributeText = attributes.text,
@@ -32,128 +31,128 @@ var attributes = require('./attributes'),
 var TEXTPAD = 3;
 
 module.exports = function plot(gd, plotinfo, cdbar, barLayer) {
-    var bartraces = makeTraceGroups(gd, plotinfo, cdbar, barLayer, 'trace bars', plotOne);
-
-    // error bars are on the top
-    Registry.getComponentMethod('errorbars', 'plot')(bartraces, plotinfo);
-};
-
-function plotOne(gd, plotinfo, cd, plotGroup) {
     var xa = plotinfo.xaxis;
     var ya = plotinfo.yaxis;
     var fullLayout = gd._fullLayout;
-    var cd0 = cd[0];
-    var t = cd0.t;
-    var trace = cd0.trace;
 
-    if(!plotinfo.isRangePlot) cd0.node3 = plotGroup;
+    var bartraces = Lib.makeTraceGroups(barLayer, cdbar, 'trace bars').each(function(cd) {
+        var plotGroup = d3.select(this);
+        var cd0 = cd[0];
+        var t = cd0.t;
+        var trace = cd0.trace;
 
-    var poffset = t.poffset;
-    var poffsetIsArray = Array.isArray(poffset);
+        if(!plotinfo.isRangePlot) cd0.node3 = plotGroup;
 
-    var pointGroup = Lib.ensureSingle(plotGroup, 'g', 'points');
+        var poffset = t.poffset;
+        var poffsetIsArray = Array.isArray(poffset);
 
-    var bars = pointGroup.selectAll('g.point').data(Lib.identity);
+        var pointGroup = Lib.ensureSingle(plotGroup, 'g', 'points');
 
-    bars.enter().append('g')
-        .classed('point', true);
+        var bars = pointGroup.selectAll('g.point').data(Lib.identity);
 
-    bars.exit().remove();
+        bars.enter().append('g')
+            .classed('point', true);
 
-    bars.each(function(di, i) {
-        var bar = d3.select(this);
+        bars.exit().remove();
 
-        // now display the bar
-        // clipped xf/yf (2nd arg true): non-positive
-        // log values go off-screen by plotwidth
-        // so you see them continue if you drag the plot
-        var p0 = di.p + ((poffsetIsArray) ? poffset[i] : poffset),
-            p1 = p0 + di.w,
-            s0 = di.b,
-            s1 = s0 + di.s;
+        bars.each(function(di, i) {
+            var bar = d3.select(this);
 
-        var x0, x1, y0, y1;
-        if(trace.orientation === 'h') {
-            y0 = ya.c2p(p0, true);
-            y1 = ya.c2p(p1, true);
-            x0 = xa.c2p(s0, true);
-            x1 = xa.c2p(s1, true);
+            // now display the bar
+            // clipped xf/yf (2nd arg true): non-positive
+            // log values go off-screen by plotwidth
+            // so you see them continue if you drag the plot
+            var p0 = di.p + ((poffsetIsArray) ? poffset[i] : poffset),
+                p1 = p0 + di.w,
+                s0 = di.b,
+                s1 = s0 + di.s;
 
-            // for selections
-            di.ct = [x1, (y0 + y1) / 2];
-        }
-        else {
-            x0 = xa.c2p(p0, true);
-            x1 = xa.c2p(p1, true);
-            y0 = ya.c2p(s0, true);
-            y1 = ya.c2p(s1, true);
+            var x0, x1, y0, y1;
+            if(trace.orientation === 'h') {
+                y0 = ya.c2p(p0, true);
+                y1 = ya.c2p(p1, true);
+                x0 = xa.c2p(s0, true);
+                x1 = xa.c2p(s1, true);
 
-            // for selections
-            di.ct = [(x0 + x1) / 2, y1];
-        }
+                // for selections
+                di.ct = [x1, (y0 + y1) / 2];
+            }
+            else {
+                x0 = xa.c2p(p0, true);
+                x1 = xa.c2p(p1, true);
+                y0 = ya.c2p(s0, true);
+                y1 = ya.c2p(s1, true);
 
-        if(!isNumeric(x0) || !isNumeric(x1) ||
-                !isNumeric(y0) || !isNumeric(y1) ||
-                x0 === x1 || y0 === y1) {
-            bar.remove();
-            return;
-        }
+                // for selections
+                di.ct = [(x0 + x1) / 2, y1];
+            }
+
+            if(!isNumeric(x0) || !isNumeric(x1) ||
+                    !isNumeric(y0) || !isNumeric(y1) ||
+                    x0 === x1 || y0 === y1) {
+                bar.remove();
+                return;
+            }
 
-        var lw = (di.mlw + 1 || trace.marker.line.width + 1 ||
-                (di.trace ? di.trace.marker.line.width : 0) + 1) - 1,
-            offset = d3.round((lw / 2) % 1, 2);
+            var lw = (di.mlw + 1 || trace.marker.line.width + 1 ||
+                    (di.trace ? di.trace.marker.line.width : 0) + 1) - 1,
+                offset = d3.round((lw / 2) % 1, 2);
 
-        function roundWithLine(v) {
-            // if there are explicit gaps, don't round,
-            // it can make the gaps look crappy
-            return (fullLayout.bargap === 0 && fullLayout.bargroupgap === 0) ?
-                d3.round(Math.round(v) - offset, 2) : v;
-        }
+            function roundWithLine(v) {
+                // if there are explicit gaps, don't round,
+                // it can make the gaps look crappy
+                return (fullLayout.bargap === 0 && fullLayout.bargroupgap === 0) ?
+                    d3.round(Math.round(v) - offset, 2) : v;
+            }
 
-        function expandToVisible(v, vc) {
-            // if it's not in danger of disappearing entirely,
-            // round more precisely
-            return Math.abs(v - vc) >= 2 ? roundWithLine(v) :
-            // but if it's very thin, expand it so it's
-            // necessarily visible, even if it might overlap
-            // its neighbor
-            (v > vc ? Math.ceil(v) : Math.floor(v));
-        }
+            function expandToVisible(v, vc) {
+                // if it's not in danger of disappearing entirely,
+                // round more precisely
+                return Math.abs(v - vc) >= 2 ? roundWithLine(v) :
+                // but if it's very thin, expand it so it's
+                // necessarily visible, even if it might overlap
+                // its neighbor
+                (v > vc ? Math.ceil(v) : Math.floor(v));
+            }
 
-        if(!gd._context.staticPlot) {
-            // if bars are not fully opaque or they have a line
-            // around them, round to integer pixels, mainly for
-            // safari so we prevent overlaps from its expansive
-            // pixelation. if the bars ARE fully opaque and have
-            // no line, expand to a full pixel to make sure we
-            // can see them
-            var op = Color.opacity(di.mc || trace.marker.color),
-                fixpx = (op < 1 || lw > 0.01) ?
-                    roundWithLine : expandToVisible;
-            x0 = fixpx(x0, x1);
-            x1 = fixpx(x1, x0);
-            y0 = fixpx(y0, y1);
-            y1 = fixpx(y1, y0);
-        }
+            if(!gd._context.staticPlot) {
+                // if bars are not fully opaque or they have a line
+                // around them, round to integer pixels, mainly for
+                // safari so we prevent overlaps from its expansive
+                // pixelation. if the bars ARE fully opaque and have
+                // no line, expand to a full pixel to make sure we
+                // can see them
+                var op = Color.opacity(di.mc || trace.marker.color),
+                    fixpx = (op < 1 || lw > 0.01) ?
+                        roundWithLine : expandToVisible;
+                x0 = fixpx(x0, x1);
+                x1 = fixpx(x1, x0);
+                y0 = fixpx(y0, y1);
+                y1 = fixpx(y1, y0);
+            }
 
-        Lib.ensureSingle(bar, 'path')
-            .style('vector-effect', 'non-scaling-stroke')
-            .attr('d',
-                'M' + x0 + ',' + y0 + 'V' + y1 + 'H' + x1 + 'V' + y0 + 'Z')
-            .call(Drawing.setClipUrl, plotinfo.layerClipId);
+            Lib.ensureSingle(bar, 'path')
+                .style('vector-effect', 'non-scaling-stroke')
+                .attr('d',
+                    'M' + x0 + ',' + y0 + 'V' + y1 + 'H' + x1 + 'V' + y0 + 'Z')
+                .call(Drawing.setClipUrl, plotinfo.layerClipId);
 
-        appendBarText(gd, bar, cd, i, x0, x1, y0, y1);
+            appendBarText(gd, bar, cd, i, x0, x1, y0, y1);
 
-        if(plotinfo.layerClipId) {
-            Drawing.hideOutsideRangePoint(di, bar.select('text'), xa, ya, trace.xcalendar, trace.ycalendar);
-        }
+            if(plotinfo.layerClipId) {
+                Drawing.hideOutsideRangePoint(di, bar.select('text'), xa, ya, trace.xcalendar, trace.ycalendar);
+            }
+        });
+
+        // lastly, clip points groups of `cliponaxis !== false` traces
+        // on `plotinfo._hasClipOnAxisFalse === true` subplots
+        var hasClipOnAxisFalse = cd0.trace.cliponaxis === false;
+        Drawing.setClipUrl(plotGroup, hasClipOnAxisFalse ? null : plotinfo.layerClipId);
     });
 
-    // lastly, clip points groups of `cliponaxis !== false` traces
-    // on `plotinfo._hasClipOnAxisFalse === true` subplots
-    var hasClipOnAxisFalse = cd0.trace.cliponaxis === false;
-    Drawing.setClipUrl(plotGroup, hasClipOnAxisFalse ? null : plotinfo.layerClipId);
-}
+    // error bars are on the top
+    Registry.getComponentMethod('errorbars', 'plot')(bartraces, plotinfo);
+};
 
 function appendBarText(gd, bar, calcTrace, i, x0, x1, y0, y1) {
     var textPosition;

src/traces/box/plot.js

@@ -12,62 +12,59 @@ var d3 = require('d3');
 
 var Lib = require('../../lib');
 var Drawing = require('../../components/drawing');
-var makeTraceGroups = require('../../plots/cartesian/make_trace_groups');
 
 // constants for dynamic jitter (ie less jitter for sparser points)
 var JITTERCOUNT = 5; // points either side of this to include
 var JITTERSPREAD = 0.01; // fraction of IQR to count as "dense"
 
 function plot(gd, plotinfo, cdbox, boxLayer) {
-    makeTraceGroups(gd, plotinfo, cdbox, boxLayer, 'trace boxes', plotOne);
-}
-
-function plotOne(gd, plotinfo, cd, plotGroup) {
     var fullLayout = gd._fullLayout;
     var xa = plotinfo.xaxis;
     var ya = plotinfo.yaxis;
-    var cd0 = cd[0];
-    var t = cd0.t;
-    var trace = cd0.trace;
-    if(!plotinfo.isRangePlot) cd0.node3 = plotGroup;
     var numBoxes = fullLayout._numBoxes;
-
     var groupFraction = (1 - fullLayout.boxgap);
-
     var group = (fullLayout.boxmode === 'group' && numBoxes > 1);
-    // box half width
-    var bdPos = t.dPos * groupFraction * (1 - fullLayout.boxgroupgap) / (group ? numBoxes : 1);
-    // box center offset
-    var bPos = group ? 2 * t.dPos * (-0.5 + (t.num + 0.5) / numBoxes) * groupFraction : 0;
-    // whisker width
-    var wdPos = bdPos * trace.whiskerwidth;
-
-    if(trace.visible !== true || t.empty) {
-        plotGroup.remove();
-        return;
-    }
 
-    var posAxis, valAxis;
+    Lib.makeTraceGroups(boxLayer, cdbox, 'trace boxes').each(function(cd) {
+        var plotGroup = d3.select(this);
+        var cd0 = cd[0];
+        var t = cd0.t;
+        var trace = cd0.trace;
+        if(!plotinfo.isRangePlot) cd0.node3 = plotGroup;
+        // box half width
+        var bdPos = t.dPos * groupFraction * (1 - fullLayout.boxgroupgap) / (group ? numBoxes : 1);
+        // box center offset
+        var bPos = group ? 2 * t.dPos * (-0.5 + (t.num + 0.5) / numBoxes) * groupFraction : 0;
+        // whisker width
+        var wdPos = bdPos * trace.whiskerwidth;
+
+        if(trace.visible !== true || t.empty) {
+            plotGroup.remove();
+            return;
+        }
 
-    if(trace.orientation === 'h') {
-        posAxis = ya;
-        valAxis = xa;
-    } else {
-        posAxis = xa;
-        valAxis = ya;
-    }
+        var posAxis, valAxis;
+
+        if(trace.orientation === 'h') {
+            posAxis = ya;
+            valAxis = xa;
+        } else {
+            posAxis = xa;
+            valAxis = ya;
+        }
 
-    // save the box size and box position for use by hover
-    t.bPos = bPos;
-    t.bdPos = bdPos;
-    t.wdPos = wdPos;
-    // half-width within which to accept hover for this box
-    // always split the distance to the closest box
-    t.wHover = t.dPos * (group ? groupFraction / numBoxes : 1);
-
-    plotBoxAndWhiskers(plotGroup, {pos: posAxis, val: valAxis}, trace, t);
-    plotPoints(plotGroup, {x: xa, y: ya}, trace, t);
-    plotBoxMean(plotGroup, {pos: posAxis, val: valAxis}, trace, t);
+        // save the box size and box position for use by hover
+        t.bPos = bPos;
+        t.bdPos = bdPos;
+        t.wdPos = wdPos;
+        // half-width within which to accept hover for this box
+        // always split the distance to the closest box
+        t.wHover = t.dPos * (group ? groupFraction / numBoxes : 1);
+
+        plotBoxAndWhiskers(plotGroup, {pos: posAxis, val: valAxis}, trace, t);
+        plotPoints(plotGroup, {x: xa, y: ya}, trace, t);
+        plotBoxMean(plotGroup, {pos: posAxis, val: valAxis}, trace, t);
+    });
 }
 
 function plotBoxAndWhiskers(sel, axes, trace, t) {