-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Enable touch interactions for select/lasso/others #1804
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
Conversation
Interesting... I'll have to take a look at this and play with it, but to answer your question:
We wanted users to be able to drag off the edge of the plot, but without interacting with other things they might be dragging over until they mouse up. So for example put two plotly plots next to each other, and pan one of them (or zoom by dragging one axis end) until the cursor is over the other - there should be no hover effects in the second one. |
@alexcjohnson I don't see any side-effects of panning/dragging over the second plot, although I caught one more bug. |
@dfcreative would you mind making a standalone page with these patches to help us test them? |
Having a bunch of fun playing with @jackparmer wanna give it a shot too? |
@dfcreative Non-blocking for this PR of course, but I'm curious: how hard would it be to mock double-clicks on mobile? |
Well, this thing look great to me. 🎉 @alexcjohnson any objections to those patches in |
src/components/dragelement/index.js
Outdated
|
||
function pointerOffset(e) { | ||
return mouseOffset( | ||
e.changedTouches && e.changedTouches[0] || e, |
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.
not blocking, but I'd prefer more parentheses in cases like this - it works fine, but takes me too much mental effort to work out the operator precedence while reading it.
Yeah, looks great! What would it take to test this? |
@alexcjohnson emulating touch interactions. Although we had some issues reproducing selection test-cases in CI, so now they are disabled. |
👍
Right, we have issue for the select tests for |
@etpinard @alexcjohnson done. Probably we may want to enable touch interactions for rangeslider? |
That's in -> #1098 would be nice to do at some point. |
@etpinard may I ask you to run |
Looks great to me! FWIW I played with it at 473f72a on desktop, and I saw the issues that led to the creation of the |
This reverts commit 5dffeab.
- so that window.TouchEvent API works in Ubuntu
✅ on CI, @dfcreative merge away 💃 |
Resolves #1790
We have to avoid creating
dragCover
for touch-only devices, becausetouchmove
, unlikemousemove
, requires the same element wheretouchstart
happened.Is there any particular reason we don't use
document
for catching mouse events? What makesdragCover
different fromdocument
?@etpinard @alexcjohnson
TODO:
dragCover
does not hide cursor for panning and makes it a text cursor, we have to disableselection in a way similar to this