-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Annotations drag fix #1441
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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, | ||
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); | ||
}); | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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() { | ||
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
}); | ||
}); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done. Thanks!