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

event.keyCode is deprecated, should upgrade to event.code #849

Closed
mengwong opened this issue Mar 4, 2023 · 3 comments · May be fixed by #851
Closed

event.keyCode is deprecated, should upgrade to event.code #849

mengwong opened this issue Mar 4, 2023 · 3 comments · May be fixed by #851

Comments

@mengwong
Copy link
Contributor

mengwong commented Mar 4, 2023 •
edited
Loading

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

You should avoid using this if possible; it's been deprecated for some time. Instead, you should use KeyboardEvent.code, if it's implemented. Unfortunately, some browsers still don't have it, so you'll have to be careful to make sure you use one which is supported on all target browsers.

event.keyCode shows up in places like

impress.js/src/plugins/navigation/navigation.js

Lines 95 to 107 in 9475720

switch ( event.keyCode ) {
case 33: // Pg up
case 37: // Left
case 38: // Up
api.prev( event );
break;
case 9: // Tab
case 32: // Space
case 34: // Pg down
case 39: // Right
case 40: // Down
api.next( event );
break;

Unless there are any objections I will fork and work toward a PR to fix this.
mengwong added a commit to smucclaw/impress.js that referenced this issue Mar 5, 2023
@mengwong
replace event.keyCode with event.key
bb37a47 
due to deprecation KeyboardEvent.keyCode

resolves impress#849
@janishutz
Copy link
Contributor

That's good

@mengwong mengwong mentioned this issue Mar 5, 2023
Open
@janishutz
Copy link
Contributor

What I expected to be a problem with this solution: https://caniuse.com/?search=keyboardevent.code

This will break impress.js in older browsers like IE and mobile browsers (aka. Safari for iOS, Firefox for Android, etc.)

Compare it to: https://caniuse.com/?search=keyboardevent.keyCode, which is supported by almost every major browser out there.

We should consider implementing a way to notify the user about incompatible plugins & automatically disable the plugins relying on these features, if the browser does not support it to retain support for old browsers with the most recent versions of impress.js.

Although this is great to offer, I personally am in favour of dropping support for the old browsers barely anyone uses any more nowadays. People that need to use those browsers should use a different CDN-Link or V2.0.0 instead of a new (future) release or upstream.

@mengwong
Copy link
Contributor Author

mengwong commented Mar 5, 2023 •
edited
Loading

Let's move the discussion into the PR #851

@mengwong mengwong closed this as completed Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

replace event.keyCode with event.code, tweak documentation smucclaw/impress.js
2 participants
@mengwong @janishutz