-
Notifications
You must be signed in to change notification settings - Fork 320
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
Use KeyboardEvent.key
over deprecated KeyboardEvent.keyCode
in the Tabs component
#4811
Conversation
`KeyboardEvent.keyCode` is deprecated. All of the browsers that now run our JavaScript support the modern `KeyboardEvent.key` property. Update the button component to using `KeyboardEvent.key` instead, removing hardcoded ASCII values in the `keys` map. Unfortunately Edge 16 uses e.g. “Left" instead of “ArrowLeft”, so we need to add those possible values as well if we want keyboard navigation for tabs to work in that browser. [1]: https://developer.mozilla.org/en-US/docs/Web/API/UI_Events/Keyboard_event_key_values#navigation_keys
📋 StatsFile sizes
Modules
View stats and visualisations on the review app Action run for 964a453 |
JavaScript changes to npm packagediff --git a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js
index 77bb7c649..13886ee98 100644
--- a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js
+++ b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js
@@ -824,12 +824,7 @@ class SkipLink extends GOVUKFrontendComponent {
SkipLink.moduleName = "govuk-skip-link";
class Tabs extends GOVUKFrontendComponent {
constructor(t) {
- if (super(), this.$module = void 0, this.$tabs = void 0, this.$tabList = void 0, this.$tabListItems = void 0, this.keys = {
- left: 37,
- right: 39,
- up: 38,
- down: 40
- }, this.jsHiddenClass = "govuk-tabs__panel--hidden", this.changingHash = !1, this.boundTabClick = void 0, this.boundTabKeydown = void 0, this.boundOnHashChange = void 0, this.mql = null, !t) throw new ElementError({
+ if (super(), this.$module = void 0, this.$tabs = void 0, this.$tabList = void 0, this.$tabListItems = void 0, this.jsHiddenClass = "govuk-tabs__panel--hidden", this.changingHash = !1, this.boundTabClick = void 0, this.boundTabKeydown = void 0, this.boundOnHashChange = void 0, this.mql = null, !t) throw new ElementError({
componentName: "Tabs",
element: t,
identifier: "Root element (`$module`)"
@@ -922,13 +917,17 @@ class Tabs extends GOVUKFrontendComponent {
e.id = "", this.changingHash = !0, window.location.hash = n, e.id = n
}
onTabKeydown(t) {
- switch (t.keyCode) {
- case this.keys.left:
- case this.keys.up:
+ switch (t.key) {
+ case "ArrowLeft":
+ case "ArrowUp":
+ case "Left":
+ case "Up":
this.activatePreviousTab(), t.preventDefault();
break;
- case this.keys.right:
- case this.keys.down:
+ case "ArrowRight":
+ case "ArrowDown":
+ case "Right":
+ case "Down":
this.activateNextTab(), t.preventDefault()
}
}
Action run for 964a453 |
Other changes to npm packagediff --git a/packages/govuk-frontend/dist/govuk/all.bundle.js b/packages/govuk-frontend/dist/govuk/all.bundle.js
index 595d5851f..514baf6ba 100644
--- a/packages/govuk-frontend/dist/govuk/all.bundle.js
+++ b/packages/govuk-frontend/dist/govuk/all.bundle.js
@@ -1812,12 +1812,6 @@
this.$tabs = void 0;
this.$tabList = void 0;
this.$tabListItems = void 0;
- this.keys = {
- left: 37,
- right: 39,
- up: 38,
- down: 40
- };
this.jsHiddenClass = 'govuk-tabs__panel--hidden';
this.changingHash = false;
this.boundTabClick = void 0;
@@ -1997,14 +1991,18 @@
$panel.id = panelId;
}
onTabKeydown(event) {
- switch (event.keyCode) {
- case this.keys.left:
- case this.keys.up:
+ switch (event.key) {
+ case 'ArrowLeft':
+ case 'ArrowUp':
+ case 'Left':
+ case 'Up':
this.activatePreviousTab();
event.preventDefault();
break;
- case this.keys.right:
- case this.keys.down:
+ case 'ArrowRight':
+ case 'ArrowDown':
+ case 'Right':
+ case 'Down':
this.activateNextTab();
event.preventDefault();
break;
diff --git a/packages/govuk-frontend/dist/govuk/all.bundle.mjs b/packages/govuk-frontend/dist/govuk/all.bundle.mjs
index 7375fe834..c52909c71 100644
--- a/packages/govuk-frontend/dist/govuk/all.bundle.mjs
+++ b/packages/govuk-frontend/dist/govuk/all.bundle.mjs
@@ -1806,12 +1806,6 @@ class Tabs extends GOVUKFrontendComponent {
this.$tabs = void 0;
this.$tabList = void 0;
this.$tabListItems = void 0;
- this.keys = {
- left: 37,
- right: 39,
- up: 38,
- down: 40
- };
this.jsHiddenClass = 'govuk-tabs__panel--hidden';
this.changingHash = false;
this.boundTabClick = void 0;
@@ -1991,14 +1985,18 @@ class Tabs extends GOVUKFrontendComponent {
$panel.id = panelId;
}
onTabKeydown(event) {
- switch (event.keyCode) {
- case this.keys.left:
- case this.keys.up:
+ switch (event.key) {
+ case 'ArrowLeft':
+ case 'ArrowUp':
+ case 'Left':
+ case 'Up':
this.activatePreviousTab();
event.preventDefault();
break;
- case this.keys.right:
- case this.keys.down:
+ case 'ArrowRight':
+ case 'ArrowDown':
+ case 'Right':
+ case 'Down':
this.activateNextTab();
event.preventDefault();
break;
diff --git a/packages/govuk-frontend/dist/govuk/components/tabs/tabs.bundle.js b/packages/govuk-frontend/dist/govuk/components/tabs/tabs.bundle.js
index bc832d2f8..8b0bdb9c0 100644
--- a/packages/govuk-frontend/dist/govuk/components/tabs/tabs.bundle.js
+++ b/packages/govuk-frontend/dist/govuk/components/tabs/tabs.bundle.js
@@ -102,12 +102,6 @@
this.$tabs = void 0;
this.$tabList = void 0;
this.$tabListItems = void 0;
- this.keys = {
- left: 37,
- right: 39,
- up: 38,
- down: 40
- };
this.jsHiddenClass = 'govuk-tabs__panel--hidden';
this.changingHash = false;
this.boundTabClick = void 0;
@@ -287,14 +281,18 @@
$panel.id = panelId;
}
onTabKeydown(event) {
- switch (event.keyCode) {
- case this.keys.left:
- case this.keys.up:
+ switch (event.key) {
+ case 'ArrowLeft':
+ case 'ArrowUp':
+ case 'Left':
+ case 'Up':
this.activatePreviousTab();
event.preventDefault();
break;
- case this.keys.right:
- case this.keys.down:
+ case 'ArrowRight':
+ case 'ArrowDown':
+ case 'Right':
+ case 'Down':
this.activateNextTab();
event.preventDefault();
break;
diff --git a/packages/govuk-frontend/dist/govuk/components/tabs/tabs.bundle.mjs b/packages/govuk-frontend/dist/govuk/components/tabs/tabs.bundle.mjs
index ba301105f..db1378320 100644
--- a/packages/govuk-frontend/dist/govuk/components/tabs/tabs.bundle.mjs
+++ b/packages/govuk-frontend/dist/govuk/components/tabs/tabs.bundle.mjs
@@ -96,12 +96,6 @@ class Tabs extends GOVUKFrontendComponent {
this.$tabs = void 0;
this.$tabList = void 0;
this.$tabListItems = void 0;
- this.keys = {
- left: 37,
- right: 39,
- up: 38,
- down: 40
- };
this.jsHiddenClass = 'govuk-tabs__panel--hidden';
this.changingHash = false;
this.boundTabClick = void 0;
@@ -281,14 +275,18 @@ class Tabs extends GOVUKFrontendComponent {
$panel.id = panelId;
}
onTabKeydown(event) {
- switch (event.keyCode) {
- case this.keys.left:
- case this.keys.up:
+ switch (event.key) {
+ case 'ArrowLeft':
+ case 'ArrowUp':
+ case 'Left':
+ case 'Up':
this.activatePreviousTab();
event.preventDefault();
break;
- case this.keys.right:
- case this.keys.down:
+ case 'ArrowRight':
+ case 'ArrowDown':
+ case 'Right':
+ case 'Down':
this.activateNextTab();
event.preventDefault();
break;
diff --git a/packages/govuk-frontend/dist/govuk/components/tabs/tabs.mjs b/packages/govuk-frontend/dist/govuk/components/tabs/tabs.mjs
index f1c6e6ef2..39cd49970 100644
--- a/packages/govuk-frontend/dist/govuk/components/tabs/tabs.mjs
+++ b/packages/govuk-frontend/dist/govuk/components/tabs/tabs.mjs
@@ -17,12 +17,6 @@ class Tabs extends GOVUKFrontendComponent {
this.$tabs = void 0;
this.$tabList = void 0;
this.$tabListItems = void 0;
- this.keys = {
- left: 37,
- right: 39,
- up: 38,
- down: 40
- };
this.jsHiddenClass = 'govuk-tabs__panel--hidden';
this.changingHash = false;
this.boundTabClick = void 0;
@@ -202,14 +196,18 @@ class Tabs extends GOVUKFrontendComponent {
$panel.id = panelId;
}
onTabKeydown(event) {
- switch (event.keyCode) {
- case this.keys.left:
- case this.keys.up:
+ switch (event.key) {
+ case 'ArrowLeft':
+ case 'ArrowUp':
+ case 'Left':
+ case 'Up':
this.activatePreviousTab();
event.preventDefault();
break;
- case this.keys.right:
- case this.keys.down:
+ case 'ArrowRight':
+ case 'ArrowDown':
+ case 'Right':
+ case 'Down':
this.activateNextTab();
event.preventDefault();
break;
Action run for 964a453 |
case this.keys.left: | ||
case this.keys.up: | ||
switch (event.key) { | ||
// 'Left', 'Right', 'Up' and 'Down' required for Edge 16 support. |
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.
If we wanted to save a few bytes, we could ignore this and just go with the values in the spec. It shouldn't cause errors – it'd just mean that navigating between the tabs using the keyboard wouldn't work in Edge 16.
The number of users visiting GOV.UK (based on those who've opted in to analytics) are vanishingly small – between 20 to 50 sessions per day in February (out of 95,335,104 – so about 0.00004% of our sessions).
Thoughts?
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.
We discussed this in our dev catch up and decided that because the only way to move between tabs using the keyboard is using the arrow keys we should keep this in to properly support Edge 16.
key
over deprecated keyCode
in the Tabs componentKeyboardEvent.key
over deprecated KeyboardEvent.keyCode
in the Tabs component
KeyboardEvent.keyCode
is deprecated. All of the browsers that now run our JavaScript support the modernKeyboardEvent.key
property.Update the button component to using
KeyboardEvent.key
instead, removing hardcoded ASCII values in thekeys
map.Unfortunately Edge 16 uses e.g. “Left" instead of “ArrowLeft”, so we need to add those possible values as well if we want keyboard navigation for tabs to work in that browser (see the question I've left further down as to whether that's sensible or not).
Part of #4709