Merge pull request #2917 from plotly/heatmap-ordering-bug

Heatmap ordering bug

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/lib/make_trace_groups.js

@@ -0,0 +1,35 @@
+/**
+* Copyright 2012-2018, Plotly, Inc.
+* All rights reserved.
+*
+* This source code is licensed under the MIT license found in the
+* LICENSE file in the root directory of this source tree.
+*/
+
+
+'use strict';
+
+/**
+ * General helper to manage trace groups based on calcdata
+ *
+ * @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
+ */
+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; });
+
+    traces.exit().remove();
+
+    traces.enter().append('g')
+        .attr('class', cls);
+
+    traces.order();
+
+    return traces;
+};

src/plots/get_data.js

@@ -124,20 +124,3 @@ exports.getSubplotData = function getSubplotData(data, type, subplotId) {
 
     return subplotData;
 };
-
-/**
- * Get a lookup object of trace uids corresponding in a given calcdata array.
- *
- * @param {array} calcdata: as in gd.calcdata (or a subset)
- * @return {object} lookup object of uids (`uid: 1`)
- */
-exports.getUidsFromCalcData = function(calcdata) {
-    var out = {};
-
-    for(var i = 0; i < calcdata.length; i++) {
-        var trace = calcdata[i][0].trace;
-        out[trace.uid] = 1;
-    }
-
-    return out;
-};

src/traces/bar/plot.js

@@ -35,30 +35,20 @@ module.exports = function plot(gd, plotinfo, cdbar, barLayer) {
     var ya = plotinfo.yaxis;
     var fullLayout = gd._fullLayout;
 
-    var bartraces = barLayer.selectAll('g.trace.bars')
-        .data(cdbar, function(d) { return d[0].trace.uid; });
-
-    bartraces.enter().append('g')
-        .attr('class', 'trace bars')
-        .append('g')
-        .attr('class', 'points');
-
-    bartraces.exit().remove();
-
-    bartraces.order();
-
-    bartraces.each(function(d) {
-        var cd0 = d[0];
+    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 sel = d3.select(this);
 
-        if(!plotinfo.isRangePlot) cd0.node3 = sel;
+        if(!plotinfo.isRangePlot) cd0.node3 = plotGroup;
 
         var poffset = t.poffset;
         var poffsetIsArray = Array.isArray(poffset);
 
-        var bars = sel.select('g.points').selectAll('g.point').data(Lib.identity);
+        var pointGroup = Lib.ensureSingle(plotGroup, 'g', 'points');
+
+        var bars = pointGroup.selectAll('g.point').data(Lib.identity);
 
         bars.enter().append('g')
             .classed('point', true);
@@ -147,17 +137,17 @@ module.exports = function plot(gd, plotinfo, cdbar, barLayer) {
                     'M' + x0 + ',' + y0 + 'V' + y1 + 'H' + x1 + 'V' + y0 + 'Z')
                 .call(Drawing.setClipUrl, plotinfo.layerClipId);
 
-            appendBarText(gd, bar, d, i, x0, x1, y0, y1);
+            appendBarText(gd, bar, cd, i, x0, x1, y0, y1);
 
             if(plotinfo.layerClipId) {
-                Drawing.hideOutsideRangePoint(d[i], bar.select('text'), xa, ya, trace.xcalendar, trace.ycalendar);
+                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 = d[0].trace.cliponaxis === false;
-        Drawing.setClipUrl(sel, hasClipOnAxisFalse ? null : plotinfo.layerClipId);
+        var hasClipOnAxisFalse = cd0.trace.cliponaxis === false;
+        Drawing.setClipUrl(plotGroup, hasClipOnAxisFalse ? null : plotinfo.layerClipId);
     });
 
     // error bars are on the top

src/traces/box/plot.js

@@ -21,28 +21,16 @@ function plot(gd, plotinfo, cdbox, boxLayer) {
     var fullLayout = gd._fullLayout;
     var xa = plotinfo.xaxis;
     var ya = plotinfo.yaxis;
+    var numBoxes = fullLayout._numBoxes;
+    var groupFraction = (1 - fullLayout.boxgap);
+    var group = (fullLayout.boxmode === 'group' && numBoxes > 1);
 
-    var boxtraces = boxLayer.selectAll('g.trace.boxes')
-        .data(cdbox, function(d) { return d[0].trace.uid; });
-
-    boxtraces.enter().append('g')
-        .attr('class', 'trace boxes');
-
-    boxtraces.exit().remove();
-
-    boxtraces.order();
-
-    boxtraces.each(function(d) {
-        var cd0 = d[0];
+    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;
-        var sel = d3.select(this);
-        if(!plotinfo.isRangePlot) cd0.node3 = sel;
-        var numBoxes = fullLayout._numBoxes;
-
-        var groupFraction = (1 - fullLayout.boxgap);
-
-        var group = (fullLayout.boxmode === 'group' && numBoxes > 1);
+        if(!plotinfo.isRangePlot) cd0.node3 = plotGroup;
         // box half width
         var bdPos = t.dPos * groupFraction * (1 - fullLayout.boxgroupgap) / (group ? numBoxes : 1);
         // box center offset
@@ -51,7 +39,7 @@ function plot(gd, plotinfo, cdbox, boxLayer) {
         var wdPos = bdPos * trace.whiskerwidth;
 
         if(trace.visible !== true || t.empty) {
-            sel.remove();
+            plotGroup.remove();
             return;
         }
 
@@ -73,9 +61,9 @@ function plot(gd, plotinfo, cdbox, boxLayer) {
         // always split the distance to the closest box
         t.wHover = t.dPos * (group ? groupFraction / numBoxes : 1);
 
-        plotBoxAndWhiskers(sel, {pos: posAxis, val: valAxis}, trace, t);
-        plotPoints(sel, {x: xa, y: ya}, trace, t);
-        plotBoxMean(sel, {pos: posAxis, val: valAxis}, trace, t);
+        plotBoxAndWhiskers(plotGroup, {pos: posAxis, val: valAxis}, trace, t);
+        plotPoints(plotGroup, {x: xa, y: ya}, trace, t);
+        plotBoxMean(plotGroup, {pos: posAxis, val: valAxis}, trace, t);
     });
 }
 

src/traces/carpet/plot.js

@@ -17,61 +17,45 @@ var orientText = require('./orient_text');
 var svgTextUtils = require('../../lib/svg_text_utils');
 var Lib = require('../../lib');
 var alignmentConstants = require('../../constants/alignment');
-var getUidsFromCalcData = require('../../plots/get_data').getUidsFromCalcData;
 
 module.exports = function plot(gd, plotinfo, cdcarpet, carpetLayer) {
-    var uidLookup = getUidsFromCalcData(cdcarpet);
-
-    carpetLayer.selectAll('g.trace').each(function() {
-        var classString = d3.select(this).attr('class');
-        var oldUid = classString.split('carpet')[1].split(/\s/)[0];
-
-        if(!uidLookup[oldUid]) {
-            d3.select(this).remove();
-        }
-    });
-
-    for(var i = 0; i < cdcarpet.length; i++) {
-        plotOne(gd, plotinfo, cdcarpet[i], carpetLayer);
-    }
-};
-
-function plotOne(gd, plotinfo, cd, carpetLayer) {
-    var t = cd[0];
-    var trace = cd[0].trace,
-        xa = plotinfo.xaxis,
-        ya = plotinfo.yaxis,
-        aax = trace.aaxis,
-        bax = trace.baxis,
-        fullLayout = gd._fullLayout;
-
+    var xa = plotinfo.xaxis;
+    var ya = plotinfo.yaxis;
+    var fullLayout = gd._fullLayout;
     var clipLayer = fullLayout._clips;
 
-    var axisLayer = Lib.ensureSingle(carpetLayer, 'g', 'carpet' + trace.uid).classed('trace', true);
-    var minorLayer = Lib.ensureSingle(axisLayer, 'g', 'minorlayer');
-    var majorLayer = Lib.ensureSingle(axisLayer, 'g', 'majorlayer');
-    var boundaryLayer = Lib.ensureSingle(axisLayer, 'g', 'boundarylayer');
-    var labelLayer = Lib.ensureSingle(axisLayer, 'g', 'labellayer');
+    Lib.makeTraceGroups(carpetLayer, cdcarpet, 'trace').each(function(cd) {
+        var axisLayer = d3.select(this);
+        var cd0 = cd[0];
+        var trace = cd0.trace;
+        var aax = trace.aaxis;
+        var bax = trace.baxis;
 
-    axisLayer.style('opacity', trace.opacity);
+        var minorLayer = Lib.ensureSingle(axisLayer, 'g', 'minorlayer');
+        var majorLayer = Lib.ensureSingle(axisLayer, 'g', 'majorlayer');
+        var boundaryLayer = Lib.ensureSingle(axisLayer, 'g', 'boundarylayer');
+        var labelLayer = Lib.ensureSingle(axisLayer, 'g', 'labellayer');
 
-    drawGridLines(xa, ya, majorLayer, aax, 'a', aax._gridlines, true);
-    drawGridLines(xa, ya, majorLayer, bax, 'b', bax._gridlines, true);
-    drawGridLines(xa, ya, minorLayer, aax, 'a', aax._minorgridlines, true);
-    drawGridLines(xa, ya, minorLayer, bax, 'b', bax._minorgridlines, true);
+        axisLayer.style('opacity', trace.opacity);
 
-    // NB: These are not ommitted if the lines are not active. The joins must be executed
-    // in order for them to get cleaned up without a full redraw
-    drawGridLines(xa, ya, boundaryLayer, aax, 'a-boundary', aax._boundarylines);
-    drawGridLines(xa, ya, boundaryLayer, bax, 'b-boundary', bax._boundarylines);
+        drawGridLines(xa, ya, majorLayer, aax, 'a', aax._gridlines, true);
+        drawGridLines(xa, ya, majorLayer, bax, 'b', bax._gridlines, true);
+        drawGridLines(xa, ya, minorLayer, aax, 'a', aax._minorgridlines, true);
+        drawGridLines(xa, ya, minorLayer, bax, 'b', bax._minorgridlines, true);
 
-    var labelOrientationA = drawAxisLabels(gd, xa, ya, trace, t, labelLayer, aax._labels, 'a-label');
-    var labelOrientationB = drawAxisLabels(gd, xa, ya, trace, t, labelLayer, bax._labels, 'b-label');
+        // NB: These are not ommitted if the lines are not active. The joins must be executed
+        // in order for them to get cleaned up without a full redraw
+        drawGridLines(xa, ya, boundaryLayer, aax, 'a-boundary', aax._boundarylines);
+        drawGridLines(xa, ya, boundaryLayer, bax, 'b-boundary', bax._boundarylines);
 
-    drawAxisTitles(gd, labelLayer, trace, t, xa, ya, labelOrientationA, labelOrientationB);
+        var labelOrientationA = drawAxisLabels(gd, xa, ya, trace, cd0, labelLayer, aax._labels, 'a-label');
+        var labelOrientationB = drawAxisLabels(gd, xa, ya, trace, cd0, labelLayer, bax._labels, 'b-label');
 
-    drawClipPath(trace, t, clipLayer, xa, ya);
-}
+        drawAxisTitles(gd, labelLayer, trace, cd0, xa, ya, labelOrientationA, labelOrientationB);
+
+        drawClipPath(trace, cd0, clipLayer, xa, ya);
+    });
+};
 
 function drawClipPath(trace, t, layer, xaxis, yaxis) {
     var seg, xp, yp, i;

src/traces/choropleth/plot.js

@@ -22,18 +22,8 @@ module.exports = function plot(gd, geo, calcData) {
         calcGeoJSON(calcData[i], geo.topojson);
     }
 
-    function keyFunc(d) { return d[0].trace.uid; }
-
-    var gTraces = geo.layers.backplot.select('.choroplethlayer')
-        .selectAll('g.trace.choropleth')
-        .data(calcData, keyFunc);
-
-    gTraces.enter().append('g')
-        .attr('class', 'trace choropleth');
-
-    gTraces.exit().remove();
-
-    gTraces.each(function(calcTrace) {
+    var choroplethLayer = geo.layers.backplot.select('.choroplethlayer');
+    Lib.makeTraceGroups(choroplethLayer, calcData, 'trace choropleth').each(function(calcTrace) {
         var sel = calcTrace[0].node3 = d3.select(this);
 
         var paths = sel.selectAll('path.choroplethlocation')