From d8cf5e5542b29dddc59fc0a8a02e40322699cd9f Mon Sep 17 00:00:00 2001 From: Alistair Laing Date: Fri, 17 May 2019 11:21:23 +0100 Subject: [PATCH 1/3] fixes tabs keyboard navigation bug in IE8 In IE8, the browser could not find the next/previous tab because it does not support `nextElementSibling` and `previousElementSibling` DOM traversal methods. To fix it I applied a polyfill for it. Once the browser could find the tab we then had to find the `firstChildElement` (i.e the anchor element) to add/edit the various data attributes to show/hide the tab panel. Again IE8 doesn't support it and instead of introducing another polyfill I used `querySelector instead to look up the "a". I assumed the first element would always be anchor for navigation purposes and also the nunjucks template uses an anchor with the class name that I'm looking up. --- src/components/tabs/tabs.js | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/src/components/tabs/tabs.js b/src/components/tabs/tabs.js index e3bb561f2f..3ef5ab2c55 100644 --- a/src/components/tabs/tabs.js +++ b/src/components/tabs/tabs.js @@ -204,10 +204,25 @@ Tabs.prototype.onTabKeydown = function (e) { } Tabs.prototype.activateNextTab = function () { + // IE doesn't support 'nextElementSibling' this code polyfills IE8 + // https://developer.mozilla.org/en-US/docs/Web/API/NonDocumentTypeChildNode/nextElementSibling#Polyfill_for_Internet_Explorer_8 + // Source: https://github.com/Alhadis/Snippets/blob/master/js/polyfills/IE8-child-elements.js + if (!('nextElementSibling' in document.documentElement)) { + Object.defineProperty(Element.prototype, 'nextElementSibling', { // eslint-disable-line no-undef + get: function () { + var e = this.nextSibling + while (e && e.nodeType !== 1) { + e = e.nextSibling + } + return e + } + }) + } + var currentTab = this.getCurrentTab() var nextTabListItem = currentTab.parentNode.nextElementSibling if (nextTabListItem) { - var nextTab = nextTabListItem.firstElementChild + var nextTab = nextTabListItem.querySelector('.govuk-tabs__tab') } if (nextTab) { this.hideTab(currentTab) @@ -218,10 +233,25 @@ Tabs.prototype.activateNextTab = function () { } Tabs.prototype.activatePreviousTab = function () { + // IE doesn't support 'previousElementSibling' this code polyfills IE8 + // https://developer.mozilla.org/en-US/docs/Web/API/NonDocumentTypeChildNode/previousElementSibling#Polyfill_for_Internet_Explorer_8 + // Source: https://github.com/Alhadis/Snippets/blob/master/js/polyfills/IE8-child-elements.js + if (!('previousElementSibling' in document.documentElement)) { + Object.defineProperty(Element.prototype, 'previousElementSibling', { // eslint-disable-line no-undef + get: function () { + var e = this.previousSibling + while (e && e.nodeType !== 1) { + e = e.previousSibling + } + return e + } + }) + } + var currentTab = this.getCurrentTab() var previousTabListItem = currentTab.parentNode.previousElementSibling if (previousTabListItem) { - var previousTab = previousTabListItem.firstElementChild + var previousTab = previousTabListItem.querySelector('.govuk-tabs__tab') } if (previousTab) { this.hideTab(currentTab) From 4997c72ff5d070ca193fd0021bd61f7d3ce199bc Mon Sep 17 00:00:00 2001 From: Alistair Laing Date: Fri, 17 May 2019 12:05:51 +0100 Subject: [PATCH 2/3] Add two polyfills for nextElementSibling and previousElementSibling I've added two new files polyfill files although they didn't come directly from polyfill.io they were based off a PR that was merged in the library but not included in the new polyfill-library repo. I've added comments pointing to the original pull request for the detection and for the polyfill. --- src/components/tabs/tabs.js | 32 ++----------------- .../Element/prototype/nextElementSibling.js | 27 ++++++++++++++++ .../prototype/previousElementSibling.js | 25 +++++++++++++++ 3 files changed, 54 insertions(+), 30 deletions(-) create mode 100644 src/vendor/polyfills/Element/prototype/nextElementSibling.js create mode 100644 src/vendor/polyfills/Element/prototype/previousElementSibling.js diff --git a/src/components/tabs/tabs.js b/src/components/tabs/tabs.js index 3ef5ab2c55..4ea8801b74 100644 --- a/src/components/tabs/tabs.js +++ b/src/components/tabs/tabs.js @@ -1,5 +1,7 @@ import '../../vendor/polyfills/Function/prototype/bind' import '../../vendor/polyfills/Element/prototype/classList' +import '../../vendor/polyfills/Element/prototype/nextElementSibling' +import '../../vendor/polyfills/Element/prototype/previousElementSibling' import '../../vendor/polyfills/Event' // addEventListener and event.target normaliziation import { nodeListForEach } from '../../common' @@ -204,21 +206,6 @@ Tabs.prototype.onTabKeydown = function (e) { } Tabs.prototype.activateNextTab = function () { - // IE doesn't support 'nextElementSibling' this code polyfills IE8 - // https://developer.mozilla.org/en-US/docs/Web/API/NonDocumentTypeChildNode/nextElementSibling#Polyfill_for_Internet_Explorer_8 - // Source: https://github.com/Alhadis/Snippets/blob/master/js/polyfills/IE8-child-elements.js - if (!('nextElementSibling' in document.documentElement)) { - Object.defineProperty(Element.prototype, 'nextElementSibling', { // eslint-disable-line no-undef - get: function () { - var e = this.nextSibling - while (e && e.nodeType !== 1) { - e = e.nextSibling - } - return e - } - }) - } - var currentTab = this.getCurrentTab() var nextTabListItem = currentTab.parentNode.nextElementSibling if (nextTabListItem) { @@ -233,21 +220,6 @@ Tabs.prototype.activateNextTab = function () { } Tabs.prototype.activatePreviousTab = function () { - // IE doesn't support 'previousElementSibling' this code polyfills IE8 - // https://developer.mozilla.org/en-US/docs/Web/API/NonDocumentTypeChildNode/previousElementSibling#Polyfill_for_Internet_Explorer_8 - // Source: https://github.com/Alhadis/Snippets/blob/master/js/polyfills/IE8-child-elements.js - if (!('previousElementSibling' in document.documentElement)) { - Object.defineProperty(Element.prototype, 'previousElementSibling', { // eslint-disable-line no-undef - get: function () { - var e = this.previousSibling - while (e && e.nodeType !== 1) { - e = e.previousSibling - } - return e - } - }) - } - var currentTab = this.getCurrentTab() var previousTabListItem = currentTab.parentNode.previousElementSibling if (previousTabListItem) { diff --git a/src/vendor/polyfills/Element/prototype/nextElementSibling.js b/src/vendor/polyfills/Element/prototype/nextElementSibling.js new file mode 100644 index 0000000000..ec3c615496 --- /dev/null +++ b/src/vendor/polyfills/Element/prototype/nextElementSibling.js @@ -0,0 +1,27 @@ +import '../../Object/defineProperty' +import '../../Element' + +(function(undefined) { + + // Detection from https://github.com/Financial-Times/polyfill-service/pull/1062/files#diff-b09a5d2acf3314b46a6c8f8d0c31b85c + var detect = ( + 'Element' in this && "nextElementSibling" in document.documentElement + ) + + if (detect) return + + + (function (global) { + + // Polyfill from https://github.com/Financial-Times/polyfill-service/pull/1062/files#diff-404b69b4750d18dea4174930a49170fd + Object.defineProperty(Element.prototype, "nextElementSibling", { + get: function(){ + var el = this.nextSibling; + while (el && el.nodeType !== 1) { el = el.nextSibling; } + return (el.nodeType === 1) ? el : null; + } + }); + + }(this)); + +}).call('object' === typeof window && window || 'object' === typeof self && self || 'object' === typeof global && global || {}); diff --git a/src/vendor/polyfills/Element/prototype/previousElementSibling.js b/src/vendor/polyfills/Element/prototype/previousElementSibling.js new file mode 100644 index 0000000000..1184a0a3e3 --- /dev/null +++ b/src/vendor/polyfills/Element/prototype/previousElementSibling.js @@ -0,0 +1,25 @@ +import '../../Object/defineProperty' +import '../../Element' + +(function(undefined) { + + // Detection from https://github.com/Financial-Times/polyfill-service/pull/1062/files#diff-a162235fbc9c0dd40d4032265f44942e + var detect = ( + 'Element' in this && 'previousElementSibling' in document.documentElement + ) + + if (detect) return + + (function (global) { + // Polyfill from https://github.com/Financial-Times/polyfill-service/pull/1062/files#diff-b45a1197b842728cb76b624b6ba7d739 + Object.defineProperty(Element.prototype, 'previousElementSibling', { + get: function(){ + var el = this.previousSibling; + while (el && el.nodeType !== 1) { el = el.previousSibling; } + return (el.nodeType === 1) ? el : null; + } + }); + + }(this)); + +}).call('object' === typeof window && window || 'object' === typeof self && self || 'object' === typeof global && global || {}); From 9e1c25fd0b5a035b831ccc4fdae95bfc06eecb55 Mon Sep 17 00:00:00 2001 From: Alistair Laing Date: Fri, 17 May 2019 11:46:04 +0100 Subject: [PATCH 3/3] adds CHANGELOG entry --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 94c9a828da..84e8ab2bba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -194,6 +194,13 @@ 🔧 Fixes: +- fixes tabs keyboard navigation bug in IE8 + + Users were unable to tab between tab panels using the keyboard and had to + use their mouse to toggle between panels. + + ([PR #1359](https://github.com/alphagov/govuk-frontend/pull/1359)) + - Update accordion focus styles to remove firefox outlines ([PR #1324](https://github.com/alphagov/govuk-frontend/pull/1324))