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

etpinard · 2017-07-11T21:57:34Z

to be merged in #1861 with (hopefully) all the requirements for #1385

This PR adds an axis attribute layer with two possible values: 'above traces' (the default and current behavior) and 'below traces' which allows together with cliponaxis: false to draw markers and text nodes above axis lines and tick labels. See cliponaxis_false mock in e44aa4d for an example.

We chose to make layer have a hard 'above traces' default value corresponding to the behavior on master. This might be revisited in v2.

This PR also fixes a bug in the cliponaxis: false implementation in 7b62b73 and lints a few things.

In a private convo with @alexcjohnson , we opted to not include special auto-range logic for cliponaxis: false traces. This issue will be dealt with by having a few more axis options in a later PR. I'll open up a ticket about this tomorrow. DONE in #1876

Unless someone ⛔ me, I'll won't add a workaround patch for the cartesian line clip path problems on pan (see #1861 (comment)) nor will I fix the IE / Edge issues from #1861 (comment). In trying to fix these, I broke some animation tests; I think it would be wise to wait until Ricky is back to working full-time before making another attempt. I'll document this things in new separate issues tomorrow. FIXED in e234827

- which is more meaningul - other misc lint

- has 2 possible values: 'above traces' and 'below traces' for now, but opens the door for other values down the road.

- show off `(x|y)axis.layer` - lock down behavior for `cliponaxis: true` traces on hasClipOnAxisFalse subplot - add annotations for more descriptive mock

alexcjohnson · 2017-07-12T13:23:52Z

I won't add a workaround patch for the cartesian line clip path problems on pan (see #1861 (comment)) nor will I fix the IE / Edge issues from #1861 (comment).

That's fine, I'll make an issue for the IE/Edge one since it's not new. For the chrome issue since it IS new, can you investigate a little (perhaps reduce it to a static SVG bug in codepen?) and then either make an internal issue or report it to chromium?

alexcjohnson · 2017-07-12T13:49:06Z

test/jasmine/tests/cartesian_test.js

+        })
+        .catch(failTest)
+        .then(done);
+    });


Beautiful. This test is exactly the question I was about to ask when looking at the code.

alexcjohnson · 2017-07-12T13:55:11Z

src/plots/cartesian/layout_attributes.js

+            '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.'


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

Good call 📚

done in 6f494c6

- which replaces all `require('d3')` with `require('./strict-d3')` - so that strict-d3.js can more easily be used across all our tools - activate when bundling plotly.js in jasmine tests, `npm run watch` and `npm run cibuild` - 🔪 <script> injecting old IIFE - require('fast-isnumeric') in strict-d3.js

- instead of on <clipPath> element itself - see https://codepen.io/etpinard/pen/eRbxrp for more info on the problem - lock down this rule by adding condition in strict-d3.js

etpinard · 2017-07-12T20:51:48Z

tasks/util/strict_d3.js

+ * Transform `require('d3')` expressions to `require(/path/to/strict-d3.js)`
+ */
+
+module.exports = transformTools.makeRequireTransform('requireTransform',


@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

yeah, looks great!

etpinard · 2017-07-12T20:52:58Z

test/image/strict-d3.js

+function checkAttrVal(sel, key) {
+    // setting the transform attribute on a <clipPath> does not
+    // work in Chrome, IE and Edge
+    if(sel.node().nodeName === 'clipPath' && key === 'transform') {


No need for additional jasmine tests cases, this here made many animation and interaction test fail.

etpinard · 2017-07-12T20:53:20Z

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')


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

alexcjohnson · 2017-07-12T21:43:06Z

tasks/util/strict_d3.js

+        var pathIn = args[0];
+        var pathOut;
+
+        if(pathIn === 'd3' && opts.file !== pathToStrictD3Module) {


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

Bingo. 🎲

alexcjohnson · 2017-07-12T22:16:27Z

💃

etpinard added 4 commits July 11, 2017 17:22

handle cliponaxis false/true traces coexisting on same subplot

7b62b73

rename 'layers' cartesian constant -> 'traceLayerClasses'

02b9fbc

- which is more meaningul - other misc lint

add 'layer' axis attribute

1c85de6

- has 2 possible values: 'above traces' and 'below traces' for now, but opens the door for other values down the road.

improve cliponaxis_false mock

e44aa4d

- show off `(x|y)axis.layer` - lock down behavior for `cliponaxis: true` traces on hasClipOnAxisFalse subplot - add annotations for more descriptive mock

etpinard added status: reviewable feature something new labels Jul 11, 2017

etpinard added this to the 1.29.0 milestone Jul 11, 2017

etpinard mentioned this pull request Jul 11, 2017

[take 2] Implement scatter cliponaxis: false #1861

Merged

alexcjohnson mentioned this pull request Jul 12, 2017

clip path translation is wrong in IE & Edge during pan #1873

Closed

alexcjohnson reviewed Jul 12, 2017

View reviewed changes

etpinard added 2 commits July 12, 2017 16:40

fixes #1873 - apply transform on <clipPath> child

e234827

- instead of on <clipPath> element itself - see https://codepen.io/etpinard/pen/eRbxrp for more info on the problem - lock down this rule by adding condition in strict-d3.js

etpinard commented Jul 12, 2017

View reviewed changes

alexcjohnson reviewed Jul 12, 2017

View reviewed changes

etpinard mentioned this pull request Jul 12, 2017

Implement Axis boundaries #1353

Closed

talk about 'cliponaxis' in axis 'layer' description and vice-versa

6f494c6

etpinard merged commit bbb31f4 into cliponaxis-false-take2 Jul 13, 2017

etpinard deleted the axis-layer-above-below branch July 13, 2017 00:58

etpinard mentioned this pull request Aug 15, 2017

Ternary axis layer #1952

Merged

etpinard mentioned this pull request Sep 28, 2017

Add way to customize geo base layer ordering #2042

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

etpinard commented Jul 11, 2017 •

edited

Loading

alexcjohnson commented Jul 12, 2017

alexcjohnson Jul 12, 2017

alexcjohnson Jul 12, 2017

etpinard Jul 12, 2017

etpinard Jul 12, 2017

etpinard Jul 12, 2017

alexcjohnson Jul 12, 2017

etpinard Jul 12, 2017

etpinard Jul 12, 2017

alexcjohnson Jul 12, 2017

etpinard Jul 12, 2017 •

edited

Loading

alexcjohnson commented Jul 12, 2017

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

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

Conversation

etpinard commented Jul 11, 2017 • edited Loading

alexcjohnson commented Jul 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard Jul 12, 2017 • edited Loading

Choose a reason for hiding this comment

alexcjohnson commented Jul 12, 2017

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

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

etpinard commented Jul 11, 2017 •

edited

Loading

etpinard Jul 12, 2017 •

edited

Loading