Merge pull request #1871 from plotly/axis-layer-above-below

etpinard · web-flow · commit bbb31f4269f4 · 2017-07-12T20:57:58.000-04:00
Implement `axis.layer` with 'above traces' and 'below traces' values
diff --git a/devtools/test_dashboard/index.html b/devtools/test_dashboard/index.html
@@ -21,7 +21,6 @@
 
   <script type="text/javascript" src="../../dist/extras/mathjax/MathJax.js?config=TeX-AMS-MML_SVG"></script>
   <script id="source" type="text/javascript" src="../../build/plotly.js"></script>
-  <script type="text/javascript" src="../../test/image/strict-d3.js" charset="utf-8"></script>
   <script type="text/javascript" src="../../build/test_dashboard-bundle.js"></script>
 </body>
 </html>
diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js
@@ -180,8 +180,8 @@ exports.lsInner = function(gd) {
 
         Drawing.setClipUrl(plotinfo.plot, plotClipId);
 
-        for(i = 0; i < cartesianConstants.layers.length; i++) {
-            var layer = cartesianConstants.layers[i];
+        for(i = 0; i < cartesianConstants.traceLayerClasses.length; i++) {
+            var layer = cartesianConstants.traceLayerClasses[i];
             if(layer !== 'scatterlayer') {
                 plotinfo.plot.selectAll('g.' + layer).call(Drawing.setClipUrl, layerClipId);
             }
diff --git a/src/plots/cartesian/constants.js b/src/plots/cartesian/constants.js
@@ -51,19 +51,24 @@ module.exports = {
     DFLTRANGEX: [-1, 6],
     DFLTRANGEY: [-1, 4],
 
-    // Layers to keep plot types in the right order.
+    // Layers to keep trace 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
-    layers: [
+    traceLayerClasses: [
         'imagelayer',
         'maplayer',
         'barlayer',
         'carpetlayer',
         'boxlayer',
         'scatterlayer'
-    ]
+    ],
+
+    layerValue2layerClass: {
+        'above traces': 'above',
+        'below traces': 'below'
+    }
 };
diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js
@@ -743,7 +743,7 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) {
             var plotDx = xa2._offset - clipDx / xScaleFactor2,
                 plotDy = ya2._offset - clipDy / yScaleFactor2;
 
-            fullLayout._defs.selectAll('#' + subplot.clipId)
+            fullLayout._defs.select('#' + subplot.clipId + '> rect')
                 .call(Drawing.setTranslate, clipDx, clipDy)
                 .call(Drawing.setScale, xScaleFactor2, yScaleFactor2);
 
diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js
@@ -296,6 +296,8 @@ function makeSubplotData(gd) {
 function makeSubplotLayer(plotinfo) {
     var plotgroup = plotinfo.plotgroup;
     var id = plotinfo.id;
+    var xLayer = constants.layerValue2layerClass[plotinfo.xaxis.layer];
+    var yLayer = constants.layerValue2layerClass[plotinfo.yaxis.layer];
 
     if(!plotinfo.mainplot) {
         var backLayer = joinLayer(plotgroup, 'g', 'layer-subplot');
@@ -308,19 +310,36 @@ function makeSubplotLayer(plotinfo) {
         plotinfo.zerolinelayer = joinLayer(plotgroup, 'g', 'zerolinelayer');
         plotinfo.overzero = joinLayer(plotgroup, 'g', 'overzero');
 
+        joinLayer(plotgroup, 'path', 'xlines-below');
+        joinLayer(plotgroup, 'path', 'ylines-below');
+        plotinfo.overlinesBelow = joinLayer(plotgroup, 'g', 'overlines-below');
+
+        joinLayer(plotgroup, 'g', 'xaxislayer-below');
+        joinLayer(plotgroup, 'g', 'yaxislayer-below');
+        plotinfo.overaxesBelow = joinLayer(plotgroup, 'g', 'overaxes-below');
+
         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');
+        joinLayer(plotgroup, 'path', 'xlines-above');
+        joinLayer(plotgroup, 'path', 'ylines-above');
+        plotinfo.overlinesAbove = joinLayer(plotgroup, 'g', 'overlines-above');
+
+        joinLayer(plotgroup, 'g', 'xaxislayer-above');
+        joinLayer(plotgroup, 'g', 'yaxislayer-above');
+        plotinfo.overaxesAbove = joinLayer(plotgroup, 'g', 'overaxes-above');
 
-        plotinfo.xaxislayer = joinLayer(plotgroup, 'g', 'xaxislayer');
-        plotinfo.yaxislayer = joinLayer(plotgroup, 'g', 'yaxislayer');
-        plotinfo.overaxes = joinLayer(plotgroup, 'g', 'overaxes');
+        // set refs to correct layers as determined by 'axis.layer'
+        plotinfo.xlines = plotgroup.select('.xlines-' + xLayer);
+        plotinfo.ylines = plotgroup.select('.ylines-' + yLayer);
+        plotinfo.xaxislayer = plotgroup.select('.xaxislayer-' + xLayer);
+        plotinfo.yaxislayer = plotgroup.select('.yaxislayer-' + yLayer);
     }
     else {
         var mainplotinfo = plotinfo.mainplotinfo;
+        var mainplotgroup = mainplotinfo.plotgroup;
+        var xId = id + '-x';
+        var yId = id + '-y';
 
         // now make the components of overlaid subplots
         // overlays don't have backgrounds, and append all
@@ -330,17 +349,29 @@ function makeSubplotLayer(plotinfo) {
         plotinfo.gridlayer = joinLayer(mainplotinfo.overgrid, 'g', id);
         plotinfo.zerolinelayer = joinLayer(mainplotinfo.overzero, 'g', id);
 
+        joinLayer(mainplotinfo.overlinesBelow, 'path', xId);
+        joinLayer(mainplotinfo.overlinesBelow, 'path', yId);
+        joinLayer(mainplotinfo.overaxesBelow, 'g', xId);
+        joinLayer(mainplotinfo.overaxesBelow, 'g', yId);
+
         plotinfo.plot = joinLayer(mainplotinfo.overplot, 'g', id);
-        plotinfo.xlines = joinLayer(mainplotinfo.overlines, 'path', id + '-x');
-        plotinfo.ylines = joinLayer(mainplotinfo.overlines, 'path', id + '-y');
-        plotinfo.xaxislayer = joinLayer(mainplotinfo.overaxes, 'g', id + '-x');
-        plotinfo.yaxislayer = joinLayer(mainplotinfo.overaxes, 'g', id + '-y');
+
+        joinLayer(mainplotinfo.overlinesAbove, 'path', xId);
+        joinLayer(mainplotinfo.overlinesAbove, 'path', yId);
+        joinLayer(mainplotinfo.overaxesAbove, 'g', xId);
+        joinLayer(mainplotinfo.overaxesAbove, 'g', yId);
+
+        // set refs to correct layers as determined by 'abovetraces'
+        plotinfo.xlines = mainplotgroup.select('.overlines-' + xLayer).select('.' + xId);
+        plotinfo.ylines = mainplotgroup.select('.overlines-' + yLayer).select('.' + yId);
+        plotinfo.xaxislayer = mainplotgroup.select('.overaxes-' + xLayer).select('.' + xId);
+        plotinfo.yaxislayer = mainplotgroup.select('.overaxes-' + yLayer).select('.' + yId);
     }
 
     // common attributes for all subplots, overlays or not
 
-    for(var i = 0; i < constants.layers.length; i++) {
-        joinLayer(plotinfo.plot, 'g', constants.layers[i]);
+    for(var i = 0; i < constants.traceLayerClasses.length; i++) {
+        joinLayer(plotinfo.plot, 'g', constants.traceLayerClasses[i]);
     }
 
     plotinfo.xlines
diff --git a/src/plots/cartesian/layout_attributes.js b/src/plots/cartesian/layout_attributes.js
@@ -573,6 +573,20 @@ module.exports = {
             'If *false*, this axis does not overlay any same-letter axes.'
         ].join(' ')
     },
+    layer: {
+        valType: 'enumerated',
+        values: ['above traces', 'below traces'],
+        dflt: 'above traces',
+        role: 'info',
+        description: [
+            'Sets the layer on which this axis is displayed.',
+            'If *above traces*, this axis is displayed above all the subplot\'s traces',
+            'If *below traces*, this axis is displayed below all the subplot\'s traces,',
+            'but above the grid lines.',
+            'Useful when used together with scatter-like traces with `cliponaxis`',
+            'set to *false* to show markers and/or text nodes above this axis.'
+        ].join(' ')
+    },
     domain: {
         valType: 'info_array',
         role: 'info',
diff --git a/src/plots/cartesian/position_defaults.js b/src/plots/cartesian/position_defaults.js
@@ -59,5 +59,7 @@ module.exports = function handlePositionDefaults(containerIn, containerOut, coer
         Lib.noneOrAll(containerIn.domain, containerOut.domain, [0, 1]);
     }
 
+    coerce('layer');
+
     return containerOut;
 };
diff --git a/src/plots/cartesian/transition_axes.js b/src/plots/cartesian/transition_axes.js
@@ -137,7 +137,7 @@ module.exports = function transitionAxes(gd, newLayout, transitionOpts, makeOnCo
         var xa2 = subplot.xaxis;
         var ya2 = subplot.yaxis;
 
-        fullLayout._defs.selectAll('#' + subplot.clipId)
+        fullLayout._defs.select('#' + subplot.clipId + '> rect')
             .call(Drawing.setTranslate, 0, 0)
             .call(Drawing.setScale, 1, 1);
 
@@ -221,7 +221,7 @@ module.exports = function transitionAxes(gd, newLayout, transitionOpts, makeOnCo
         var plotDx = xa2._offset - fracDx,
             plotDy = ya2._offset - fracDy;
 
-        fullLayout._defs.selectAll('#' + subplot.clipId)
+        fullLayout._defs.select('#' + subplot.clipId + '> rect')
             .call(Drawing.setTranslate, clipDx, clipDy)
             .call(Drawing.setScale, 1 / xScaleFactor, 1 / yScaleFactor);
 
diff --git a/src/plots/plots.js b/src/plots/plots.js
@@ -638,24 +638,35 @@ plots.linkSubplots = function(newFullData, newFullLayout, oldFullData, oldFullLa
     var ids = Plotly.Axes.getSubplots(mockGd);
 
     for(var i = 0; i < ids.length; i++) {
-        var id = ids[i],
-            oldSubplot = oldSubplots[id],
-            plotinfo;
+        var id = ids[i];
+        var oldSubplot = oldSubplots[id];
+        var xaxis = Plotly.Axes.getFromId(mockGd, id, 'x');
+        var yaxis = Plotly.Axes.getFromId(mockGd, id, 'y');
+        var plotinfo;
 
         if(oldSubplot) {
             plotinfo = newSubplots[id] = oldSubplot;
 
             if(plotinfo._scene2d) {
                 plotinfo._scene2d.updateRefs(newFullLayout);
             }
-        }
-        else {
+
+            if(plotinfo.xaxis.layer !== xaxis.layer) {
+                plotinfo.xlines.attr('d', null);
+                plotinfo.xaxislayer.selectAll('*').remove();
+            }
+
+            if(plotinfo.yaxis.layer !== yaxis.layer) {
+                plotinfo.ylines.attr('d', null);
+                plotinfo.yaxislayer.selectAll('*').remove();
+            }
+        } else {
             plotinfo = newSubplots[id] = {};
             plotinfo.id = id;
         }
 
-        plotinfo.xaxis = Plotly.Axes.getFromId(mockGd, id, 'x');
-        plotinfo.yaxis = Plotly.Axes.getFromId(mockGd, id, 'y');
+        plotinfo.xaxis = xaxis;
+        plotinfo.yaxis = yaxis;
 
         // By default, we clip at the subplot level,
         // but if one trace on a given subplot has *cliponaxis* set to false,
diff --git a/src/plots/ternary/ternary.js b/src/plots/ternary/ternary.js
@@ -292,7 +292,7 @@ proto.adjustLayout = function(ternaryLayout, graphSize) {
     _this.plotContainer.selectAll('.scatterlayer,.maplayer')
         .attr('transform', plotTransform);
 
-    _this.clipDefRelative.attr('transform', null);
+    _this.clipDefRelative.select('path').attr('transform', null);
 
     // TODO: shift axes to accommodate linewidth*sin(30) tick mark angle
 
@@ -619,7 +619,7 @@ proto.initInteractions = function() {
             .attr('transform', plotTransform);
 
         var plotTransform2 = 'translate(' + -dx + ',' + -dy + ')';
-        _this.clipDefRelative.attr('transform', plotTransform2);
+        _this.clipDefRelative.select('path').attr('transform', plotTransform2);
 
         // move the ticks
         _this.aaxis.range = [mins.a, _this.sum - mins.b - mins.c];
diff --git a/src/traces/scatter/attributes.js b/src/traces/scatter/attributes.js
@@ -186,7 +186,9 @@ module.exports = {
         editType: 'doplot',
         description: [
             'Determines whether or not markers and text nodes',
-            'are clipped about the subplot axes.'
+            'are clipped about the subplot axes.',
+            'To show markers and text nodes above axis lines and tick labels,',
+            'make sure to set `xaxis.layer` and `yaxis.layer` to *below traces*.'
         ].join(' ')
     },
 
diff --git a/src/traces/scatter/plot.js b/src/traces/scatter/plot.js
@@ -404,8 +404,7 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
         var trace = d[0].trace,
             s = d3.select(this),
             showMarkers = subTypes.hasMarkers(trace),
-            showText = subTypes.hasText(trace),
-            hasClipOnAxisFalse = trace.cliponaxis === false;
+            showText = subTypes.hasText(trace);
 
         var keyFunc = getKeyFunc(trace),
             markerFilter = hideFilter,
@@ -450,7 +449,7 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
             if(hasNode) {
                 Drawing.singlePointStyle(d, sel, trace, markerScale, lineScale, gd);
 
-                if(hasClipOnAxisFalse) {
+                if(plotinfo.layerClipId) {
                     Drawing.hideOutsideRangePoint(d, sel, xa, ya);
                 }
 
@@ -486,7 +485,7 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
             hasNode = Drawing.translatePoint(d, sel, xa, ya);
 
             if(hasNode) {
-                if(hasClipOnAxisFalse) {
+                if(plotinfo.layerClipId) {
                     Drawing.hideOutsideRangePoint(d, g, xa, ya);
                 }
             } else {
@@ -525,6 +524,13 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
         .each(makePoints);
 
     join.exit().remove();
+
+    // lastly, clip points groups of `cliponaxis !== false` traces
+    // on `plotinfo._hasClipOnAxisFalse === true` subplots
+    join.each(function(d) {
+        var hasClipOnAxisFalse = d[0].trace.cliponaxis === false;
+        Drawing.setClipUrl(d3.select(this), hasClipOnAxisFalse ? null : plotinfo.layerClipId);
+    });
 }
 
 function selectMarkers(gd, idx, plotinfo, cdscatter, cdscatterAll) {
diff --git a/tasks/cibundle.js b/tasks/cibundle.js
@@ -16,6 +16,7 @@ var _bundle = require('./util/browserify_wrapper');
 _bundle(constants.pathToPlotlyIndex, constants.pathToPlotlyBuild, {
     standalone: 'Plotly',
     pathToMinBundle: constants.pathToPlotlyDistMin,
+    debug: true
 });
 
 // Browserify the geo assets
diff --git a/tasks/util/browserify_wrapper.js b/tasks/util/browserify_wrapper.js
@@ -7,6 +7,7 @@ var UglifyJS = require('uglify-js');
 var constants = require('./constants');
 var compressAttributes = require('./compress_attributes');
 var patchMinified = require('./patch_minified');
+var strictD3 = require('./strict_d3');
 
 /** Convenience browserify wrapper
  *
@@ -32,16 +33,20 @@ module.exports = function _bundle(pathToIndex, pathToBundle, opts) {
     opts = opts || {};
 
     // do we output a minified file?
-    var pathToMinBundle = opts.pathToMinBundle,
-        outputMinified = !!pathToMinBundle && !opts.debug;
+    var pathToMinBundle = opts.pathToMinBundle;
+    var outputMinified = !!pathToMinBundle;
 
     var browserifyOpts = {};
     browserifyOpts.standalone = opts.standalone;
     browserifyOpts.debug = opts.debug;
     browserifyOpts.transform = outputMinified ? [compressAttributes] : [];
 
-    var b = browserify(pathToIndex, browserifyOpts),
-        bundleWriteStream = fs.createWriteStream(pathToBundle);
+    if(opts.debug) {
+        browserifyOpts.transform.push(strictD3);
+    }
+
+    var b = browserify(pathToIndex, browserifyOpts);
+    var bundleWriteStream = fs.createWriteStream(pathToBundle);
 
     bundleWriteStream.on('finish', function() {
         logger(pathToBundle);
diff --git a/tasks/util/constants.js b/tasks/util/constants.js
@@ -58,6 +58,7 @@ module.exports = {
     pathToTestDashboardBundle: path.join(pathToBuild, 'test_dashboard-bundle.js'),
     pathToImageViewerBundle: path.join(pathToBuild, 'image_viewer-bundle.js'),
 
+    pathToImageTest: pathToImageTest,
     pathToTestImageMocks: path.join(pathToImageTest, 'mocks/'),
     pathToTestImageBaselines: path.join(pathToImageTest, 'baselines/'),
     pathToTestImages: path.join(pathToBuild, 'test_images/'),
diff --git a/tasks/util/strict_d3.js b/tasks/util/strict_d3.js
@@ -0,0 +1,26 @@
+var path = require('path');
+var transformTools = require('browserify-transform-tools');
+var constants = require('./constants');
+
+var pathToStrictD3Module = path.join(
+    constants.pathToImageTest,
+    'strict-d3.js'
+);
+
+/**
+ * Transform `require('d3')` expressions to `require(/path/to/strict-d3.js)`
+ */
+
+module.exports = transformTools.makeRequireTransform('requireTransform',
+    { evaluateArguments: true, jsFilesOnly: true },
+    function(args, opts, cb) {
+        var pathIn = args[0];
+        var pathOut;
+
+        if(pathIn === 'd3' && opts.file !== pathToStrictD3Module) {
+            pathOut = 'require(\'' + pathToStrictD3Module + '\')';
+        }
+
+        if(pathOut) return cb(null, pathOut);
+        else return cb();
+    });
diff --git a/tasks/util/watchified_bundle.js b/tasks/util/watchified_bundle.js
@@ -7,6 +7,7 @@ var prettySize = require('prettysize');
 var constants = require('./constants');
 var common = require('./common');
 var compressAttributes = require('./compress_attributes');
+var strictD3 = require('./strict_d3');
 
 /**
  * Make a plotly.js browserify bundle function watched by watchify.
@@ -22,7 +23,7 @@ module.exports = function makeWatchifiedBundle(onFirstBundleCallback) {
     var b = browserify(constants.pathToPlotlyIndex, {
         debug: true,
         standalone: 'Plotly',
-        transform: [compressAttributes],
+        transform: [strictD3, compressAttributes],
         cache: {},
         packageCache: {},
         plugin: [watchify]
diff --git a/test/image/baselines/cliponaxis_false.png b/test/image/baselines/cliponaxis_false.png
diff --git a/test/image/index.html b/test/image/index.html
diff --git a/test/image/mocks/cliponaxis_false.json b/test/image/mocks/cliponaxis_false.json
diff --git a/test/image/strict-d3.js b/test/image/strict-d3.js
diff --git a/test/jasmine/karma.conf.js b/test/jasmine/karma.conf.js
diff --git a/test/jasmine/tests/cartesian_test.js b/test/jasmine/tests/cartesian_test.js
diff --git a/test/jasmine/tests/plots_test.js b/test/jasmine/tests/plots_test.js
diff --git a/test/jasmine/tests/scatter_test.js b/test/jasmine/tests/scatter_test.js

Original file line number	Diff line number	Diff line change
`@@ -180,8 +180,8 @@ exports.lsInner = function(gd) {`
`180`	`180`
`181`	`181`	`Drawing.setClipUrl(plotinfo.plot, plotClipId);`
`182`	`182`
`183`		`- for(i = 0; i < cartesianConstants.layers.length; i++) {`
`184`		`- var layer = cartesianConstants.layers[i];`
	`183`	`+ for(i = 0; i < cartesianConstants.traceLayerClasses.length; i++) {`
	`184`	`+ var layer = cartesianConstants.traceLayerClasses[i];`
`185`	`185`	`if(layer !== 'scatterlayer') {`
`186`	`186`	`plotinfo.plot.selectAll('g.' + layer).call(Drawing.setClipUrl, layerClipId);`
`187`	`187`	`}`
Original file line number	Diff line number	Diff line change
`@@ -59,5 +59,7 @@ module.exports = function handlePositionDefaults(containerIn, containerOut, coer`
`59`	`59`	`Lib.noneOrAll(containerIn.domain, containerOut.domain, [0, 1]);`
`60`	`60`	`}`
`61`	`61`
	`62`	`+ coerce('layer');`
	`63`	`+`
`62`	`64`	`return containerOut;`
`63`	`65`	`};`