Add axis domain references to shapes, annotations, and layout images #5014
Conversation
This allows shapes and eventually annotations to refer to the length of an axis, so if this length is changed, the shape changes accordingly. For example for a shape, setting xref: "x2 domain", x0: 0, x1: 1 makes a shape that spans exactly the width of xaxis2. When this axis changes length (via the setting of its 'domain' attribute), then the shape will span the new axis.
however there's a funny case when running ./test/image/mocks/sunburst_with-without_values.json where xAx (local function) gets passed an array in ./src/components/shapes/defaults.js (it expects a string or undefined). So we still need to solve this.
This is so that shapes, annotations and images can use the same functions. Still need to add the functionality for annotations and images.
Need to test more thoroughly though.
However, when setting [xy]ref to "paper" and a sizex or sizey of 1, it doesn't seem to fill the plotting area, is this a regression?
It is not working yet...
This is consistent with what 'paper' axis references does, which I think is expected behaviour.
Still funny problem with log axes and shapes though.
before, set_convert was clipping coordinates to 0 if they were negative, so we changed it so domain referenced shapes behave like paper referenced shapes.
The curent paper axis reference has y = 0 at the bottom and y = 1 at the top, so we also make this the case for domain axis references.
They are getting written in test/domain_ref_tests.html for now so they can be debugged quickly. We can rebase this file out once moved to the jasmine tests.
nicholas-esterer
requested review from
alexcjohnson,
archmoj and
nicolaskruchten
| // xaxistype can be linear|log, only used if xref has type 'range' or 'domain', | ||
| // same for yaxistype and yref | ||
| function annotationTest(gd, layout, x0, y0, ax, ay, xref, yref, axref, ayref, | ||
| xaxistype, yaxistype, xid, yid) { |
There was a problem hiding this comment.
Non-blocking:
When having several input arguments to a function like this
function annotationTest(gd, layout, x0, y0, ax, ay, xref, yref, axref, ayref,
xaxistype, yaxistype, xid, yid) {There is a chance to use wrong order in the caller(s).
It is possible to refactor it to use an object for last arguments.
Something like:
function annotationTest(gd, layout, opts) {
var x0 = opts.x0;
...Then change the caller to be
annotationTest(gd, layout, {
x0: x0,
y0: y0,
ax: ax,
ay: ay,
xref: xref,
yref: yref,
axref: axref,
ayref: ayref
xaxistype: xaxistype,
yaxistype: yaxistype,
xid: xid,
yid: yid
});
|
|
||
|
|
||
| function imageTestCombos() { | ||
| var testCombos = Array.from(iterable.cartesianProduct( |
There was a problem hiding this comment.
Might be non-blocking:
Array.from is not supported in some browsers namely IE11: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/from
| @@ -0,0 +1,3 @@ | |||
| // module.exports can be set to a specific test number to run just a single | |||
| // test, otherwise if set to undefined, runs all the tests. | |||
| module.exports = undefined; | |||
There was a problem hiding this comment.
Sorry. I don't understand if we really need this to be a separate file?
There was a problem hiding this comment.
It's there because jasmine can't read environment variables which would be handy to pin point a specific test that is not passing. So instead I made a file that you change. But I can remove this now because the tests pass.
There was a problem hiding this comment.
@nicholas-esterer have you seen fit and fdescribe? https://davidtang.io/2016/01/03/controlling-which-tests-run-in-jasmine.html
There was a problem hiding this comment.
Thanks, those are handy but in this case I had a for loop that generates many it(...) so if I changed it to fit(...) it would disable all the tests. Plus I personally don't like changing the source file, environment variables are more ephemeral and the risk of them getting committed is lower. But thanks for the tip!
| var aroBBox = shapeToBBox(gd.layout, aro); | ||
| var ret = compareBBoxes(aroBBox, aroPathBBox); | ||
| console.log('aroBBox: ' + JSON.stringify(aroBBox)); | ||
| console.log('aroPathBBox: ' + JSON.stringify(SVGTools.svgRectToObj(aroPathBBox))); |
There was a problem hiding this comment.
Please remove or comment these console.logs here and in other tests.
because it is always true wherever this is called.
You can set testnumber in test/jasmine/tests/domain_ref_test.js if you want to run a single test.
now they use opt instead of many arguments
This wasn't picked up by a local run of `npm run test-syntax`
|
OK looks like all the checkboxes are in place, which is great! anything still blocking here @archmoj @alexcjohnson ? |
|
@nicholas-esterer this is looking great, nice work! Just one blocking comment ( |
|
💃 domain 💃 |
Resolves #4958.
These elements can now be drawn relative to an axis's domain, so if this domain is updated, say, the element's position will update its position according to the new length and offset of the domain.
For example, to add a horizontal line that always spans the x axis and starts 25% of the way up the y axis, you would add such a shape to the layout:
You can see more examples of shapes by looking in
test/domain_ref_tests.html. I've written a bunch of tests there that will eventually be made into jasmine tests (we can rebase out that file once that happens). I'll push some more tests (related to editing of shapes and annotations) as I make them.