From 5d9500bdfdd02b3b1d91df2be86b1723f517fc52 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Julien=20D=C3=A9ramond?= Date: Fri, 6 May 2022 04:26:15 +0200 Subject: [PATCH] Handle disabled focused tabs with tab JavaScript plugin (#36169) * Handle disabled tabs * Fix after feedback * Update js/src/tab.js Co-authored-by: GeoSot * Update js/src/tab.js Co-authored-by: GeoSot * Commit suggestions via GitHub broke the thing * Add some unit tests * Remove temp doc modification * Add tests for left arrow * Add disabled tabs in JavaScript Behavior section * Compact 4 tests to 2 tests * Compact 4 tests to 2 tests * Add 'disabled' attribute for all buttons * Change the disabled pane position only for the vertical version * Change ids for the confusing first example in JavaScript behavior * Use disabled attribute instead of the class for buttons in tabs Co-authored-by: GeoSot --- js/src/tab.js | 7 +- js/tests/unit/tab.spec.js | 66 +++++++++++++++++++ scss/_nav.scss | 9 ++- site/content/docs/5.1/components/navs-tabs.md | 64 +++++++++++++----- 4 files changed, 127 insertions(+), 19 deletions(-) diff --git a/js/src/tab.js b/js/src/tab.js index 135e929dda..3fa5e4c9e9 100644 --- a/js/src/tab.js +++ b/js/src/tab.js @@ -168,8 +168,11 @@ class Tab extends BaseComponent { event.stopPropagation()// stopPropagation/preventDefault both added to support up/down keys without scrolling the page event.preventDefault() const isNext = [ARROW_RIGHT_KEY, ARROW_DOWN_KEY].includes(event.key) - const nextActiveElement = getNextActiveElement(this._getChildren(), event.target, isNext, true) - Tab.getOrCreateInstance(nextActiveElement).show() + const nextActiveElement = getNextActiveElement(this._getChildren().filter(element => !isDisabled(element)), event.target, isNext, true) + + if (nextActiveElement) { + Tab.getOrCreateInstance(nextActiveElement).show() + } } _getChildren() { // collection of inner elements diff --git a/js/tests/unit/tab.spec.js b/js/tests/unit/tab.spec.js index 4b8fb62de6..e6225b23db 100644 --- a/js/tests/unit/tab.spec.js +++ b/js/tests/unit/tab.spec.js @@ -548,6 +548,72 @@ describe('Tab', () => { expect(Event.prototype.stopPropagation).toHaveBeenCalledTimes(2) expect(Event.prototype.preventDefault).toHaveBeenCalledTimes(2) }) + + it('if keydown event is right arrow and next element is disabled', () => { + fixtureEl.innerHTML = [ + '' + ].join('') + + const tabEl = fixtureEl.querySelector('#tab1') + const tabEl2 = fixtureEl.querySelector('#tab2') + const tabEl3 = fixtureEl.querySelector('#tab3') + const tabEl4 = fixtureEl.querySelector('#tab4') + const tab = new Tab(tabEl) + const tab2 = new Tab(tabEl2) + const tab3 = new Tab(tabEl3) + const tab4 = new Tab(tabEl4) + spyOn(tab, 'show').and.callThrough() + spyOn(tab2, 'show').and.callThrough() + spyOn(tab3, 'show').and.callThrough() + spyOn(tab4, 'show').and.callThrough() + + const keydown = createEvent('keydown') + keydown.key = 'ArrowRight' + + tabEl.dispatchEvent(keydown) + expect(tab.show).not.toHaveBeenCalled() + expect(tab2.show).not.toHaveBeenCalled() + expect(tab3.show).not.toHaveBeenCalled() + expect(tab4.show).toHaveBeenCalledTimes(1) + }) + + it('if keydown event is left arrow and next element is disabled', () => { + fixtureEl.innerHTML = [ + '' + ].join('') + + const tabEl = fixtureEl.querySelector('#tab1') + const tabEl2 = fixtureEl.querySelector('#tab2') + const tabEl3 = fixtureEl.querySelector('#tab3') + const tabEl4 = fixtureEl.querySelector('#tab4') + const tab = new Tab(tabEl) + const tab2 = new Tab(tabEl2) + const tab3 = new Tab(tabEl3) + const tab4 = new Tab(tabEl4) + spyOn(tab, 'show').and.callThrough() + spyOn(tab2, 'show').and.callThrough() + spyOn(tab3, 'show').and.callThrough() + spyOn(tab4, 'show').and.callThrough() + + const keydown = createEvent('keydown') + keydown.key = 'ArrowLeft' + + tabEl4.dispatchEvent(keydown) + expect(tab4.show).not.toHaveBeenCalled() + expect(tab3.show).not.toHaveBeenCalled() + expect(tab2.show).not.toHaveBeenCalled() + expect(tab.show).toHaveBeenCalledTimes(1) + }) }) describe('jQueryInterface', () => { diff --git a/scss/_nav.scss b/scss/_nav.scss index 21ee1b2b44..aa1468e944 100644 --- a/scss/_nav.scss +++ b/scss/_nav.scss @@ -74,7 +74,8 @@ border-color: var(--#{$prefix}nav-tabs-link-hover-border-color); } - &.disabled { + &.disabled, + &:disabled { color: var(--#{$prefix}nav-link-disabled-color); background-color: transparent; border-color: transparent; @@ -112,6 +113,12 @@ background: none; border: 0; @include border-radius(var(--#{$prefix}nav-pills-border-radius)); + + &:disabled { + color: var(--#{$prefix}nav-link-disabled-color); + background-color: transparent; + border-color: transparent; + } } .nav-link.active, diff --git a/site/content/docs/5.1/components/navs-tabs.md b/site/content/docs/5.1/components/navs-tabs.md index d43058b33e..b3d3067503 100644 --- a/site/content/docs/5.1/components/navs-tabs.md +++ b/site/content/docs/5.1/components/navs-tabs.md @@ -335,44 +335,54 @@ Use the tab JavaScript plugin—include it individually or through the compiled
-
+

This is some placeholder content the Home tab's associated content. Clicking another tab will toggle the visibility of this one for the next. The tab JavaScript swaps classes to control the content visibility and styling. You can use it with tabs, pills, and any other .nav-powered navigation.

-
+

This is some placeholder content the Profile tab's associated content. Clicking another tab will toggle the visibility of this one for the next. The tab JavaScript swaps classes to control the content visibility and styling. You can use it with tabs, pills, and any other .nav-powered navigation.

-
+

This is some placeholder content the Contact tab's associated content. Clicking another tab will toggle the visibility of this one for the next. The tab JavaScript swaps classes to control the content visibility and styling. You can use it with tabs, pills, and any other .nav-powered navigation.

+
+

This is some placeholder content the Disabled tab's associated content.

+
```html
-
...
-
...
-
...
+
...
+
...
+
...
+
...
``` @@ -384,6 +394,7 @@ To help fit your needs, this works with `
    `-based markup, as shown above, or +
@@ -405,12 +419,14 @@ To help fit your needs, this works with `
    `-based markup, as shown above, or +
``` @@ -427,6 +443,9 @@ The tabs plugin also works with pills. +
@@ -438,6 +457,9 @@ The tabs plugin also works with pills.

This is some placeholder content the Contact tab's associated content. Clicking another tab will toggle the visibility of this one for the next. The tab JavaScript swaps classes to control the content visibility and styling. You can use it with tabs, pills, and any other .nav-powered navigation.

+
+

This is some placeholder content the Disabled tab's associated content.

+
@@ -452,11 +474,15 @@ The tabs plugin also works with pills. +
...
...
...
+
...
``` @@ -467,21 +493,25 @@ And with vertical pills. Ideally, for vertical tabs, you should also add `aria-o
-

This is some placeholder content the Home tab's associated content. Clicking another tab will toggle the visibility of this one for the next. The tab JavaScript swaps classes to control the content visibility and styling. You can use it with tabs, pills, and any other .nav-powered navigation.

+

This is some placeholder content the Home tab's associated content. Clicking another tab will toggle the visibility of this one for the next. The tab JavaScript swaps classes to control the content visibility and styling. You can use it with tabs, pills, and any other .nav-powered navigation.

-

This is some placeholder content the Profile tab's associated content. Clicking another tab will toggle the visibility of this one for the next. The tab JavaScript swaps classes to control the content visibility and styling. You can use it with tabs, pills, and any other .nav-powered navigation.

+

This is some placeholder content the Profile tab's associated content. Clicking another tab will toggle the visibility of this one for the next. The tab JavaScript swaps classes to control the content visibility and styling. You can use it with tabs, pills, and any other .nav-powered navigation.

+
+
+

This is some placeholder content the Disabled tab's associated content.

-

This is some placeholder content the Messages tab's associated content. Clicking another tab will toggle the visibility of this one for the next. The tab JavaScript swaps classes to control the content visibility and styling. You can use it with tabs, pills, and any other .nav-powered navigation.

+

This is some placeholder content the Messages tab's associated content. Clicking another tab will toggle the visibility of this one for the next. The tab JavaScript swaps classes to control the content visibility and styling. You can use it with tabs, pills, and any other .nav-powered navigation.

-

This is some placeholder content the Settings tab's associated content. Clicking another tab will toggle the visibility of this one for the next. The tab JavaScript swaps classes to control the content visibility and styling. You can use it with tabs, pills, and any other .nav-powered navigation.

+

This is some placeholder content the Settings tab's associated content. Clicking another tab will toggle the visibility of this one for the next. The tab JavaScript swaps classes to control the content visibility and styling. You can use it with tabs, pills, and any other .nav-powered navigation.

@@ -492,12 +522,14 @@ And with vertical pills. Ideally, for vertical tabs, you should also add `aria-o
...
...
+
...
...
...
-- 2.47.2