Skip to content

Annotations drag fix #1441

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 3 commits into from
Mar 3, 2017
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions src/components/annotations/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,13 @@ function drawOne(gd, index) {
posPx.text = basePx + textShift;
}

// padplus/minus are used by autorange
options['_' + axLetter + 'padplus'] = (annSize / 2) + textPadShift;
options['_' + axLetter + 'padminus'] = (annSize / 2) - textPadShift;

// size/shift are used during dragging
options['_' + axLetter + 'size'] = annSize;
options['_' + axLetter + 'shift'] = textShift;
});

if(annotationIsOffscreen) {
Expand Down
78 changes: 78 additions & 0 deletions test/jasmine/assets/drag.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
var mouseEvent = require('../assets/mouse_event');

/*
* drag: grab a node and drag it (dx, dy) pixels
* optionally specify an edge ('n', 'se', 'w' etc)
* to grab it by an edge or corner (otherwise the middle is used)
*/
module.exports = function(node, dx, dy, edge) {

edge = edge || '';
var bbox = node.getBoundingClientRect(),
fromX, fromY;

if(edge.indexOf('n') !== -1) fromY = bbox.top;
else if(edge.indexOf('s') !== -1) fromY = bbox.bottom;
else fromY = (bbox.bottom + bbox.top) / 2;

if(edge.indexOf('w') !== -1) fromX = bbox.left;
else if(edge.indexOf('e') !== -1) fromX = bbox.right;
else fromX = (bbox.left + bbox.right) / 2;


var toX = fromX + dx,
toY = fromY + dy;

mouseEvent('mousemove', fromX, fromY, {element: node});
mouseEvent('mousedown', fromX, fromY, {element: node});

var promise = waitForDragCover().then(function(dragCoverNode) {
mouseEvent('mousemove', toX, toY, {element: dragCoverNode});
mouseEvent('mouseup', toX, toY, {element: dragCoverNode});
return waitForDragCoverRemoval();
});

return promise;
};

function waitForDragCover() {
return new Promise(function(resolve) {
var interval = 5,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Previously this was DBLCLICKDELAY/4 which is 75 msec, and it was cumulatively responsible for almost 20 extra seconds of delay in shapes_test. Runs fine at 5 msec.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nicely done. Thanks!

timeout = 5000;

var id = setInterval(function() {
var dragCoverNode = document.querySelector('.dragcover');
if(dragCoverNode) {
clearInterval(id);
resolve(dragCoverNode);
}

timeout -= interval;
if(timeout < 0) {
clearInterval(id);
throw new Error('waitForDragCover: timeout');
}
}, interval);
});
}

function waitForDragCoverRemoval() {
return new Promise(function(resolve) {
var interval = 5,
timeout = 5000;

var id = setInterval(function() {
var dragCoverNode = document.querySelector('.dragcover');
if(!dragCoverNode) {
clearInterval(id);
resolve(dragCoverNode);
}

timeout -= interval;
if(timeout < 0) {
clearInterval(id);
throw new Error('waitForDragCoverRemoval: timeout');
}
}, interval);
});
}
4 changes: 3 additions & 1 deletion test/jasmine/assets/mouse_event.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module.exports = function(type, x, y, opts) {
fullOpts.buttons = opts.buttons;
}

var el = document.elementFromPoint(x, y),
var el = (opts && opts.element) || document.elementFromPoint(x, y),
ev;

if(type === 'scroll') {
Expand All @@ -20,4 +20,6 @@ module.exports = function(type, x, y, opts) {
}

el.dispatchEvent(ev);

return el;
};
231 changes: 231 additions & 0 deletions test/jasmine/tests/annotations_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ var customMatchers = require('../assets/custom_matchers');
var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');
var failTest = require('../assets/fail_test');
var drag = require('../assets/drag');


describe('Test annotations', function() {
Expand Down Expand Up @@ -740,3 +741,233 @@ describe('annotation clicktoshow', function() {
.then(done);
});
});

describe('annotation dragging', function() {
var gd;

function textDrag() { return gd.querySelector('.annotation-text-g>g'); }
function arrowDrag() { return gd.querySelector('.annotation-arrow-g>.anndrag'); }
function textBox() { return gd.querySelector('.annotation-text-g'); }

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

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

// we've already tested autorange with relayout, so fix the geometry
// completely so we know exactly what we're dealing with
// plot area is 300x300, and covers data range 100x100
Plotly.plot(gd,
[{x: [0, 100], y: [0, 100], mode: 'markers'}],
{
xaxis: {range: [0, 100]},
yaxis: {range: [0, 100]},
width: 500,
height: 500,
margin: {l: 100, r: 100, t: 100, b: 100, pad: 0}
},
{editable: true}
)
.then(done);
});

afterEach(destroyGraphDiv);

function initAnnotation(annotation) {
return Plotly.relayout(gd, {annotations: [annotation]})
.then(function() {
return Plots.previousPromises(gd);
});
}

function dragAndReplot(node, dx, dy, edge) {
return drag(node, dx, dy, edge).then(function() {
return Plots.previousPromises(gd);
});
}

/*
* run through a series of drags of the same annotation
* findDragger: fn that returns the element to drag on
* (either textDrag or ArrowDrag)
* autoshiftX, autoshiftY: how much does the annotation anchor shift
* moving between one region and the next. Zero except if autoanchor
* is active, ie paper-referenced with no arrow
* coordScale: how big is the full plot? paper-referenced has scale 1
* and for the plot defined above, data-referenced has scale 100
*/
function checkDragging(findDragger, autoshiftX, autoshiftY, coordScale) {
var bboxInitial = textBox().getBoundingClientRect();
// first move it within the same auto-anchor zone
return dragAndReplot(findDragger(), 30, -30)
.then(function() {
var bbox = textBox().getBoundingClientRect();

// I'm not sure why these calculations aren't exact - they end up
// being off by a fraction of a pixel, or a full pixel sometimes
// even though as far as I can see in practice the positioning is
// exact. In any event, this precision is enough to ensure that
// anchor: auto is being used.
expect(bbox.left).toBeWithin(bboxInitial.left + 30, 1);
expect(bbox.top).toBeWithin(bboxInitial.top - 30, 1);

var ann = gd.layout.annotations[0];
expect(ann.x).toBeWithin(0.1 * coordScale, 0.01 * coordScale);
expect(ann.y).toBeWithin(0.1 * coordScale, 0.01 * coordScale);

// now move it to the center
// note that we explicitly offset by half the box size because the
// auto-anchor will move to the center
return dragAndReplot(findDragger(), 120 - autoshiftX, -120 + autoshiftY);
})
.then(function() {
var bbox = textBox().getBoundingClientRect();
expect(bbox.left).toBeWithin(bboxInitial.left + 150 - autoshiftX, 2);
expect(bbox.top).toBeWithin(bboxInitial.top - 150 + autoshiftY, 2);

var ann = gd.layout.annotations[0];
expect(ann.x).toBeWithin(0.5 * coordScale, 0.01 * coordScale);
expect(ann.y).toBeWithin(0.5 * coordScale, 0.01 * coordScale);

// next move it near the upper right corner, where the auto-anchor
// moves to the top right corner
// we don't move it all the way to the corner, so the annotation will
// still be entirely on the plot even with an arrow.
return dragAndReplot(findDragger(), 90 - autoshiftX, -90 + autoshiftY);
})
.then(function() {
var bbox = textBox().getBoundingClientRect();
expect(bbox.left).toBeWithin(bboxInitial.left + 240 - 2 * autoshiftX, 2);
expect(bbox.top).toBeWithin(bboxInitial.top - 240 + 2 * autoshiftY, 2);

var ann = gd.layout.annotations[0];
expect(ann.x).toBeWithin(0.8 * coordScale, 0.01 * coordScale);
expect(ann.y).toBeWithin(0.8 * coordScale, 0.01 * coordScale);

// finally move it back to 0, 0
return dragAndReplot(findDragger(), -240 + 2 * autoshiftX, 240 - 2 * autoshiftY);
})
.then(function() {
var bbox = textBox().getBoundingClientRect();
expect(bbox.left).toBeWithin(bboxInitial.left, 2);
expect(bbox.top).toBeWithin(bboxInitial.top, 2);

var ann = gd.layout.annotations[0];
expect(ann.x).toBeWithin(0 * coordScale, 0.01 * coordScale);
expect(ann.y).toBeWithin(0 * coordScale, 0.01 * coordScale);
});
}

// for annotations with arrows: check that dragging the text moves only
// ax and ay (and the textbox itself)
function checkTextDrag() {
var ann = gd.layout.annotations[0],
x0 = ann.x,
y0 = ann.y,
ax0 = ann.ax,
ay0 = ann.ay;

var bboxInitial = textBox().getBoundingClientRect();

return dragAndReplot(textDrag(), 50, -50)
.then(function() {
var bbox = textBox().getBoundingClientRect();
expect(bbox.left).toBeWithin(bboxInitial.left + 50, 1);
expect(bbox.top).toBeWithin(bboxInitial.top - 50, 1);

ann = gd.layout.annotations[0];

expect(ann.x).toBe(x0);
expect(ann.y).toBe(y0);
expect(ann.ax).toBeWithin(ax0 + 50, 1);
expect(ann.ay).toBeWithin(ay0 - 50, 1);
});
}

it('respects anchor: auto when paper-referenced without arrow', function(done) {
initAnnotation({
x: 0,
y: 0,
showarrow: false,
text: 'blah<br>blah blah',
xref: 'paper',
yref: 'paper'
})
.then(function() {
var bbox = textBox().getBoundingClientRect();

return checkDragging(textDrag, bbox.width / 2, bbox.height / 2, 1);
})
.catch(failTest)
.then(done);
});

it('also works paper-referenced with explicit anchors and no arrow', function(done) {
initAnnotation({
x: 0,
y: 0,
showarrow: false,
text: 'blah<br>blah blah',
xref: 'paper',
yref: 'paper',
xanchor: 'left',
yanchor: 'top'
})
.then(function() {
// with offsets 0, 0 because the anchor doesn't change now
return checkDragging(textDrag, 0, 0, 1);
})
.catch(failTest)
.then(done);
});

it('works paper-referenced with arrows', function(done) {
initAnnotation({
x: 0,
y: 0,
text: 'blah<br>blah blah',
xref: 'paper',
yref: 'paper',
ax: 30,
ay: 30
})
.then(function() {
return checkDragging(arrowDrag, 0, 0, 1);
})
.then(checkTextDrag)
.catch(failTest)
.then(done);
});

it('works data-referenced with no arrow', function(done) {
initAnnotation({
x: 0,
y: 0,
showarrow: false,
text: 'blah<br>blah blah'
})
.then(function() {
return checkDragging(textDrag, 0, 0, 100);
})
.catch(failTest)
.then(done);
});

it('works data-referenced with arrow', function(done) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Beautiful tests 🎉

initAnnotation({
x: 0,
y: 0,
text: 'blah<br>blah blah',
ax: 30,
ay: -30
})
.then(function() {
return checkDragging(arrowDrag, 0, 0, 100);
})
.then(checkTextDrag)
.catch(failTest)
.then(done);
});
});
Loading