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.

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
etpinard merged 7 commits into from
Jul 13, 2017
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7b62b73
handle cliponaxis false/true traces coexisting on same subplot
etpinard Jul 11, 2017
02b9fbc
rename 'layers' cartesian constant -> 'traceLayerClasses'
etpinard Jul 11, 2017
1c85de6
add 'layer' axis attribute
etpinard Jul 11, 2017
e44aa4d
improve cliponaxis_false mock
etpinard Jul 11, 2017
e84701f
replace strict-d3 IIFE with browserify 'require' transform
etpinard Jul 12, 2017
e234827
fixes #1873 - apply transform on <clipPath> child
etpinard Jul 12, 2017
6f494c6
talk about 'cliponaxis' in axis 'layer' description and vice-versa
etpinard Jul 12, 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
1 change: 0 additions & 1 deletion devtools/test_dashboard/index.html
View file
Open in desktop
Original file line numberDiff line numberDiff 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
View file
Open in desktop
Original file line numberDiff line numberDiff 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
View file
Open in desktop
Original file line numberDiff line numberDiff 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'
}
};
2 changes: 1 addition & 1 deletion src/plots/cartesian/dragbox.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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')

@etpinardetpinardJul 12, 2017

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

See https://codepen.io/etpinard/pen/eRbxrp?editors=1010 for more info on this topic.

.call(Drawing.setTranslate, clipDx, clipDy)
.call(Drawing.setScale, xScaleFactor2, yScaleFactor2);

Expand Down
55 changes: 43 additions & 12 deletions src/plots/cartesian/index.js
View file
Open in desktop
Original file line numberDiff line numberDiff 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
14 changes: 14 additions & 0 deletions src/plots/cartesian/layout_attributes.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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',
Expand Down
2 changes: 2 additions & 0 deletions src/plots/cartesian/position_defaults.js
View file
Open in desktop
Original file line numberDiff line numberDiff 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;
};
4 changes: 2 additions & 2 deletions src/plots/cartesian/transition_axes.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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);

Expand DownExpand Up@@ -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);

Expand Down
25 changes: 18 additions & 7 deletions src/plots/plots.js
View file
Open in desktop
Original file line numberDiff line numberDiff 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
4 changes: 2 additions & 2 deletions src/plots/ternary/ternary.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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

Expand DownExpand Up@@ -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];
Expand Down
4 changes: 3 additions & 1 deletion src/traces/scatter/attributes.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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(' ')
},

Expand Down
14 changes: 10 additions & 4 deletions src/traces/scatter/plot.js
View file
Open in desktop
Original file line numberDiff line numberDiff 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 DownExpand 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 DownExpand 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 DownExpand 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
View file
Open in desktop
Original file line numberDiff line numberDiff 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
View file
Open in desktop
Original file line numberDiff line numberDiff 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
View file
Open in desktop
Original file line numberDiff line numberDiff 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
View file
Open in desktop
Original file line numberDiff line numberDiff 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',

@etpinardetpinardJul 12, 2017

Copy link
Copy Markdown
ContributorAuthor

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

@alexcjohnsonalexcjohnsonJul 12, 2017

Copy link
Copy Markdown
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) {

@alexcjohnsonalexcjohnsonJul 12, 2017

Copy link
Copy Markdown
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?

@etpinardetpinardJul 12, 2017•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

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
View file
Open in desktop
Original file line numberDiff line numberDiff 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 modifiedtest/image/baselines/cliponaxis_false.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading