Skip to content

gl2d data-referenced annotations #1301

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 14 commits into from
Jan 19, 2017
Merged
Show file tree
Hide file tree
Changes from 8 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
2 changes: 1 addition & 1 deletion src/plots/cartesian/axes.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ axes.getFromTrace = axisIds.getFromTrace;
*/
axes.coerceRef = function(containerIn, containerOut, gd, attr, dflt, extraOption) {
var axLetter = attr.charAt(attr.length - 1),
axlist = gd._fullLayout._has('gl2d') ? [] : axes.listIds(gd, axLetter),
axlist = axes.listIds(gd, axLetter),
refAttr = attr + 'ref',
attrDef = {};

Expand Down
2 changes: 2 additions & 0 deletions src/plots/gl2d/camera.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ function createCamera(scene) {
result.lastInputTime = Date.now();
unSetAutoRange();
scene.cameraChanged();
scene.handleAnnotations();
}
break;
}
Expand Down Expand Up @@ -152,6 +153,7 @@ function createCamera(scene) {
result.lastInputTime = Date.now();
unSetAutoRange();
scene.cameraChanged();
scene.handleAnnotations();
break;
}

Expand Down
35 changes: 30 additions & 5 deletions src/plots/gl2d/scene2d.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

'use strict';

var Registry = require('../../registry');
var Axes = require('../../plots/cartesian/axes');
var Fx = require('../../plots/cartesian/graph_interact');

Expand Down Expand Up @@ -237,6 +238,11 @@ proto.updateSize = function(canvas) {
};

