-
-
Notifications
You must be signed in to change notification settings - Fork 35.3k
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
Examples: Fix controls in webgl_multiple_scenes_comparison. #20223
Conversation
@@ -7,6 +7,10 @@ | |||
<link type="text/css" rel="stylesheet" href="main.css"> | |||
<style> | |||
|
|||
html, body { | |||
touch-action: none; |
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 was not able to fix touch controls without this CSS, see https://stackoverflow.com/questions/48124372/pointermove-event-not-working-with-touch-why-not
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 also battled with this today. Safari/iOS doesn't support this.
I ended up doing this instead:
window.addEventListener( 'touchstart', function ( event ) {
event.preventDefault(); // prevent scrolling
} );
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.
If I add this code, I get the following runtime error in Chrome:
webgl_multiple_scenes_comparison:190 [Intervention] Unable to preventDefault inside passive event listener due to target being treated as passive. See https://www.chromestatus.com/features/5093566007214080
It only seems to work if I mark the listener as non-passive:
window.addEventListener( 'touchstart', function ( event ) {
event.preventDefault(); // prevent scrolling
}, { passive: false } );
Not sure if this is appropriate 🤔 .
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.
Ah, maybe document.body
instead?
This touchstart
and touch-action
gotcha is such a pain...
Maybe in this case touch-action
is better actually.
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.
Ah, maybe document.body instead?
Unfortunately, I see the same error with document.body
.
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.
In TransformControls
I did this:
domElement.addEventListener( 'touchstart', function ( event ) {
event.preventDefault(); // prevent scrolling
} );
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.
Um, why does this approach work with TransformControls
but not in the example 🤔 ?
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.
Maybe because I'm adding it to the renderer.domElement
directly?
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've tested this right now. The errors go away but the UI behavior is still the same. Meaning pointermove
gets canceled after a short usage.
It only works if I add all event listeners to renderer.domElement
. But then it's not possible to just drag the slider
element.
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.
Okay, I'll merge this and play around with the code 👍
Thanks! |
I ended up doing quite a bit of clean up: 65494f3 But the solution was adding the listener to slider.addEventListener( 'touchstart', function ( event ) {
event.preventDefault(); // prevent scrolling
} ); I have not tested if |
Okay, seems like I'll update the code. |
see #20194 (comment)