Skip to content

use only data target to query elements in our plugin #31185

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

use only data target to query elements in our plugin #31185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion js/src/collapse.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ class Collapse {
this._element = element
this._config = this._getConfig(config)
this._triggerArray = SelectorEngine.find(
`${SELECTOR_DATA_TOGGLE}[href="#${element.id}"],` +
`${SELECTOR_DATA_TOGGLE}[data-target="#${element.id}"]`
)

Expand Down
14 changes: 13 additions & 1 deletion js/src/scrollspy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class ScrollSpy {
targets
.map(element => {
let target
const targetSelector = getSelectorFromElement(element)
const targetSelector = this._getSelectorFromElement(element)

if (targetSelector) {
target = SelectorEngine.findOne(targetSelector)
Expand Down Expand Up @@ -163,6 +163,18 @@ class ScrollSpy {

// Private

_getSelectorFromElement(element) {
let selector = getSelectorFromElement(element)

if (!selector) {
const hrefAttr = element.getAttribute('href')

selector = hrefAttr && hrefAttr !== '#' ? hrefAttr.trim() : null
}

return selector
}

_getConfig(config) {
config = {
...Default,
Expand Down
8 changes: 3 additions & 5 deletions js/src/util/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,10 @@ const getUID = prefix => {
}

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

if (!selector || selector === '#') {
const hrefAttr = element.getAttribute('href')

selector = hrefAttr && hrefAttr !== '#' ? hrefAttr.trim() : null
if (!selector) {
return null
}

return selector
Expand Down
24 changes: 12 additions & 12 deletions js/tests/unit/collapse.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ describe('Collapse', () => {
describe('data-api', () => {
it('should show multiple collapsed elements', done => {
fixtureEl.innerHTML = [
'<a role="button" data-toggle="collapse" class="collapsed" href=".multi"></a>',
'<a href="#" role="button" data-toggle="collapse" class="collapsed" data-target=".multi"></a>',
'<div id="collapse1" class="collapse multi"></div>',
'<div id="collapse2" class="collapse multi"></div>'
].join('')
Expand All @@ -398,7 +398,7 @@ describe('Collapse', () => {

it('should hide multiple collapsed elements', done => {
fixtureEl.innerHTML = [
'<a role="button" data-toggle="collapse" href=".multi"></a>',
'<a href="#" role="button" data-toggle="collapse" data-target=".multi"></a>',
'<div id="collapse1" class="collapse multi show"></div>',
'<div id="collapse2" class="collapse multi show"></div>'
].join('')
Expand Down Expand Up @@ -466,11 +466,11 @@ describe('Collapse', () => {
fixtureEl.innerHTML = [
'<div id="accordion">',
' <div class="item">',
' <a id="linkTrigger" data-toggle="collapse" href="#collapseOne" aria-expanded="false" aria-controls="collapseOne"></a>',
' <a id="linkTrigger" href="#" data-toggle="collapse" data-target="#collapseOne" aria-expanded="false" aria-controls="collapseOne"></a>',
' <div id="collapseOne" class="collapse" role="tabpanel" aria-labelledby="headingThree" data-parent="#accordion"></div>',
' </div>',
' <div class="item">',
' <a id="linkTriggerTwo" data-toggle="collapse" href="#collapseTwo" aria-expanded="false" aria-controls="collapseTwo"></a>',
' <a id="linkTriggerTwo" href="#" data-toggle="collapse" data-target="#collapseTwo" aria-expanded="false" aria-controls="collapseTwo"></a>',
' <div id="collapseTwo" class="collapse show" role="tabpanel" aria-labelledby="headingTwo" data-parent="#accordion"></div>',
' </div>',
'</div>'
Expand Down Expand Up @@ -521,13 +521,13 @@ describe('Collapse', () => {
' <div class="row">',
' <div class="col-lg-6">',
' <div class="item">',
' <a id="linkTrigger" data-toggle="collapse" href="#collapseOne" aria-expanded="false" aria-controls="collapseOne"></a>',
' <a id="linkTrigger" href="#" data-toggle="collapse" data-target="#collapseOne" aria-expanded="false" aria-controls="collapseOne"></a>',
' <div id="collapseOne" class="collapse" role="tabpanel" aria-labelledby="headingThree" data-parent="#accordion"></div>',
' </div>',
' </div>',
' <div class="col-lg-6">',
' <div class="item">',
' <a id="linkTriggerTwo" data-toggle="collapse" href="#collapseTwo" aria-expanded="false" aria-controls="collapseTwo"></a>',
' <a id="linkTriggerTwo" href="#" data-toggle="collapse" data-target="#collapseTwo" aria-expanded="false" aria-controls="collapseTwo"></a>',
' <div id="collapseTwo" class="collapse show" role="tabpanel" aria-labelledby="headingTwo" data-parent="#accordion"></div>',
' </div>',
' </div>',
Expand Down Expand Up @@ -647,18 +647,18 @@ describe('Collapse', () => {
fixtureEl.innerHTML = [
'<div id="accordion">',
' <div class="item">',
' <a id="linkTrigger" data-toggle="collapse" href="#collapseOne" aria-expanded="false" aria-controls="collapseOne"></a>',
' <a id="linkTrigger" href="#" data-toggle="collapse" data-target="#collapseOne" aria-expanded="false" aria-controls="collapseOne"></a>',
' <div id="collapseOne" data-parent="#accordion" class="collapse" role="tabpanel" aria-labelledby="headingThree">',
' <div id="nestedAccordion">',
' <div class="item">',
' <a id="nestedLinkTrigger" data-toggle="collapse" href="#nestedCollapseOne" aria-expanded="false" aria-controls="nestedCollapseOne"></a>',
' <a id="nestedLinkTrigger" href="#" data-toggle="collapse" data-target="#nestedCollapseOne" aria-expanded="false" aria-controls="nestedCollapseOne"></a>',
' <div id="nestedCollapseOne" data-parent="#nestedAccordion" class="collapse" role="tabpanel" aria-labelledby="headingThree"></div>',
' </div>',
' </div>',
' </div>',
' </div>',
' <div class="item">',
' <a id="linkTriggerTwo" data-toggle="collapse" href="#collapseTwo" aria-expanded="false" aria-controls="collapseTwo"></a>',
' <a id="linkTriggerTwo" href="#" data-toggle="collapse" data-target="#collapseTwo" aria-expanded="false" aria-controls="collapseTwo"></a>',
' <div id="collapseTwo" data-parent="#accordion" class="collapse show" role="tabpanel" aria-labelledby="headingTwo"></div>',
' </div>',
'</div>'
Expand Down Expand Up @@ -703,9 +703,9 @@ describe('Collapse', () => {

it('should add "collapsed" class and set aria-expanded to triggers only when all the targeted collapse are hidden', done => {
fixtureEl.innerHTML = [
'<a id="trigger1" role="button" data-toggle="collapse" href="#test1"></a>',
'<a id="trigger2" role="button" data-toggle="collapse" href="#test2"></a>',
'<a id="trigger3" role="button" data-toggle="collapse" href=".multi"></a>',
'<a id="trigger1" href="#" role="button" data-toggle="collapse" data-target="#test1"></a>',
'<a id="trigger2" href="#" role="button" data-toggle="collapse" data-target="#test2"></a>',
'<a id="trigger3" href="#" role="button" data-toggle="collapse" data-target=".multi"></a>',
'<div id="test1" class="multi"></div>',
'<div id="test2" class="multi"></div>'
].join('')
Expand Down
20 changes: 10 additions & 10 deletions js/tests/unit/dropdown.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1086,9 +1086,9 @@ describe('Dropdown', () => {
fixtureEl.innerHTML = [
'<div class="nav">',
' <div class="dropdown" id="testmenu">',
' <a class="dropdown-toggle" data-toggle="dropdown" href="#testmenu">Test menu</a>',
' <a href="#" class="dropdown-toggle" data-toggle="dropdown" data-target="#testmenu">Test menu</a>',
' <div class="dropdown-menu">',
' <a class="dropdown-item" href="#sub1">Submenu 1</a>',
' <a href="#" class="dropdown-item" data-target="#sub1">Submenu 1</a>',
' </div>',
' </div>',
'</div>',
Expand Down Expand Up @@ -1138,9 +1138,9 @@ describe('Dropdown', () => {
it('should remove "show" class if body if tabbing outside of menu, with multiple dropdowns', done => {
fixtureEl.innerHTML = [
'<div class="dropdown">',
' <a class="dropdown-toggle" data-toggle="dropdown" href="#testmenu">Test menu</a>',
' <a href="#" class="dropdown-toggle" data-toggle="dropdown" data-target="#testmenu">Test menu</a>',
' <div class="dropdown-menu">',
' <a class="dropdown-item" href="#sub1">Submenu 1</a>',
' <a href="#" class="dropdown-item" data-target="#sub1">Submenu 1</a>',
' </div>',
'</div>',
'<div class="btn-group">',
Expand Down Expand Up @@ -1199,7 +1199,7 @@ describe('Dropdown', () => {
'<div class="dropdown">',
' <button class="btn dropdown-toggle" data-toggle="dropdown">Dropdown</button>',
' <div class="dropdown-menu">',
' <a class="dropdown-item" href="#sub1">Submenu 1</a>',
' <a href="#" class="dropdown-item" data-target="#sub1">Submenu 1</a>',
' </div>',
'</div>'
].join('')
Expand Down Expand Up @@ -1231,7 +1231,7 @@ describe('Dropdown', () => {
'<div class="dropdown">',
' <button class="btn dropdown-toggle" data-toggle="dropdown">Dropdown</button>',
' <div class="dropdown-menu">',
' <a class="dropdown-item" href="#sub1">Submenu 1</a>',
' <a href="#" class="dropdown-item" data-target="#sub1">Submenu 1</a>',
' <input type="text">',
' <textarea></textarea>',
' </div>',
Expand Down Expand Up @@ -1267,7 +1267,7 @@ describe('Dropdown', () => {
'<div class="dropdown">',
' <button class="btn dropdown-toggle" data-toggle="dropdown">Dropdown</button>',
' <div class="dropdown-menu">',
' <a class="dropdown-item disabled" href="#sub1">Submenu 1</a>',
' <a href="#" class="dropdown-item disabled" data-target="#sub1">Submenu 1</a>',
' <button class="dropdown-item" type="button" disabled>Disabled button</button>',
' <a id="item1" class="dropdown-item" href="#">Another link</a>',
' </div>',
Expand Down Expand Up @@ -1303,8 +1303,8 @@ describe('Dropdown', () => {
' <button class="btn dropdown-toggle" data-toggle="dropdown">Dropdown</button>',
' <div class="dropdown-menu">',
' <button class="dropdown-item d-none" type="button">Hidden button by class</button>',
' <a class="dropdown-item" href="#sub1" style="display: none">Hidden link</a>',
' <a class="dropdown-item" href="#sub1" style="visibility: hidden">Hidden link</a>',
' <a href="#" class="dropdown-item" data-target="#sub1" style="display: none">Hidden link</a>',
' <a href="#" class="dropdown-item" data-target="#sub1" style="visibility: hidden">Hidden link</a>',
' <a id="item1" class="dropdown-item" href="#">Another link</a>',
' </div>',
'</div>'
Expand Down Expand Up @@ -1454,7 +1454,7 @@ describe('Dropdown', () => {
'<div class="dropdown">',
' <button class="btn dropdown-toggle" data-toggle="dropdown">Dropdown</button>',
' <div class="dropdown-menu">',
' <a class="dropdown-item" href="#sub1">Submenu 1</a>',
' <a href="#" class="dropdown-item" data-target="#sub1">Submenu 1</a>',
' <input type="text">',
' <textarea></textarea>',
' </div>',
Expand Down
24 changes: 23 additions & 1 deletion js/tests/unit/scrollspy.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ describe('ScrollSpy', () => {
expect(navEl.getAttribute('id')).not.toEqual(null)
})

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

it('should process element with target in data-target', () => {
fixtureEl.innerHTML = [
'<nav id="navigation" class="navbar">',
' <ul class="navbar-nav">',
' <li class="nav-item active"><a class="nav-link" id="one-link" href="#">One</a></li>',
' <li class="nav-item"><a class="nav-link" id="two-link" href="#" data-target="#two">Two</a></li>',
' <li class="nav-item"><a class="nav-link" id="three-link" href="#" data-target="#three">Three</a></li>',
' </ul>',
'</nav>',
'<div id="content" style="height: 200px; overflow-y: auto;">',
' <div id="two" style="height: 300px;"></div>',
' <div id="three" style="height: 10px;"></div>',
'</div>'
].join('')

const scrollSpy = new ScrollSpy(fixtureEl.querySelector('#content'), {
target: '#navigation'
})

expect(scrollSpy._targets.length).toEqual(2)
})

it('should only switch "active" class on current target', done => {
fixtureEl.innerHTML = [
'<div id="root" class="active" style="display: block">',
Expand Down
Loading