-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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
refactor: Replace keycode dependency with event.key #8735
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8735 +/- ##
==========================================
- Coverage 83.15% 83.00% -0.15%
==========================================
Files 120 119 -1
Lines 7995 7991 -4
Branches 1924 1921 -3
==========================================
- Hits 6648 6633 -15
- Misses 1347 1358 +11 ☔ View full report in Codecov by Sentry. |
@@ -298,19 +297,19 @@ class MenuButton extends Component { | |||
handleKeyDown(event) { | |||
|
|||
// Escape or Tab unpress the 'button' | |||
if (keycode.isEventKey(event, 'Esc') || keycode.isEventKey(event, 'Tab')) { | |||
if (event.key === 'Esc' || event.key === 'Tab') { |
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.
looks like during review, we missed that these should be Escape
rather than Esc
, which keycode likely helped with. Someone noticed this issue in slack.
Description
Removes the keycode dependency. That relies on now deprecated event properties.
KeyboardEvent.key
is now ubiquitous, so can be used directly instead.Reduces min.js slightly, 646.9 kB => 645.22 kB
The uses of deprecated
keyCode
in utils/spatial-navigation-keycode.js ought to be reviewed too.Requirements Checklist