proto.computeTickMarks = function() {
this.xaxis.setScale();
this.yaxis.setScale();

// override _length from backward compatibility
// even though setScale 'should' give the correct result
this.xaxis._length =
this.glplot.viewBox[2] - this.glplot.viewBox[0];
this.yaxis._length =
Expand Down Expand Up @@ -286,10 +292,11 @@ proto.updateFx = function(options) {

fullLayout.dragmode = options.dragmode;
fullLayout.hovermode = options.hovermode;
};

var relayoutCallback = function(scene) {
this.graphDiv._fullLayout = fullLayout;
Copy link
Collaborator

Choose a reason for hiding this comment

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

this seems a bit scary - why is this necessary? I'm just worried that if gd._fullLayout and scene.fullLayout have diverged, there could somehow be changes in gd._fullLayout that this will throw away.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe the problem is in the doModebar relayout shortcut.

I'll double-check why the _fullLayout gets lost. Thanks for the headsup!

Copy link
Collaborator

Choose a reason for hiding this comment

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

would it be an option to not use this.fullLayout at all, but only to use this.graphDiv._fullLayout?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's an idea.

So, the _fullLayout gets lost in Plots.supplyDefaults. We could add a block handling refs on scene objects similar to this one for cartesian axes. But I think we could do better.

The reason the lost of reference only affects dragmode relayouts is because all other relayout calls pass through scene2d.plot which update the fullLayout ref here on every pass.

Copy link
Collaborator

Choose a reason for hiding this comment

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

alright, well I'm not entirely sure what would happen, but the test would be something like:

  • do a relayout that causes a new gd._fullLayout to be constructed by supplyDefaults but DOESN'T result in a new scene2d.plot (like what? change legend background color? change something cosmetic on an axis of a different subplot?)
  • trigger this code by changing dragmode
  • do something else that causes a full plot redraw (maybe just Plotly.redraw(gd))
  • does the initial relayout get reverted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's the cleanest way (I think) of updating gd._fullLayout._plots.xy._scene2d.fullLayout

b50b7e9

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah, that seems better, thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

full solution in #1319

};

function relayoutCallback(scene) {
var xrange = scene.xaxis.range,
yrange = scene.yaxis.range;

Expand All @@ -300,14 +307,16 @@ var relayoutCallback = function(scene) {
scene.graphDiv.layout.yaxis.range = yrange.slice(0);

// Make a meaningful value to be passed on to the possible 'plotly_relayout' subscriber(s)
var update = { // scene.camera has no many useful projection or scale information
lastInputTime: scene.camera.lastInputTime // helps determine which one is the latest input (if async)
// scene.camera has no many useful projection or scale information
// helps determine which one is the latest input (if async)
var update = {
lastInputTime: scene.camera.lastInputTime
};
update[scene.xaxis._name] = xrange.slice();
update[scene.yaxis._name] = yrange.slice();

scene.graphDiv.emit('plotly_relayout', update);
};
}

proto.cameraChanged = function() {
var camera = this.camera;
Expand All @@ -321,10 +330,25 @@ proto.cameraChanged = function() {
this.glplotOptions.ticks = nextTicks;
this.glplotOptions.dataBox = camera.dataBox;
this.glplot.update(this.glplotOptions);
this.handleAnnotations();

relayoutCallback(this);
}
};

proto.handleAnnotations = function() {
var gd = this.graphDiv,
annotations = this.fullLayout.annotations;

for(var i = 0; i < annotations.length; i++) {
var ann = annotations[i];

if(ann.xref === this.xaxis._id && ann.yref === this.yaxis._id) {
Registry.getComponentMethod('annotations', 'drawOne')(gd, i);
}
}
};

proto.destroy = function() {
var traces = this.traces;

Expand Down Expand Up @@ -406,6 +430,7 @@ proto.plot = function(fullData, calcData, fullLayout) {
ax._length = options.viewBox[i + 2] - options.viewBox[i];

Axes.doAutoRange(ax);
ax.setScale();
}

options.ticks = this.computeTickMarks();
Expand Down
Binary file added test/image/baselines/gl2d_annotations.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
52 changes: 52 additions & 0 deletions test/image/mocks/gl2d_annotations.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
{
"data":[{
"type": "scattergl",
"x":[1,1,1,1,1,2,2,2,2,2,3,3,3,3,3,4,4,4,4,4,5,5,5,5,5],
"y":[1,2,3,4,5,1,2,3,4,5,1,2,3,4,5,1,2,3,4,5,1,2,3,4,5],
"mode":"markers"
}],
"layout": {
"yaxis":{"autorange":false,"range":[1,5],"showgrid":false,"zeroline":false,"showticklabels":false},
"xaxis":{"autorange":false,"range":[1,5],"showgrid":false,"zeroline":false,"showticklabels":false},
"height":500,
"width":800,
"margin": {"l":100,"r":100,"top":80,"bottom":80,"pad":0},
"showlegend":false,
"annotations":[
{"text":"left top","showarrow":false,"xref":"paper","yref":"paper","xanchor":"left","yanchor":"top","x":0,"y":1},
{"text":"center middle","showarrow":false,"xref":"paper","yref":"paper","xanchor":"center","yanchor":"middle","x":0.25,"y":1},
{"text":"right bottom","showarrow":false,"xref":"paper","yref":"paper","xanchor":"right","yanchor":"bottom","x":0.5,"y":1},
{"text":"move with page","xref":"paper","yref":"paper","x":0.75,"y":1},
{"text":"opacity","opacity":0.5,"x":5,"y":5},
{"text":"not-visible", "visible": false},
{"text":"left<br>justified","showarrow":false,"align":"left","x":1,"y":4},
{"text":"center<br>justified","showarrow":false,"x":2,"y":4},
{"text":"right<br>justified","showarrow":false,"align":"right","x":3,"y":4},
{"text":"no arrow<br>page auto TR","showarrow":false,"xref":"paper","yref":"paper","x":0.75,"y":0.75},
{"text":"no arrow<br>page auto ML","showarrow":false,"xref":"paper","yref":"paper","x":0.25,"y":0.5},
{"text":"no arrow<br>page auto BC","showarrow":false,"xref":"paper","yref":"paper","x":0.5,"y":0.25},
{"text":"default"},
{"text":"no arrow","x":5,"y":4,"showarrow":false},
{"text":"border","showarrow":false,"bordercolor":"rgb(148, 103, 189)","x":4,"y":3},
{"text":"border width","showarrow":false,"bordercolor":"rgb(0, 0, 255)","borderwidth":3,"x":5,"y":3},
{"text":"background","showarrow":false,"bgcolor":"rgb(255, 127, 14)","x":4,"y":2},
{"text":"padding","showarrow":false,"bordercolor":"rgb(0, 0, 0)","borderpad":3,"x":5,"y":2},
{"text":"angle<br>Bottom R","showarrow":false,"textangle":40,"x":1,"y":1,"xanchor":"right","yanchor":"bottom"},
{"text":"angle<br>Middle C","showarrow":false,"textangle":-70,"x":2,"y":1,"xanchor":"center","yanchor":"middle"},
{"text":"angle<br>Top L","showarrow":false,"textangle":160,"x":3,"y":1,"xanchor":"left","yanchor":"top"},
{
"text":"arrowhead styling","arrowcolor":"rgb(214, 39, 40)","arrowwidth":4.1,"arrowhead":7,
"ax":-34,"ay":37,"arrowsize":2,"x":4,"y":1
},
{
"text":"All together<br>with<sup style=\"font-family:raleway;fill:rgb(0,255,255)\">STYLE</sup>",
"opacity":0.6,"arrowwidth":5,"arrowhead":3,"ax":-77,"ay":-5,
"bordercolor":"rgb(255, 0, 0)","borderwidth":4,"bgcolor":"rgba(255,255,0,0.5)",
"font":{"color":"rgb(0, 0, 255)","size":20},
"arrowcolor":"rgb(166, 28, 0)","borderpad":3,"textangle":50,"x":5,"y":1
},
{"text":"","showarrow":true,"borderwidth":1.2,"arrowhead":2,"axref":"x","ayref":"y","x":5,"y":3,"ax":4,"ay":5},
{"text":"","showarrow":true,"borderwidth":1.2,"arrowhead":2,"axref":"x","ayref":"y","x":6,"y":2,"ax":3,"ay":3}
]
}
}
65 changes: 65 additions & 0 deletions test/jasmine/tests/gl_plot_interact_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -710,3 +710,68 @@ describe('Test gl plot side effects', function() {
}).then(done);
});
});

