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.

Sign up for GitHub

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

OrbitControls: Set certain event listeners to non-passive #19782

Closed
wants to merge 3 commits into from
Closed

OrbitControls: Set certain event listeners to non-passive #19782

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b32f7c4
Set certain event listeners to non-passive
WestLangley Jul 3, 2020
e89fc38
Merge branch 'dev' into dev_orbit_controls_passive
WestLangley Sep 2, 2020
ec9d2bc
set wheel listener to non-passive
WestLangley Sep 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions examples/js/controls/OrbitControls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1125,11 +1125,11 @@ THREE.OrbitControls = function ( object, domElement ) {
scope.domElement.addEventListener( 'contextmenu', onContextMenu, false );

scope.domElement.addEventListener( 'mousedown', onMouseDown, false );
scope.domElement.addEventListener( 'wheel', onMouseWheel, false );
scope.domElement.addEventListener( 'wheel', onMouseWheel, { capture: false, passive: false } );
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find on the internets if capture is false by default...

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For context, useCapture (the third parameter) is supposed to be false by default, but Firefox used to error a decade ago if it was not specified.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See this spec, but the relevant section is non-normative.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

capture (a boolean, initially false)

I guess I can remove it from the options object then?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since non-normative means the specifications in that section are recommendations, not requirements, I think it would be best if we specify the value we want.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was reading a different section than you were.

When set to true, options’s capture prevents callback from being invoked when the event’s eventPhase attribute value is BUBBLING_PHASE. When false (or not present), ...

Copy link
Owner

@mrdoob mrdoob Sep 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I guess it should just be this?

scope.domElement.addEventListener( 'wheel', onMouseWheel, { passive: false } );

PS: I realised I didn't explain the useCapture issue property. Firefox used to crash because the third parameter was not passed.


scope.domElement.addEventListener( 'touchstart', onTouchStart, false );
scope.domElement.addEventListener( 'touchstart', onTouchStart, { capture: false, passive: false } );
scope.domElement.addEventListener( 'touchend', onTouchEnd, false );
scope.domElement.addEventListener( 'touchmove', onTouchMove, false );
scope.domElement.addEventListener( 'touchmove', onTouchMove, { capture: false, passive: false } );

scope.domElement.addEventListener( 'keydown', onKeyDown, false );

Expand Down
6 changes: 3 additions & 3 deletions examples/jsm/controls/OrbitControls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1134,11 +1134,11 @@ var OrbitControls = function ( object, domElement ) {
scope.domElement.addEventListener( 'contextmenu', onContextMenu, false );

scope.domElement.addEventListener( 'mousedown', onMouseDown, false );
scope.domElement.addEventListener( 'wheel', onMouseWheel, false );
scope.domElement.addEventListener( 'wheel', onMouseWheel, { capture: false, passive: false } );

scope.domElement.addEventListener( 'touchstart', onTouchStart, false );
scope.domElement.addEventListener( 'touchstart', onTouchStart, { capture: false, passive: false } );
scope.domElement.addEventListener( 'touchend', onTouchEnd, false );
scope.domElement.addEventListener( 'touchmove', onTouchMove, false );
scope.domElement.addEventListener( 'touchmove', onTouchMove, { capture: false, passive: false } );

scope.domElement.addEventListener( 'keydown', onKeyDown, false );

Expand Down