Skip to content

Dynamic tabs: use buttons rather than links #32630

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.

Sign up for GitHub

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

Merged
merged 24 commits into from
Feb 9, 2021
Merged

Dynamic tabs: use buttons rather than links #32630

Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
6ce2580
Dynamic tabs: use buttons rather than links
patrickhlauke Dec 27, 2020
b3993db
Merge branch 'main' into patrickhlauke-navs-tabs
patrickhlauke Dec 29, 2020
5bfe12e
Merge branch 'main' into patrickhlauke-navs-tabs
patrickhlauke Dec 29, 2020
96cd222
Merge branch 'main' into patrickhlauke-navs-tabs
patrickhlauke Jan 5, 2021
fe7577a
Add mention that tabs should be <button> elements
patrickhlauke Jan 5, 2021
34f38ce
Update js unit and visual test accordingly
patrickhlauke Jan 5, 2021
26afac9
Tweak tests
patrickhlauke Jan 5, 2021
64d47aa
Fix tests
patrickhlauke Jan 5, 2021
cb657f9
Tests
patrickhlauke Jan 5, 2021
f94ffc4
Merge branch 'main' into patrickhlauke-navs-tabs
patrickhlauke Jan 5, 2021
f5c8e2f
Remove test for unusued and frankly inappropriate structure/handling
patrickhlauke Jan 5, 2021
c03553d
Tweak code example in events part of the docs
patrickhlauke Jan 5, 2021
e92a2a5
Merge branch 'main' into patrickhlauke-navs-tabs
patrickhlauke Jan 7, 2021
926ab5f
Merge branch 'main' into patrickhlauke-navs-tabs
patrickhlauke Jan 9, 2021
9e27cc0
Add isolation:isolate to prevent focus being overlapped
patrickhlauke Jan 9, 2021
1e562c8
Merge branch 'patrickhlauke-navs-tabs' of https://github.com/twbs/boo…
patrickhlauke Jan 9, 2021
8bc358d
Merge branch 'main' into patrickhlauke-navs-tabs
patrickhlauke Jan 10, 2021
6346c0e
Merge branch 'main' into patrickhlauke-navs-tabs
patrickhlauke Jan 17, 2021
fe10f28
Merge branch 'main' into patrickhlauke-navs-tabs
patrickhlauke Jan 29, 2021
a1aaa19
Merge branch 'main' into patrickhlauke-navs-tabs
patrickhlauke Feb 4, 2021
229a4af
Put back 2 of the removed tests
patrickhlauke Feb 8, 2021
92b72cb
Merge branch 'main' into patrickhlauke-navs-tabs
patrickhlauke Feb 8, 2021
b4a3db8
Reintroduce and fix visual test
patrickhlauke Feb 8, 2021
c541ba7
Merge branch 'main' into patrickhlauke-navs-tabs
patrickhlauke Feb 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
196 changes: 74 additions & 122 deletions js/tests/unit/tab.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,39 @@ describe('Tab', () => {
})

describe('show', () => {
it('should activate element by tab id', done => {
it('should activate element by tab id (using buttons, the preferred semantic way)', done => {
fixtureEl.innerHTML = [
'<ul class="nav">',
'<ul class="nav" role="tablist">',
' <li><button type="button" data-bs-target="#home" role="tab">Home</button></li>',
' <li><button type="button" id="triggerProfile" data-bs-target="#profile" role="tab">Profile</button></li>',
'</ul>',
'<ul>',
' <li id="home" role="tabpanel"></li>',
' <li id="profile" role="tabpanel"></li>',
'</ul>'
].join('')

const profileTriggerEl = fixtureEl.querySelector('#triggerProfile')
const tab = new Tab(profileTriggerEl)

profileTriggerEl.addEventListener('shown.bs.tab', () => {
expect(fixtureEl.querySelector('#profile').classList.contains('active')).toEqual(true)
expect(profileTriggerEl.getAttribute('aria-selected')).toEqual('true')
done()
})

tab.show()
})

it('should activate element by tab id (using links for tabs - not ideal, but still supported)', done => {
fixtureEl.innerHTML = [
'<ul class="nav" role="tablist">',
' <li><a href="#home" role="tab">Home</a></li>',
' <li><a id="triggerProfile" role="tab" href="#profile">Profile</a></li>',
' <li><a id="triggerProfile" href="#profile" role="tab">Profile</a></li>',
'</ul>',
'<ul>',
' <li id="home"></li>',
' <li id="profile"></li>',
' <li id="home" role="tabpanel"></li>',
' <li id="profile" role="tabpanel"></li>',
'</ul>'
].join('')

Expand All @@ -48,12 +72,12 @@ describe('Tab', () => {
it('should activate element by tab id in ordered list', done => {
fixtureEl.innerHTML = [
'<ol class="nav nav-pills">',
' <li><a href="#home">Home</a></li>',
' <li><a id="triggerProfile" href="#profile">Profile</a></li>',
' <li><button type="button" data-bs-target="#home" role="tab">Home</button></li>',
' <li><button type="button" id="triggerProfile" href="#profile" role="tab">Profile</button></li>',
'</ol>',
'<ol>',
' <li id="home"></li>',
' <li id="profile"></li>',
' <li id="home" role="tabpanel"></li>',
' <li id="profile" role="tabpanel"></li>',
'</ol>'
].join('')

Expand All @@ -71,10 +95,10 @@ describe('Tab', () => {
it('should activate element by tab id in nav list', done => {
fixtureEl.innerHTML = [
'<nav class="nav">',
' <a href="#home">Home</a>',
' <a id="triggerProfile" href="#profile">Profile</a>',
' <button type="button" data-bs-target="#home" role="tab">Home</button>',
' <button type="button" id="triggerProfile" data-bs-target="#profile" role="tab">Profile</a>',
'</nav>',
'<nav><div id="home"></div><div id="profile"></div></nav>'
'<div><div id="home" role="tabpanel"></div><div id="profile" role="tabpanel"></div></div>'
].join('')

const profileTriggerEl = fixtureEl.querySelector('#triggerProfile')
Expand All @@ -90,11 +114,11 @@ describe('Tab', () => {

it('should activate element by tab id in list group', done => {
fixtureEl.innerHTML = [
'<div class="list-group">',
' <a href="#home">Home</a>',
' <a id="triggerProfile" href="#profile">Profile</a>',
'<div class="list-group" role="tablist">',
' <button type="button" data-bs-target="#home" role="tab">Home</button>',
' <button type="button" id="triggerProfile" data-bs-target="#profile" role="tab">Profile</button>',
'</div>',
'<nav><div id="home"></div><div id="profile"></div></nav>'
'<div><div id="home" role="tabpanel"></div><div id="profile" role="tabpanel"></div></div>'
].join('')

const profileTriggerEl = fixtureEl.querySelector('#triggerProfile')
Expand Down Expand Up @@ -135,16 +159,16 @@ describe('Tab', () => {
it('should not fire shown when tab is already active', done => {
fixtureEl.innerHTML = [
'<ul class="nav nav-tabs" role="tablist">',
' <li class="nav-item" role="presentation"><a href="#home" class="nav-link active" role="tab">Home</a></li>',
' <li class="nav-item" role="presentation"><a href="#profile" class="nav-link" role="tab">Profile</a></li>',
' <li class="nav-item" role="presentation"><button type="button" data-bs-target="#home" class="nav-link active" role="tab" aria-selected="true">Home</button></li>',
' <li class="nav-item" role="presentation"><button type="button" href="#profile" class="nav-link" role="tab">Profile</button></li>',
'</ul>',
'<div class="tab-content">',
' <div class="tab-pane active" id="home" role="tabpanel"></div>',
' <div class="tab-pane" id="profile" role="tabpanel"></div>',
'</div>'
].join('')

const triggerActive = fixtureEl.querySelector('a.active')
const triggerActive = fixtureEl.querySelector('button.active')
const tab = new Tab(triggerActive)

triggerActive.addEventListener('shown.bs.tab', () => {
Expand All @@ -161,16 +185,16 @@ describe('Tab', () => {
it('should not fire shown when tab is disabled', done => {
fixtureEl.innerHTML = [
'<ul class="nav nav-tabs" role="tablist">',
' <li class="nav-item" role="presentation"><a href="#home" class="nav-link active" role="tab">Home</a></li>',
' <li class="nav-item" role="presentation"><a href="#profile" class="nav-link disabled" role="tab">Profile</a></li>',
' <li class="nav-item" role="presentation"><button type="button" data-bs-target="#home" class="nav-link active" role="tab" aria-selected="true">Home</button></li>',
' <li class="nav-item" role="presentation"><button type="button" data-bs-target="#profile" class="nav-link disabled" role="tab">Profile</button></li>',
'</ul>',
'<div class="tab-content">',
' <div class="tab-pane active" id="home" role="tabpanel"></div>',
' <div class="tab-pane" id="profile" role="tabpanel"></div>',
'</div>'
].join('')

const triggerDisabled = fixtureEl.querySelector('a.disabled')
const triggerDisabled = fixtureEl.querySelector('button.disabled')
const tab = new Tab(triggerDisabled)

triggerDisabled.addEventListener('shown.bs.tab', () => {
Expand All @@ -187,8 +211,8 @@ describe('Tab', () => {
it('show and shown events should reference correct relatedTarget', done => {
fixtureEl.innerHTML = [
'<ul class="nav nav-tabs" role="tablist">',
' <li class="nav-item" role="presentation"><a href="#home" class="nav-link active" role="tab">Home</a></li>',
' <li class="nav-item" role="presentation"><a id="triggerProfile" href="#profile" class="nav-link" role="tab">Profile</a></li>',
' <li class="nav-item" role="presentation"><button type="button" data-bs-target="#home" class="nav-link active" role="tab" aria-selected="true">Home</button></li>',
' <li class="nav-item" role="presentation"><button type="button" id="triggerProfile" data-bs-target="#profile" class="nav-link" role="tab">Profile</button></li>',
'</ul>',
'<div class="tab-content">',
' <div class="tab-pane active" id="home" role="tabpanel"></div>',
Expand All @@ -200,13 +224,13 @@ describe('Tab', () => {
const secondTab = new Tab(secondTabTrigger)

secondTabTrigger.addEventListener('show.bs.tab', ev => {
expect(ev.relatedTarget.hash).toEqual('#home')
expect(ev.relatedTarget.getAttribute('data-bs-target')).toEqual('#home')
})

secondTabTrigger.addEventListener('shown.bs.tab', ev => {
expect(ev.relatedTarget.hash).toEqual('#home')
expect(ev.relatedTarget.getAttribute('data-bs-target')).toEqual('#home')
expect(secondTabTrigger.getAttribute('aria-selected')).toEqual('true')
expect(fixtureEl.querySelector('a:not(.active)').getAttribute('aria-selected')).toEqual('false')
expect(fixtureEl.querySelector('button:not(.active)').getAttribute('aria-selected')).toEqual('false')
done()
})

Expand All @@ -215,13 +239,13 @@ describe('Tab', () => {

it('should fire hide and hidden events', done => {
fixtureEl.innerHTML = [
'<ul class="nav">',
' <li><a href="#home">Home</a></li>',
' <li><a href="#profile">Profile</a></li>',
'<ul class="nav" role="tablist">',
' <li><button type="button" data-bs-target="#home" role="tab">Home</button></li>',
' <li><button type="button" data-bs-target="#profile">Profile</button></li>',
'</ul>'
].join('')

const triggerList = fixtureEl.querySelectorAll('a')
const triggerList = fixtureEl.querySelectorAll('button')
const firstTab = new Tab(triggerList[0])
const secondTab = new Tab(triggerList[1])

Expand All @@ -232,12 +256,12 @@ describe('Tab', () => {

triggerList[0].addEventListener('hide.bs.tab', ev => {
hideCalled = true
expect(ev.relatedTarget.hash).toEqual('#profile')
expect(ev.relatedTarget.getAttribute('data-bs-target')).toEqual('#profile')
})

triggerList[0].addEventListener('hidden.bs.tab', ev => {
expect(hideCalled).toEqual(true)
expect(ev.relatedTarget.hash).toEqual('#profile')
expect(ev.relatedTarget.getAttribute('data-bs-target')).toEqual('#profile')
done()
})

Expand All @@ -246,13 +270,13 @@ describe('Tab', () => {

it('should not fire hidden when hide is prevented', done => {
fixtureEl.innerHTML = [
'<ul class="nav">',
' <li><a href="#home">Home</a></li>',
' <li><a href="#profile">Profile</a></li>',
'<ul class="nav" role="tablist">',
' <li><button type="button" data-bs-target="#home" role="tab">Home</button></li>',
' <li><button type="button" data-bs-target="#profile" role="tab">Profile</button></li>',
'</ul>'
].join('')

const triggerList = fixtureEl.querySelectorAll('a')
const triggerList = fixtureEl.querySelectorAll('button')
const firstTab = new Tab(triggerList[0])
const secondTab = new Tab(triggerList[1])
const expectDone = () => {
Expand All @@ -277,55 +301,6 @@ describe('Tab', () => {

firstTab.show()
})

it('should handle removed tabs', done => {
fixtureEl.innerHTML = [
'<ul class="nav nav-tabs" role="tablist">',
' <li class="nav-item" role="presentation">',
' <a class="nav-link nav-tab" href="#profile" role="tab" data-bs-toggle="tab">',
' <button class="btn-close" aria-label="Close"></button>',
' </a>',
' </li>',
' <li class="nav-item" role="presentation">',
' <a id="secondNav" class="nav-link nav-tab" href="#buzz" role="tab" data-bs-toggle="tab">',
' <button class="btn-close" aria-label="Close"></button>',
' </a>',
' </li>',
' <li class="nav-item" role="presentation">',
' <a class="nav-link nav-tab" href="#references" role="tab" data-bs-toggle="tab">',
' <button id="btnClose" class="btn-close" aria-label="Close"></button>',
' </a>',
' </li>',
'</ul>',
'<div class="tab-content">',
' <div role="tabpanel" class="tab-pane fade show active" id="profile">test 1</div>',
' <div role="tabpanel" class="tab-pane fade" id="buzz">test 2</div>',
' <div role="tabpanel" class="tab-pane fade" id="references">test 3</div>',
'</div>'
].join('')

const secondNavEl = fixtureEl.querySelector('#secondNav')
const btnCloseEl = fixtureEl.querySelector('#btnClose')
const secondNavTab = new Tab(secondNavEl)

secondNavEl.addEventListener('shown.bs.tab', () => {
expect(fixtureEl.querySelectorAll('.nav-tab').length).toEqual(2)
done()
})

btnCloseEl.addEventListener('click', () => {
const linkEl = btnCloseEl.parentNode
const liEl = linkEl.parentNode
const tabId = linkEl.getAttribute('href')
const tabIdEl = fixtureEl.querySelector(tabId)

liEl.parentNode.removeChild(liEl)
tabIdEl.parentNode.removeChild(tabIdEl)
secondNavTab.show()
})

btnCloseEl.click()
})
})

describe('dispose', () => {
Expand Down Expand Up @@ -423,8 +398,8 @@ describe('Tab', () => {
it('should create dynamically a tab', done => {
fixtureEl.innerHTML = [
'<ul class="nav nav-tabs" role="tablist">',
' <li class="nav-item" role="presentation"><a href="#home" class="nav-link active" role="tab">Home</a></li>',
' <li class="nav-item" role="presentation"><a id="triggerProfile" data-bs-toggle="tab" href="#profile" class="nav-link" role="tab">Profile</a></li>',
' <li class="nav-item" role="presentation"><button type="button" data-bs-target="#home" class="nav-link active" role="tab" aria-selected="true">Home</button></li>',
' <li class="nav-item" role="presentation"><button type="button" id="triggerProfile" data-bs-toggle="tab" data-bs-target="#profile" class="nav-link" role="tab">Profile</button></li>',
'</ul>',
'<div class="tab-content">',
' <div class="tab-pane active" id="home" role="tabpanel"></div>',
Expand All @@ -443,41 +418,18 @@ describe('Tab', () => {
secondTabTrigger.click()
})

it('selected tab should deactivate previous selected link in dropdown', () => {
fixtureEl.innerHTML = [
'<ul class="nav nav-tabs">',
' <li class="nav-item"><a class="nav-link" href="#home" data-bs-toggle="tab">Home</a></li>',
' <li class="nav-item"><a class="nav-link" href="#profile" data-bs-toggle="tab">Profile</a></li>',
' <li class="nav-item dropdown">',
' <a class="nav-link dropdown-toggle active" data-bs-toggle="dropdown" href="#">Dropdown</a>',
' <div class="dropdown-menu">',
' <a class="dropdown-item active" href="#dropdown1" id="dropdown1-tab" data-bs-toggle="tab">@fat</a>',
' <a class="dropdown-item" href="#dropdown2" id="dropdown2-tab" data-bs-toggle="tab">@mdo</a>',
' </div>',
' </li>',
'</ul>'
].join('')

const firstLiLinkEl = fixtureEl.querySelector('li:first-child a')

firstLiLinkEl.click()
expect(firstLiLinkEl.classList.contains('active')).toEqual(true)
expect(fixtureEl.querySelector('li:last-child a').classList.contains('active')).toEqual(false)
expect(fixtureEl.querySelector('li:last-child .dropdown-menu a:first-child').classList.contains('active')).toEqual(false)
})

it('should handle nested tabs', done => {
fixtureEl.innerHTML = [
'<nav class="nav nav-tabs" role="tablist">',
' <a id="tab1" href="#x-tab1" class="nav-link" data-bs-toggle="tab" role="tab" aria-controls="x-tab1">Tab 1</a>',
' <a href="#x-tab2" class="nav-link active" data-bs-toggle="tab" role="tab" aria-controls="x-tab2" aria-selected="true">Tab 2</a>',
' <a href="#x-tab3" class="nav-link" data-bs-toggle="tab" role="tab" aria-controls="x-tab3">Tab 3</a>',
' <button type="button" id="tab1" data-bs-target="#x-tab1" class="nav-link" data-bs-toggle="tab" role="tab" aria-controls="x-tab1">Tab 1</button>',
' <button type="button" data-bs-target="#x-tab2" class="nav-link active" data-bs-toggle="tab" role="tab" aria-controls="x-tab2" aria-selected="true">Tab 2</button>',
' <button type="button" data-bs-target="#x-tab3" class="nav-link" data-bs-toggle="tab" role="tab" aria-controls="x-tab3">Tab 3</button>',
'</nav>',
'<div class="tab-content">',
' <div class="tab-pane" id="x-tab1" role="tabpanel">',
' <nav class="nav nav-tabs" role="tablist">',
' <a href="#nested-tab1" class="nav-link active" data-bs-toggle="tab" role="tab" aria-controls="x-tab1" aria-selected="true">Nested Tab 1</a>',
' <a id="tabNested2" href="#nested-tab2" class="nav-link" data-bs-toggle="tab" role="tab" aria-controls="x-profile">Nested Tab2</a>',
' <button type="button" data-bs-target="#nested-tab1" class="nav-link active" data-bs-toggle="tab" role="tab" aria-controls="x-tab1" aria-selected="true">Nested Tab 1</button>',
' <button type="button" id="tabNested2" data-bs-target="#nested-tab2" class="nav-link" data-bs-toggle="tab" role="tab" aria-controls="x-profile">Nested Tab2</button>',
' </nav>',
' <div class="tab-content">',
' <div class="tab-pane active" id="nested-tab1" role="tabpanel">Nested Tab1 Content</div>',
Expand Down Expand Up @@ -509,8 +461,8 @@ describe('Tab', () => {
it('should not remove fade class if no active pane is present', done => {
fixtureEl.innerHTML = [
'<ul class="nav nav-tabs" role="tablist">',
' <li class="nav-item" role="presentation"><a id="tab-home" href="#home" class="nav-link" data-bs-toggle="tab" role="tab">Home</a></li>',
' <li class="nav-item" role="presentation"><a id="tab-profile" href="#profile" class="nav-link" data-bs-toggle="tab" role="tab">Profile</a></li>',
' <li class="nav-item" role="presentation"><button type="button" id="tab-home" data-bs-target="#home" class="nav-link" data-bs-toggle="tab" role="tab">Home</button></li>',
' <li class="nav-item" role="presentation"><button type="button" id="tab-profile" data-bs-target="#profile" class="nav-link" data-bs-toggle="tab" role="tab">Profile</button></li>',
'</ul>',
'<div class="tab-content">',
' <div class="tab-pane fade" id="home" role="tabpanel"></div>',
Expand Down Expand Up @@ -547,10 +499,10 @@ describe('Tab', () => {
fixtureEl.innerHTML = [
'<ul class="nav nav-tabs" role="tablist">',
' <li class="nav-item" role="presentation">',
' <a class="nav-link nav-tab" href="#home" role="tab" data-bs-toggle="tab">Home</a>',
' <button type="button" class="nav-link nav-tab" data-bs-target="#home" role="tab" data-bs-toggle="tab">Home</button>',
' </li>',
' <li class="nav-item" role="presentation">',
' <a id="secondNav" class="nav-link nav-tab" href="#profile" role="tab" data-bs-toggle="tab">Profile</a>',
' <button type="button" id="secondNav" class="nav-link nav-tab" data-bs-target="#profile" role="tab" data-bs-toggle="tab">Profile</button>',
' </li>',
'</ul>',
'<div class="tab-content">',
Expand All @@ -573,10 +525,10 @@ describe('Tab', () => {
fixtureEl.innerHTML = [
'<ul class="nav nav-tabs" role="tablist">',
' <li class="nav-item" role="presentation">',
' <a class="nav-link nav-tab" href="#home" role="tab" data-bs-toggle="tab">Home</a>',
' <button type="button" class="nav-link nav-tab" data-bs-target="#home" role="tab" data-bs-toggle="tab">Home</button>',
' </li>',
' <li class="nav-item" role="presentation">',
' <a id="secondNav" class="nav-link nav-tab" href="#profile" role="tab" data-bs-toggle="tab">Profile</a>',
' <button type="button" id="secondNav" class="nav-link nav-tab" data-bs-target="#profile" role="tab" data-bs-toggle="tab">Profile</button>',
' </li>',
'</ul>',
'<div class="tab-content">',
Expand Down
Loading