Skip to content

Implement axis.layer with 'above traces' and 'below traces' values #1871

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 7 commits into from
Jul 13, 2017
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
1 change: 0 additions & 1 deletion devtools/test_dashboard/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
4 changes: 2 additions & 2 deletions src/plot_api/subroutines.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
11 changes: 8 additions & 3 deletions src/plots/cartesian/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
};
55 changes: 43 additions & 12 deletions src/plots/cartesian/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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
Expand 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
Expand Down
12 changes: 12 additions & 0 deletions src/plots/cartesian/layout_attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,18 @@ 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.'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Per #1861 (comment) - can you mention scatter.cliponaxis here (and vice versa)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call 📚

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in 6f494c6

].join(' ')
},
domain: {
valType: 'info_array',
role: 'info',
Expand Down
2 changes: 2 additions & 0 deletions src/plots/cartesian/position_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,7 @@ module.exports = function handlePositionDefaults(containerIn, containerOut, coer
Lib.noneOrAll(containerIn.domain, containerOut.domain, [0, 1]);
}

coerce('layer');

return containerOut;
};
25 changes: 18 additions & 7 deletions src/plots/plots.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 10 additions & 4 deletions src/traces/scatter/plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions tasks/cibundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 9 additions & 4 deletions tasks/util/browserify_wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions tasks/util/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/'),
Expand Down
26 changes: 26 additions & 0 deletions tasks/util/strict_d3.js
Original file line number Diff line number Diff line change
@@ -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',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexcjohnson I hope you don't mind this commit too much. This made incorporating strict-d3.js in our jasmine tests a lot easier - which is very useful e.g. to 🔒 down e234827

Companion plotly-mock-viewer PR: rreusser/plotly-mock-viewer#15

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah, looks great!

{ evaluateArguments: true, jsFilesOnly: true },
function(args, opts, cb) {
var pathIn = args[0];
var pathOut;

if(pathIn === 'd3' && opts.file !== pathToStrictD3Module) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I gather this is so that strict-d3 itself can still get the original d3?

Copy link
Contributor Author

@etpinard etpinard Jul 12, 2017

Choose a reason for hiding this comment

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

Bingo. 🎲

pathOut = 'require(\'' + pathToStrictD3Module + '\')';
}

if(pathOut) return cb(null, pathOut);
else return cb();
});
3 changes: 2 additions & 1 deletion tasks/util/watchified_bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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]
Expand Down
Binary file modified test/image/baselines/cliponaxis_false.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 0 additions & 1 deletion test/image/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
<script type="text/javascript" src="../plotly.js/dist/extras/mathjax/MathJax.js?config=TeX-AMS-MML_SVG"></script>
<script type="text/javascript" src="../plotly.js/build/plotly.js" charset="utf-8"></script>
<script type="text/javascript" src="../plotly.js/dist/plotly-geo-assets.js" charset="utf-8"></script>
<script type="text/javascript" src="../plotly.js/test/image/strict-d3.js" charset="utf-8"></script>
<script type="text/javascript" src="./main.js"></script>
</body>
</html>
Loading