Skip to content
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

Controls: Remove preventDefault() from mouse handlers. #21935

Merged
merged 1 commit into from
Jun 4, 2021

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Jun 1, 2021

Related issue: #21889

Description

This PR removes all calls of preventDefault() in mouse handlers from all controls classes to avoid side-effects mentioned in #21889. During my local testing with the examples I was not able to spot a different behavior.

@mrdoob mrdoob added this to the r130 milestone Jun 4, 2021
@mrdoob mrdoob merged commit f4569ee into mrdoob:dev Jun 4, 2021
@mrdoob
Copy link
Owner

mrdoob commented Jun 4, 2021

Thanks!

@puxiao
Copy link
Contributor

puxiao commented Jun 5, 2021

Are you sure you want to do this?
I think it's too radical and crazy.
Why not add a new attribute to control whether to cancel?

class OrbitControls extends EventDispatcher {

	constructor( object, domElement ) {
	
+		this.preventDefault = true;
	
		function onMouseDown( event ) {
-			event.preventDefault();
		
+			if( this.preventDefault ) {
+				event.preventDefault();
+			}
		}
	}
}

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jun 5, 2021

Why is this change radical and crazy? Do you mind explaining your concerns in more detail?

@puxiao
Copy link
Contributor

puxiao commented Jun 5, 2021

If the <canvas /> is full of document.body, everything is fine.
However, if the <canvas /> is only a part of the document.body, there will be various mouse effects on the current page.

Isn't this the meaning of adding preventDefault() before?

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jun 5, 2021

Well, according to a lot of user feedback the new behavior is the more expected one. I also think that it was a mistake to add event.preventDefault(); in the first place. TransformControls does not use it either and the class works just fine.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jun 5, 2021

I'll add a note in the migration guide. Just to be on the safe side.

@Mugen87 Mugen87 mentioned this pull request Jun 6, 2021
haywirez added a commit to haywirez/three-stdlib that referenced this pull request Jun 14, 2021
haywirez added a commit to haywirez/three-stdlib that referenced this pull request Jun 14, 2021
joshuaellis pushed a commit to pmndrs/three-stdlib that referenced this pull request Jun 14, 2021
* chore: remove event.preventDefault from orbit controls

as per mrdoob/three.js#21935

* chore: remove event.preventDefault from drag controls

* chore: remove event.preventDefault from first person controls

* chore: remove event.preventDefault from fly controls

* chore: remove event.preventDefault from trackball controls
odimond pushed a commit to avs/avs-go that referenced this pull request Mar 7, 2022
…ion, no need to scale on client)

Add support for a grid of canvases for each scene.
Optimize by only creating 1 row or column of canvases for axes scenes.
Re-add event.preventDefault() to mouseDown and mouseMove listeners which were removed in this PR: mrdoob/three.js#21935
Use a "noCreateCanvas" flag to tell the client not to create a canvas grid for a Scene.
Create and dispose of canvases in the grid as they enter and exit the viewport.
Don't reproject static text anchors on a pan, just move the div.
Add zoom support for PanInteractor.
Adapt to window.devicePixelRatio being changed without a page reload.
Fix GPU picking when devicePixelRatio != 1
Update to three.js r137.4
Mercury21th000 pushed a commit to Mercury21th000/three-stdlib that referenced this pull request Nov 17, 2024
* chore: remove event.preventDefault from orbit controls

as per mrdoob/three.js#21935

* chore: remove event.preventDefault from drag controls

* chore: remove event.preventDefault from first person controls

* chore: remove event.preventDefault from fly controls

* chore: remove event.preventDefault from trackball controls
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants