Skip to content

Commit 686b88c

Browse files
Johann-SJohann-S
committed
use only data target to query elements in our plugin
1 parent 2c38800 commit 686b88c

19 files changed

+212
-221
lines changed

‎js/src/collapse.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ class Collapse {
7272
this._element = element
7373
this._config = this._getConfig(config)
7474
this._triggerArray = SelectorEngine.find(
75-
`${SELECTOR_DATA_TOGGLE}[href="#${element.id}"],` +
7675
`${SELECTOR_DATA_TOGGLE}[data-target="#${element.id}"]`
7776
)
7877

‎js/src/scrollspy.js

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ class ScrollSpy {
121121
targets
122122
.map(element => {
123123
let target
124-
const targetSelector = getSelectorFromElement(element)
124+
const targetSelector = this._getSelectorFromElement(element)
125125

126126
if (targetSelector) {
127127
target = SelectorEngine.findOne(targetSelector)
@@ -163,6 +163,18 @@ class ScrollSpy {
163163

164164
// Private
165165

166+
_getSelectorFromElement(element) {
167+
let selector = getSelectorFromElement(element)
168+
169+
if (!selector) {
170+
const hrefAttr = element.getAttribute('href')
171+
172+
selector = hrefAttr && hrefAttr !== '#' ? hrefAttr.trim() : null
173+
}
174+
175+
return selector
176+
}
177+
166178
_getConfig(config) {
167179
config = {
168180
...Default,

‎js/src/util/index.js

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,10 @@ const getUID = prefix => {
3333
}
3434

3535
const getSelector = element => {
36-
let selector = element.getAttribute('data-target')
36+
const selector = element.getAttribute('data-target')
3737

38-
if (!selector || selector === '#') {
39-
const hrefAttr = element.getAttribute('href')
40-
41-
selector = hrefAttr && hrefAttr !== '#' ? hrefAttr.trim() : null
38+
if (!selector) {
39+
return null
4240
}
4341

4442
return selector

‎js/tests/unit/collapse.spec.js

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ describe('Collapse', () => {
376376
describe('data-api', () => {
377377
it('should show multiple collapsed elements', done => {
378378
fixtureEl.innerHTML = [
379-
'<a role="button" data-toggle="collapse" class="collapsed" href=".multi"></a>',
379+
'<a href="#" role="button" data-toggle="collapse" class="collapsed" data-target=".multi"></a>',
380380
'<div id="collapse1" class="collapse multi"></div>',
381381
'<div id="collapse2" class="collapse multi"></div>'
382382
].join('')
@@ -398,7 +398,7 @@ describe('Collapse', () => {
398398

399399
it('should hide multiple collapsed elements', done => {
400400
fixtureEl.innerHTML = [
401-
'<a role="button" data-toggle="collapse" href=".multi"></a>',
401+
'<a href="#" role="button" data-toggle="collapse" data-target=".multi"></a>',
402402
'<div id="collapse1" class="collapse multi show"></div>',
403403
'<div id="collapse2" class="collapse multi show"></div>'
404404
].join('')
@@ -466,11 +466,11 @@ describe('Collapse', () => {
466466
fixtureEl.innerHTML = [
467467
'<div id="accordion">',
468468
' <div class="item">',
469-
' <a id="linkTrigger" data-toggle="collapse" href="#collapseOne" aria-expanded="false" aria-controls="collapseOne"></a>',
469+
' <a id="linkTrigger" href="#" data-toggle="collapse" data-target="#collapseOne" aria-expanded="false" aria-controls="collapseOne"></a>',
470470
' <div id="collapseOne" class="collapse" role="tabpanel" aria-labelledby="headingThree" data-parent="#accordion"></div>',
471471
' </div>',
472472
' <div class="item">',
473-
' <a id="linkTriggerTwo" data-toggle="collapse" href="#collapseTwo" aria-expanded="false" aria-controls="collapseTwo"></a>',
473+
' <a id="linkTriggerTwo" href="#" data-toggle="collapse" data-target="#collapseTwo" aria-expanded="false" aria-controls="collapseTwo"></a>',
474474
' <div id="collapseTwo" class="collapse show" role="tabpanel" aria-labelledby="headingTwo" data-parent="#accordion"></div>',
475475
' </div>',
476476
'</div>'
@@ -521,13 +521,13 @@ describe('Collapse', () => {
521521
' <div class="row">',
522522
' <div class="col-lg-6">',
523523
' <div class="item">',
524-
' <a id="linkTrigger" data-toggle="collapse" href="#collapseOne" aria-expanded="false" aria-controls="collapseOne"></a>',
524+
' <a id="linkTrigger" href="#" data-toggle="collapse" data-target="#collapseOne" aria-expanded="false" aria-controls="collapseOne"></a>',
525525
' <div id="collapseOne" class="collapse" role="tabpanel" aria-labelledby="headingThree" data-parent="#accordion"></div>',
526526
' </div>',
527527
' </div>',
528528
' <div class="col-lg-6">',
529529
' <div class="item">',
530-
' <a id="linkTriggerTwo" data-toggle="collapse" href="#collapseTwo" aria-expanded="false" aria-controls="collapseTwo"></a>',
530+
' <a id="linkTriggerTwo" href="#" data-toggle="collapse" data-target="#collapseTwo" aria-expanded="false" aria-controls="collapseTwo"></a>',
531531
' <div id="collapseTwo" class="collapse show" role="tabpanel" aria-labelledby="headingTwo" data-parent="#accordion"></div>',
532532
' </div>',
533533
' </div>',
@@ -647,18 +647,18 @@ describe('Collapse', () => {
647647
fixtureEl.innerHTML = [
648648
'<div id="accordion">',
649649
' <div class="item">',
650-
' <a id="linkTrigger" data-toggle="collapse" href="#collapseOne" aria-expanded="false" aria-controls="collapseOne"></a>',
650+
' <a id="linkTrigger" href="#" data-toggle="collapse" data-target="#collapseOne" aria-expanded="false" aria-controls="collapseOne"></a>',
651651
' <div id="collapseOne" data-parent="#accordion" class="collapse" role="tabpanel" aria-labelledby="headingThree">',
652652
' <div id="nestedAccordion">',
653653
' <div class="item">',
654-
' <a id="nestedLinkTrigger" data-toggle="collapse" href="#nestedCollapseOne" aria-expanded="false" aria-controls="nestedCollapseOne"></a>',
654+
' <a id="nestedLinkTrigger" href="#" data-toggle="collapse" data-target="#nestedCollapseOne" aria-expanded="false" aria-controls="nestedCollapseOne"></a>',
655655
' <div id="nestedCollapseOne" data-parent="#nestedAccordion" class="collapse" role="tabpanel" aria-labelledby="headingThree"></div>',
656656
' </div>',
657657
' </div>',
658658
' </div>',
659659
' </div>',
660660
' <div class="item">',
661-
' <a id="linkTriggerTwo" data-toggle="collapse" href="#collapseTwo" aria-expanded="false" aria-controls="collapseTwo"></a>',
661+
' <a id="linkTriggerTwo" href="#" data-toggle="collapse" data-target="#collapseTwo" aria-expanded="false" aria-controls="collapseTwo"></a>',
662662
' <div id="collapseTwo" data-parent="#accordion" class="collapse show" role="tabpanel" aria-labelledby="headingTwo"></div>',
663663
' </div>',
664664
'</div>'
@@ -703,9 +703,9 @@ describe('Collapse', () => {
703703

704704
it('should add "collapsed" class and set aria-expanded to triggers only when all the targeted collapse are hidden', done => {
705705
fixtureEl.innerHTML = [
706-
'<a id="trigger1" role="button" data-toggle="collapse" href="#test1"></a>',
707-
'<a id="trigger2" role="button" data-toggle="collapse" href="#test2"></a>',
708-
'<a id="trigger3" role="button" data-toggle="collapse" href=".multi"></a>',
706+
'<a id="trigger1" href="#" role="button" data-toggle="collapse" data-target="#test1"></a>',
707+
'<a id="trigger2" href="#" role="button" data-toggle="collapse" data-target="#test2"></a>',
708+
'<a id="trigger3" href="#" role="button" data-toggle="collapse" data-target=".multi"></a>',
709709
'<div id="test1" class="multi"></div>',
710710
'<div id="test2" class="multi"></div>'
711711
].join('')

‎js/tests/unit/dropdown.spec.js

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1086,9 +1086,9 @@ describe('Dropdown', () => {
10861086
fixtureEl.innerHTML = [
10871087
'<div class="nav">',
10881088
' <div class="dropdown" id="testmenu">',
1089-
' <a class="dropdown-toggle" data-toggle="dropdown" href="#testmenu">Test menu</a>',
1089+
' <a href="#" class="dropdown-toggle" data-toggle="dropdown" data-target="#testmenu">Test menu</a>',
10901090
' <div class="dropdown-menu">',
1091-
' <a class="dropdown-item" href="#sub1">Submenu 1</a>',
1091+
' <a href="#" class="dropdown-item" data-target="#sub1">Submenu 1</a>',
10921092
' </div>',
10931093
' </div>',
10941094
'</div>',
@@ -1138,9 +1138,9 @@ describe('Dropdown', () => {
11381138
it('should remove "show" class if body if tabbing outside of menu, with multiple dropdowns', done => {
11391139
fixtureEl.innerHTML = [
11401140
'<div class="dropdown">',
1141-
' <a class="dropdown-toggle" data-toggle="dropdown" href="#testmenu">Test menu</a>',
1141+
' <a href="#" class="dropdown-toggle" data-toggle="dropdown" data-target="#testmenu">Test menu</a>',
11421142
' <div class="dropdown-menu">',
1143-
' <a class="dropdown-item" href="#sub1">Submenu 1</a>',
1143+
' <a href="#" class="dropdown-item" data-target="#sub1">Submenu 1</a>',
11441144
' </div>',
11451145
'</div>',
11461146
'<div class="btn-group">',
@@ -1199,7 +1199,7 @@ describe('Dropdown', () => {
11991199
'<div class="dropdown">',
12001200
' <button class="btn dropdown-toggle" data-toggle="dropdown">Dropdown</button>',
12011201
' <div class="dropdown-menu">',
1202-
' <a class="dropdown-item" href="#sub1">Submenu 1</a>',
1202+
' <a href="#" class="dropdown-item" data-target="#sub1">Submenu 1</a>',
12031203
' </div>',
12041204
'</div>'
12051205
].join('')
@@ -1231,7 +1231,7 @@ describe('Dropdown', () => {
12311231
'<div class="dropdown">',
12321232
' <button class="btn dropdown-toggle" data-toggle="dropdown">Dropdown</button>',
12331233
' <div class="dropdown-menu">',
1234-
' <a class="dropdown-item" href="#sub1">Submenu 1</a>',
1234+
' <a href="#" class="dropdown-item" data-target="#sub1">Submenu 1</a>',
12351235
' <input type="text">',
12361236
' <textarea></textarea>',
12371237
' </div>',
@@ -1267,7 +1267,7 @@ describe('Dropdown', () => {
12671267
'<div class="dropdown">',
12681268
' <button class="btn dropdown-toggle" data-toggle="dropdown">Dropdown</button>',
12691269
' <div class="dropdown-menu">',
1270-
' <a class="dropdown-item disabled" href="#sub1">Submenu 1</a>',
1270+
' <a href="#" class="dropdown-item disabled" data-target="#sub1">Submenu 1</a>',
12711271
' <button class="dropdown-item" type="button" disabled>Disabled button</button>',
12721272
' <a id="item1" class="dropdown-item" href="#">Another link</a>',
12731273
' </div>',
@@ -1303,8 +1303,8 @@ describe('Dropdown', () => {
13031303
' <button class="btn dropdown-toggle" data-toggle="dropdown">Dropdown</button>',
13041304
' <div class="dropdown-menu">',
13051305
' <button class="dropdown-item d-none" type="button">Hidden button by class</button>',
1306-
' <a class="dropdown-item" href="#sub1" style="display: none">Hidden link</a>',
1307-
' <a class="dropdown-item" href="#sub1" style="visibility: hidden">Hidden link</a>',
1306+
' <a href="#" class="dropdown-item" data-target="#sub1" style="display: none">Hidden link</a>',
1307+
' <a href="#" class="dropdown-item" data-target="#sub1" style="visibility: hidden">Hidden link</a>',
13081308
' <a id="item1" class="dropdown-item" href="#">Another link</a>',
13091309
' </div>',
13101310
'</div>'
@@ -1454,7 +1454,7 @@ describe('Dropdown', () => {
14541454
'<div class="dropdown">',
14551455
' <button class="btn dropdown-toggle" data-toggle="dropdown">Dropdown</button>',
14561456
' <div class="dropdown-menu">',
1457-
' <a class="dropdown-item" href="#sub1">Submenu 1</a>',
1457+
' <a href="#" class="dropdown-item" data-target="#sub1">Submenu 1</a>',
14581458
' <input type="text">',
14591459
' <textarea></textarea>',
14601460
' </div>',

‎js/tests/unit/scrollspy.spec.js

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ describe('ScrollSpy', () => {
6464
expect(navEl.getAttribute('id')).not.toEqual(null)
6565
})
6666

67-
it('should not process element without target', () => {
67+
it('should not process element without target and process elements with target inside href attribute', () => {
6868
fixtureEl.innerHTML = [
6969
'<nav id="navigation" class="navbar">',
7070
' <ul class="navbar-nav">',
@@ -86,6 +86,28 @@ describe('ScrollSpy', () => {
8686
expect(scrollSpy._targets.length).toEqual(2)
8787
})
8888

89+
it('should process element with target in data-target', () => {
90+
fixtureEl.innerHTML = [
91+
'<nav id="navigation" class="navbar">',
92+
' <ul class="navbar-nav">',
93+
' <li class="nav-item active"><a class="nav-link" id="one-link" href="#">One</a></li>',
94+
' <li class="nav-item"><a class="nav-link" id="two-link" href="#" data-target="#two">Two</a></li>',
95+
' <li class="nav-item"><a class="nav-link" id="three-link" href="#" data-target="#three">Three</a></li>',
96+
' </ul>',
97+
'</nav>',
98+
'<div id="content" style="height: 200px; overflow-y: auto;">',
99+
' <div id="two" style="height: 300px;"></div>',
100+
' <div id="three" style="height: 10px;"></div>',
101+
'</div>'
102+
].join('')
103+
104+
const scrollSpy = new ScrollSpy(fixtureEl.querySelector('#content'), {
105+
target: '#navigation'
106+
})
107+
108+
expect(scrollSpy._targets.length).toEqual(2)
109+
})
110+
89111
it('should only switch "active" class on current target', done => {
90112
fixtureEl.innerHTML = [
91113
'<div id="root" class="active" style="display: block">',

0 commit comments

Comments
 (0)