From 2e5a01090136c5851e5b3ec2d5c64862c7bced29 Mon Sep 17 00:00:00 2001 From: patrickhlauke Date: Mon, 17 Jun 2019 23:45:07 +0100 Subject: [PATCH 01/13] Add initial tabindex handling for tabs set `tabindex` as well as `aria-selected` on tabs --- js/src/tab.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/js/src/tab.js b/js/src/tab.js index b9db64baa..65dc3602b 100644 --- a/js/src/tab.js +++ b/js/src/tab.js @@ -183,12 +183,14 @@ class Tab { if (active.getAttribute('role') === 'tab') { active.setAttribute('aria-selected', false) + active.setAttribute('tabindex', '-1') } } element.classList.add(ClassName.ACTIVE) if (element.getAttribute('role') === 'tab') { element.setAttribute('aria-selected', true) + element.setAttribute('tabindex', '0') } reflow(element) From da9f93da1c2d00b42a7bbf09c7d6cd436af6e37b Mon Sep 17 00:00:00 2001 From: patrickhlauke Date: Tue, 18 Jun 2019 00:02:45 +0100 Subject: [PATCH 02/13] WIP add initial keyboard handling and initialisation - initialisation happens very inelegantly - could do with some advice on how to move this to a function (should it be public, private, static?) - keyboard handling works, but seems to get confused when switching too quickly between tabs sometimes - activating tabs currently done via click() - there must be a nicer way to do this...how? fire _activate? which parameters to pass? --- js/src/tab.js | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/js/src/tab.js b/js/src/tab.js index 65dc3602b..dc0acec2a 100644 --- a/js/src/tab.js +++ b/js/src/tab.js @@ -29,13 +29,19 @@ const VERSION = '4.3.1' const DATA_KEY = 'bs.tab' const EVENT_KEY = `.${DATA_KEY}` const DATA_API_KEY = '.data-api' +const ARROW_LEFT_KEYCODE = 37 // KeyboardEvent.which value for left arrow key +const ARROW_UP_KEYCODE = 38 // KeyboardEvent.which value for up arrow key +const ARROW_RIGHT_KEYCODE = 39 // KeyboardEvent.which value for right arrow key +const ARROW_DOWN_KEYCODE = 40 // KeyboardEvent.which value for down arrow key const Event = { HIDE: `hide${EVENT_KEY}`, HIDDEN: `hidden${EVENT_KEY}`, SHOW: `show${EVENT_KEY}`, SHOWN: `shown${EVENT_KEY}`, - CLICK_DATA_API: `click${EVENT_KEY}${DATA_API_KEY}` + CLICK_DATA_API: `click${EVENT_KEY}${DATA_API_KEY}`, + KEYDOWN_DATA_API: `keydown${EVENT_KEY}${DATA_API_KEY}`, + LOAD_DATA_API: `load${EVENT_KEY}${DATA_API_KEY}` } const ClassName = { @@ -52,6 +58,7 @@ const Selector = { ACTIVE: '.active', ACTIVE_UL: ':scope > li > .active', DATA_TOGGLE: '[data-toggle="tab"], [data-toggle="pill"], [data-toggle="list"]', + TABLIST: '[role="tablist"]', DROPDOWN_TOGGLE: '.dropdown-toggle', DROPDOWN_ACTIVE_CHILD: ':scope > .dropdown-menu .active' } @@ -231,6 +238,48 @@ class Tab { }) } + static _dataApiKeydownHandler(event) { + const tablist = SelectorEngine.closest(event.target, Selector.TABLIST) + let tablistorientation = tablist.getAttribute('aria-orientation') + if (tablistorientation !== 'vertical') { + tablistorientation = 'horizontal' + } + + if ((tablistorientation === 'horizontal' && event.which !== ARROW_LEFT_KEYCODE && event.which !== ARROW_RIGHT_KEYCODE) || (tablistorientation === 'vertical' && event.which !== ARROW_UP_KEYCODE && event.which !== ARROW_DOWN_KEYCODE)) { + return + } + + event.preventDefault() + event.stopPropagation() + + if (this.disabled || this.classList.contains(ClassName.DISABLED)) { + return + } + + const tabs = makeArray(SelectorEngine.find(Selector.DATA_TOGGLE, tablist)) + + if (!tabs.length) { + return + } + + let index = tabs.indexOf(event.target) + + if ((event.which === ARROW_LEFT_KEYCODE || event.which === ARROW_UP_KEYCODE) && index > 0) { // Left / Up + index-- + } + + if ((event.which === ARROW_RIGHT_KEYCODE || event.which === ARROW_DOWN_KEYCODE) && index < tabs.length - 1) { // Right / Down + index++ + } + + if (index < 0) { + index = 0 + } + + tabs[index].focus() + tabs[index].click() // WIP naive way of doing this? any better way? calling _activate or something? + } + static _getInstance(element) { return Data.getData(element, DATA_KEY) } @@ -242,6 +291,36 @@ class Tab { * ------------------------------------------------------------------------ */ +EventHandler.on(window, Event.LOAD_DATA_API, () => { + const tablists = makeArray(SelectorEngine.find(Selector.TABLIST)) + if (tablists.length === 0) { + return + } + + // iterate over all found sets of tab lists + for (let i = 0; i < tablists.length; i++) { + const tabs = makeArray(SelectorEngine.find(Selector.DATA_TOGGLE, tablists[i])) + let selectedTabFound = false + + // iterate over each tab in the tablist, make sure they have correct tabindex/aria-selected + for (let j = 0; j < tabs.length; j++) { + if (tabs[j].hasAttribute('aria-selected') && tabs[j].getAttribute('aria-selected') === 'true' && selectedTabFound === false) { + tabs[j].setAttribute('tabindex', '0') + selectedTabFound = true + } else { + tabs[j].setAttribute('tabindex', '-1') + tabs[j].setAttribute('aria-selected', 'false') + } + } + + // if none of the tabs were explicitly marked as selected, pick first one + if (selectedTabFound === false) { + tabs[0].setAttribute('tabindex', '0') + tabs[0].setAttribute('aria-selected', 'true') + } + } +}) +EventHandler.on(document, Event.KEYDOWN_DATA_API, Selector.DATA_TOGGLE, Tab._dataApiKeydownHandler) EventHandler.on(document, Event.CLICK_DATA_API, Selector.DATA_TOGGLE, function (event) { event.preventDefault() From a1e7738632d6ccf7abd096e229d7568b5ae593e5 Mon Sep 17 00:00:00 2001 From: patrickhlauke Date: Tue, 18 Jun 2019 20:19:53 +0100 Subject: [PATCH 03/13] Remove dropdown-in-tabs related code - there's no ARIA pattern that would allow dropdowns in tablists; it cannot be expressed accessibly to AT users. it also has serious usability drawbacks. we're already saying in the docs it should not be used ... this goes a step further and removes the handling for it --- js/src/tab.js | 23 +---------------------- 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/js/src/tab.js b/js/src/tab.js index dc0acec2a..bde07818c 100644 --- a/js/src/tab.js +++ b/js/src/tab.js @@ -45,7 +45,6 @@ const Event = { } const ClassName = { - DROPDOWN_MENU: 'dropdown-menu', ACTIVE: 'active', DISABLED: 'disabled', FADE: 'fade', @@ -53,14 +52,11 @@ const ClassName = { } const Selector = { - DROPDOWN: '.dropdown', NAV_LIST_GROUP: '.nav, .list-group', ACTIVE: '.active', ACTIVE_UL: ':scope > li > .active', DATA_TOGGLE: '[data-toggle="tab"], [data-toggle="pill"], [data-toggle="list"]', - TABLIST: '[role="tablist"]', - DROPDOWN_TOGGLE: '.dropdown-toggle', - DROPDOWN_ACTIVE_CHILD: ':scope > .dropdown-menu .active' + TABLIST: '[role="tablist"]' } /** @@ -182,12 +178,6 @@ class Tab { if (active) { active.classList.remove(ClassName.ACTIVE) - const dropdownChild = SelectorEngine.findOne(Selector.DROPDOWN_ACTIVE_CHILD, active.parentNode) - - if (dropdownChild) { - dropdownChild.classList.remove(ClassName.ACTIVE) - } - if (active.getAttribute('role') === 'tab') { active.setAttribute('aria-selected', false) active.setAttribute('tabindex', '-1') @@ -206,17 +196,6 @@ class Tab { element.classList.add(ClassName.SHOW) } - if (element.parentNode && element.parentNode.classList.contains(ClassName.DROPDOWN_MENU)) { - const dropdownElement = SelectorEngine.closest(element, Selector.DROPDOWN) - - if (dropdownElement) { - makeArray(SelectorEngine.find(Selector.DROPDOWN_TOGGLE)) - .forEach(dropdown => dropdown.classList.add(ClassName.ACTIVE)) - } - - element.setAttribute('aria-expanded', true) - } - if (callback) { callback() } From 31dd87c600275d7166666c11b2132acf42101bff Mon Sep 17 00:00:00 2001 From: patrickhlauke Date: Tue, 18 Jun 2019 20:32:52 +0100 Subject: [PATCH 04/13] Add explicit note about aria-orientation, add all missing role="presentation" `role="presentation"` is needed as otherwise AT (like NVDA) will get confused/won't be able to "count" the total number of tabs properly --- site/content/docs/4.3/components/navs.md | 44 ++++++++++++------------ 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/site/content/docs/4.3/components/navs.md b/site/content/docs/4.3/components/navs.md index 5281fcdaf..f3df6d240 100644 --- a/site/content/docs/4.3/components/navs.md +++ b/site/content/docs/4.3/components/navs.md @@ -307,17 +307,17 @@ Use the tab JavaScript plugin—include it individually or through the compiled Dynamic tabbed interfaces, as described in the [WAI ARIA Authoring Practices](https://www.w3.org/TR/wai-aria-practices/#tabpanel), require `role="tablist"`, `role="tab"`, `role="tabpanel"`, and additional `aria-` attributes in order to convey their structure, functionality and current state to users of assistive technologies (such as screen readers). -Note that dynamic tabbed interfaces should not contain dropdown menus, as this causes both usability and accessibility issues. From a usability perspective, the fact that the currently displayed tab's trigger element is not immediately visible (as it's inside the closed dropdown menu) can cause confusion. From an accessibility point of view, there is currently no sensible way to map this sort of construct to a standard WAI ARIA pattern, meaning that it cannot be easily made understandable to users of assistive technologies. +Note that dynamic tabbed interfaces can't contain dropdown menus, as this causes both usability and accessibility issues. From a usability perspective, the fact that the currently displayed tab's trigger element is not immediately visible (as it's inside the closed dropdown menu) can cause confusion. From an accessibility point of view, there is currently no sensible way to map this sort of construct to a standard WAI ARIA pattern, meaning that it cannot be easily made understandable to users of assistive technologies.
@@ -336,13 +336,13 @@ Note that dynamic tabbed interfaces should not contain dropdown menus, {{< highlight html >}} @@ -395,13 +395,13 @@ The tabs plugin also works with pills.
@@ -420,13 +420,13 @@ The tabs plugin also works with pills. {{< highlight html >}} @@ -437,7 +437,7 @@ The tabs plugin also works with pills.
{{< /highlight >}} -And with vertical pills. +When making vertical tab panels, make sure to include `aria-orientation="vertical"` to switch to the appropriate keyboard behavior (switching tabs with the up and down cursor keys).
@@ -496,16 +496,16 @@ You can activate a tab or pill navigation without writing any JavaScript by simp {{< highlight html >}} @@ -564,16 +564,16 @@ Activates a tab element and content container. Tab should have either a `data-ta {{< highlight html >}} From a8cf124b82ba395c2ecabf9a557fd620f85dec52 Mon Sep 17 00:00:00 2001 From: patrickhlauke Date: Wed, 19 Jun 2019 10:18:35 +0100 Subject: [PATCH 05/13] Initial code tweaks based on comments --- js/src/tab.js | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/js/src/tab.js b/js/src/tab.js index bde07818c..0d1dc6656 100644 --- a/js/src/tab.js +++ b/js/src/tab.js @@ -59,6 +59,11 @@ const Selector = { TABLIST: '[role="tablist"]' } +const Orientation = { + VERTICAL: 'vertical', + HORIZONTAL: 'horizontal' +} + /** * ------------------------------------------------------------------------ * Class Definition @@ -219,12 +224,12 @@ class Tab { static _dataApiKeydownHandler(event) { const tablist = SelectorEngine.closest(event.target, Selector.TABLIST) - let tablistorientation = tablist.getAttribute('aria-orientation') - if (tablistorientation !== 'vertical') { - tablistorientation = 'horizontal' + let tabListOrientation = tablist.getAttribute('aria-orientation') + if (tabListOrientation !== Orientation.VERTICAL) { + tabListOrientation = Orientation.HORIZONTAL } - if ((tablistorientation === 'horizontal' && event.which !== ARROW_LEFT_KEYCODE && event.which !== ARROW_RIGHT_KEYCODE) || (tablistorientation === 'vertical' && event.which !== ARROW_UP_KEYCODE && event.which !== ARROW_DOWN_KEYCODE)) { + if ((tabListOrientation === Orientation.HORIZONTAL && event.which !== ARROW_LEFT_KEYCODE && event.which !== ARROW_RIGHT_KEYCODE) || (tabListOrientation === Orientation.VERTICAL && event.which !== ARROW_UP_KEYCODE && event.which !== ARROW_DOWN_KEYCODE)) { return } @@ -272,7 +277,7 @@ class Tab { EventHandler.on(window, Event.LOAD_DATA_API, () => { const tablists = makeArray(SelectorEngine.find(Selector.TABLIST)) - if (tablists.length === 0) { + if (!tablists.length) { return } @@ -283,7 +288,7 @@ EventHandler.on(window, Event.LOAD_DATA_API, () => { // iterate over each tab in the tablist, make sure they have correct tabindex/aria-selected for (let j = 0; j < tabs.length; j++) { - if (tabs[j].hasAttribute('aria-selected') && tabs[j].getAttribute('aria-selected') === 'true' && selectedTabFound === false) { + if (tabs[j].getAttribute('aria-selected') === 'true' && !selectedTabFound) { tabs[j].setAttribute('tabindex', '0') selectedTabFound = true } else { @@ -293,7 +298,7 @@ EventHandler.on(window, Event.LOAD_DATA_API, () => { } // if none of the tabs were explicitly marked as selected, pick first one - if (selectedTabFound === false) { + if (!selectedTabFound) { tabs[0].setAttribute('tabindex', '0') tabs[0].setAttribute('aria-selected', 'true') } From 35179a4aeeaacba120041cf8802436a4f02dc25e Mon Sep 17 00:00:00 2001 From: patrickhlauke Date: Wed, 19 Jun 2019 10:48:56 +0100 Subject: [PATCH 06/13] Remove dropdown-in-tabs related unit tests Since that logic has now been expunged --- js/tests/unit/tab.js | 73 +++++++++++++++----------------------------- 1 file changed, 24 insertions(+), 49 deletions(-) diff --git a/js/tests/unit/tab.js b/js/tests/unit/tab.js index 827fb707c..628c2008a 100644 --- a/js/tests/unit/tab.js +++ b/js/tests/unit/tab.js @@ -180,35 +180,6 @@ $(function () { .bootstrapTab('show') }) - QUnit.test('show and shown events should reference correct relatedTarget', function (assert) { - assert.expect(2) - var done = assert.async() - - var dropHTML = - '
    ' + - '
  • 1' + - '
      ' + - '
    • 1-1
      • ' + - '
    • 1-2
      • ' + - '
    ' + - '
    • ' + - '
' - - $(dropHTML) - .find('ul > li:first-child a') - .bootstrapTab('show') - .end() - .find('ul > li:last-child a') - .on('show.bs.tab', function (e) { - assert.strictEqual(e.relatedTarget.hash, '#a1-1', 'references correct element as relatedTarget') - }) - .on('shown.bs.tab', function (e) { - assert.strictEqual(e.relatedTarget.hash, '#a1-1', 'references correct element as relatedTarget') - done() - }) - .bootstrapTab('show') - }) - QUnit.test('should fire hide and hidden events', function (assert) { assert.expect(2) var done = assert.async() @@ -265,6 +236,30 @@ $(function () { .bootstrapTab('show') }) + QUnit.test('show and shown events should reference correct relatedTarget', function (assert) { + assert.expect(2) + var done = assert.async() + + var tabsHTML = '' + + $(tabsHTML) + .find('li:first-child a') + .bootstrapTab('show') + .end() + .find('li:last-child a') + .on('show.bs.tab', function (e) { + assert.strictEqual(e.relatedTarget.hash, '#home', 'references correct element as relatedTarget') + }) + .on('shown.bs.tab', function (e) { + assert.strictEqual(e.relatedTarget.hash, '#home', 'references correct element as relatedTarget') + done() + }) + .bootstrapTab('show') + }) + QUnit.test('hide and hidden events contain correct relatedTarget', function (assert) { assert.expect(2) var done = assert.async() @@ -327,26 +322,6 @@ $(function () { assert.ok($tabs.find('li:last-child a').hasClass('active')) }) - QUnit.test('selected tab should deactivate previous selected link in dropdown', function (assert) { - assert.expect(3) - var tabsHTML = '' - var $tabs = $(tabsHTML).appendTo('#qunit-fixture') - - $tabs.find('li:first-child a')[0].click() - assert.ok($tabs.find('li:first-child a').hasClass('active')) - assert.notOk($tabs.find('li:last-child a').hasClass('active')) - assert.notOk($tabs.find('li:last-child .dropdown-menu a:first-child').hasClass('active')) - }) - QUnit.test('Nested tabs', function (assert) { assert.expect(2) var done = assert.async() From 66d0d7b05e2748cedbf6f3bb5b6dca28d9457263 Mon Sep 17 00:00:00 2001 From: patrickhlauke Date: Wed, 19 Jun 2019 10:54:19 +0100 Subject: [PATCH 07/13] Make note about dropdowns in tabs an actual callout --- site/content/docs/4.3/components/navs.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/site/content/docs/4.3/components/navs.md b/site/content/docs/4.3/components/navs.md index f3df6d240..edea7b0aa 100644 --- a/site/content/docs/4.3/components/navs.md +++ b/site/content/docs/4.3/components/navs.md @@ -307,7 +307,9 @@ Use the tab JavaScript plugin—include it individually or through the compiled Dynamic tabbed interfaces, as described in the [WAI ARIA Authoring Practices](https://www.w3.org/TR/wai-aria-practices/#tabpanel), require `role="tablist"`, `role="tab"`, `role="tabpanel"`, and additional `aria-` attributes in order to convey their structure, functionality and current state to users of assistive technologies (such as screen readers). -Note that dynamic tabbed interfaces can't contain dropdown menus, as this causes both usability and accessibility issues. From a usability perspective, the fact that the currently displayed tab's trigger element is not immediately visible (as it's inside the closed dropdown menu) can cause confusion. From an accessibility point of view, there is currently no sensible way to map this sort of construct to a standard WAI ARIA pattern, meaning that it cannot be easily made understandable to users of assistive technologies. +{{< callout warning >}} +Dynamic tabbed interfaces can't contain dropdown menus, as this causes both usability and accessibility issues. From a usability perspective, the fact that the currently displayed tab's trigger element is not immediately visible (as it's inside the closed dropdown menu) can cause confusion. From an accessibility point of view, there is currently no sensible way to map this sort of construct to a standard WAI ARIA pattern, meaning that it cannot be easily made understandable to users of assistive technologies. +{{< /callout >}}