-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Correctly assign click event target in shadow dom #7357
Conversation
and remove obsolete API for creating new synthetic mouse events
@@ -1059,17 +1060,17 @@ describe('Test click interactions:', function() { | |||
width: 600, | |||
height: 600 | |||
}).then(function() { | |||
expect(gd.layout.xaxis.range).toBeCloseToArray([1, 2.068]); | |||
expect(gd.layout.xaxis.range).toBeCloseToArray([1, 2.068], 1); |
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.
I slightly loosened this test, then this whole file passed locally. I had to look it up, apparently "1" as the second arg means a range of 10^-1, ie a span of 0.1, ie +/- 0.05, whereas the default is +/- 0.005 and I was getting errors of ~0.007.
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.
Looks good, thanks @alexcjohnson ! 🚀
Thanks @alexcjohnson! |
Fixes #6108
When you start a drag - which happens on mouse down, even before you actually move the mouse - we attach a "coverslip" element to
document.body
, covering the entire page, so that we can continue tracking mouse movements outside the graph while the mouse is down. This is useful (and expected by users) for panning and sometimes zooming, the drag shouldn't stop when the mouse leaves the graph. We could look into whether this is still necessary in today's browsers, which have gotten more sophisticated about targeting events, but it was clearly required in the early days of plotly.js. Anyway the problem comes when the graph is in a shadow DOM, because then it's not really allowed access todocument.body
. The net effect of this is events that had been targeted at elements inside the graph, after the browser sees that they're being handled by the coverslip, they look like they're targeted at the shadow DOM container element instead, and when we use this element for position reference instead of the original target, we get the wrong coordinates so we select the wrong data point.I fixed this by packaging the original event into a plain object instead of a
MouseEvent
, ensuring the correct target element is included, and passing that along to our handlers.Adapted from @slietar's codepen, here's the code I've been using in the test dashboard to test this fix:
While I was at it I removed an obsolete fallback for creating new synthetic mouse events when
new MouseEvent
fails. This appears to have ~10 years of support.