describe('gl2d interaction', function() {
var gd;

beforeAll(function() {
jasmine.addMatchers(customMatchers);
});

beforeEach(function() {
gd = createGraphDiv();
});

afterEach(function() {
Plotly.purge(gd);
destroyGraphDiv();
});

it('data-referenced annotations should update on drag', function(done) {

function drag(start, end) {
var opts = { buttons: 1 };

mouseEvent('mousemove', start[0], start[1], opts);
mouseEvent('mousedown', start[0], start[1], opts);
mouseEvent('mousemove', end[0], end[1], opts);
mouseEvent('mouseup', end[0], end[1], opts);
}

function assertAnnotation(xy) {
var ann = d3.select('g.annotation-text-g');
var x = +ann.attr('x');
var y = +ann.attr('y');

expect([x, y]).toBeCloseToArray(xy);
}

Plotly.plot(gd, [{
type: 'scattergl',
x: [1, 2, 3],
y: [2, 1, 2]
}], {
annotations: [{
x: 2,
y: 1,
text: 'text'
}],
dragmode: 'pan'
})
.then(function() {
assertAnnotation([340, 334]);

drag([250, 200], [150, 300]);
assertAnnotation([410, 264]);

return Plotly.relayout(gd, {
'xaxis.range': [1.5, 2.5],
'yaxis.range': [1, 1.5]
});
})
.then(function() {
assertAnnotation([340, 340]);
})
.then(done);
});
});