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

Jump to bottom

OrbitControls/TrackballControls: Replace event.keyCode with event.code. #21409

Merged
merged 24 commits into from
Mar 30, 2021
Merged

OrbitControls/TrackballControls: Replace event.keyCode with event.code. #21409

merged 24 commits into from
Mar 30, 2021

Conversation

puxiao
Copy link
Contributor

@puxiao puxiao commented Mar 4, 2021 •
edited by Mugen87
Loading

Related issue: Related #21056.

Description

The deprecated KeyboardEvent.keyCode read-only property represents a system and implementation dependent numerical code identifying the unmodified value of the pressed key.

https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode

so...

// The four arrow keys
- this.keys = { LEFT: 37, UP: 38, RIGHT: 39, BOTTOM: 40 };
+ this.keys = { LEFT: 'ArrowLeft', UP: 'ArrowUp', RIGHT: 'ArrowRight', BOTTOM: 'ArrowDown' };


function handleKeyDown( event ) {

-		switch ( event.keyCode )
+		switch ( event.code )
		
}

@puxiao
OrbitControls: replace event.keyCode with event.code
c8a9d07 
issue: -

**Description**

The deprecated **`KeyboardEvent.keyCode`** read-only property represents a system and implementation dependent numerical code identifying the unmodified value of the pressed key. 

https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode



so...

```diff
// The four arrow keys
- this.keys = { LEFT: 37, UP: 38, RIGHT: 39, BOTTOM: 40 };
+ this.keys = { LEFT: 'ArrowLeft', UP: 'ArrowUp', RIGHT: 'ArrowRight', BOTTOM: 'ArrowDown' };


function handleKeyDown( event ) {

-		switch ( event.keyCode )
-		switch ( event.code )
		
}
```
@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 4, 2021 •
edited
Loading

Please always modify the code in example/js and then generate the modules by executing node utils/modularize.js from the project's root directory.

Besides, it would be great if you change TrackballControls, too.

Sidenote: KeyboardEvent.code does not work in IE.

@puxiao
Copy link
Contributor Author

puxiao commented Mar 4, 2021

@Mugen87 OK, KeyboardEvent.key work in IE.

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 4, 2021 •
edited
Loading

I don't think it's necessary to honor IE anymore. I've just noted it for clarification reasons.

@puxiao
Copy link
Contributor Author

puxiao commented Mar 4, 2021

@Mugen87 How to modify OrbitControls.d.ts and TrackballControls.d.ts ?

OrbitControls.d.ts

-	keys: { LEFT: number; UP: number; RIGHT: number; BOTTOM: number; };
+	keys: { LEFT: string; UP: string; RIGHT: string; BOTTOM: string; };

TrackballControls.d.ts

-	keys: number[];
+	keys: string[];

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 4, 2021

https://raw.githack.com/puxiao/three.js/patch-1/examples/index.html

Two examples configure controls.keys which do not work anymore with this change. You have to change webgl_shading_physical and misc_controls_trackball.

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 4, 2021

It's also necessary to update the documentation pages:

https://threejs.org/docs/index.html#examples/en/controls/OrbitControls.keys
https://threejs.org/docs/index.html#examples/en/controls/TrackballControls.keys
https://threejs.org/docs/index.html#examples/zh/controls/OrbitControls.keys
https://threejs.org/docs/index.html#examples/zh/controls/TrackballControls.keys

@puxiao
Copy link
Contributor Author

puxiao commented Mar 4, 2021

@Mugen87
Sorry, I'm just a rookie, I forgot to write custom commit message.
my commits are all "Update xxxx..."

Mugen87
Mugen87 approved these changes Mar 4, 2021
View reviewed changes
Copy link
Collaborator

@Mugen87 Mugen87 left a comment

Choose a reason for hiding this comment

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

Looks good!

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 4, 2021 •
edited
Loading

Don't worry about the commits. @mrdoob can squash/merge so the commits are all combined into a single one.

@Mugen87 Mugen87 changed the title OrbitControls: replace event.keyCode with event.code OrbitControls/TrackballControls: Replace event.keyCode with event.code. Mar 4, 2021
@mrdoob mrdoob added this to the r127 milestone Mar 4, 2021
mrdoob
mrdoob reviewed Mar 4, 2021
View reviewed changes
examples/js/controls/TrackballControls.js
@@ -29,7 +29,7 @@ THREE.TrackballControls = function ( object, domElement ) {
this.minDistance = 0;
this.maxDistance = Infinity;

this.keys = [ 65 /*A*/, 83 /*S*/, 68 /*D*/ ];
this.keys = [ 'KeyA' /*A*/, 'KeyS' /*S*/, 'KeyD' /*D*/ ];
Copy link
Owner

@mrdoob mrdoob Mar 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

How does this change affect french, ... keyboards?

Copy link
Collaborator

Choose a reason for hiding this comment

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

see #21056 (comment)

@mrdoob mrdoob merged commit 6f11651 into mrdoob:dev Mar 30, 2021
@mrdoob
Copy link
Owner

mrdoob commented Mar 30, 2021

Thanks!

@puxiao puxiao deleted the patch-1 branch March 31, 2021 02:26
haywirez added a commit to haywirez/three-stdlib that referenced this pull request May 11, 2021
@haywirez
chore: remove deprecated keyCode in controls
796888a 
Brings three-stdlib up to date with three: mrdoob/three.js#21409
@haywirez haywirez mentioned this pull request May 11, 2021
Merged
3 tasks
joshuaellis pushed a commit to pmndrs/three-stdlib that referenced this pull request May 11, 2021
@haywirez
fix: remove deprecated keyCode in controls (#59)
cf15c71 
Brings three-stdlib up to date with three: mrdoob/three.js#21409
Mercury21th000 pushed a commit to Mercury21th000/three-stdlib that referenced this pull request Nov 17, 2024
@haywirez
fix: remove deprecated keyCode in controls (#59)
37303bf 
Brings three-stdlib up to date with three: mrdoob/three.js#21409
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mugen87 Mugen87 Mugen87 approved these changes

@mrdoob mrdoob mrdoob left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
r127
Development

Successfully merging this pull request may close these issues.
3 participants
@puxiao @Mugen87 @mrdoob