From fc02932946424e986a72bb7b47044eab815851cb Mon Sep 17 00:00:00 2001 From: Johann-S Date: Tue, 23 Jul 2019 21:15:00 +0200 Subject: [PATCH] use get selector from element only when needed --- js/src/alert/alert.js | 9 ++----- js/src/carousel/carousel.js | 10 ++------ js/src/collapse/collapse.js | 16 +++++-------- js/src/dropdown/dropdown.js | 11 ++------- js/src/modal/modal.js | 5 ++-- js/src/tab/tab.js | 9 ++----- js/src/util/index.js | 23 ++++++++++++++---- js/src/util/index.spec.js | 48 +++++++++++++++++++++++++++++++++++++ 8 files changed, 82 insertions(+), 49 deletions(-) diff --git a/js/src/alert/alert.js b/js/src/alert/alert.js index 99164bd195..328aa16ae6 100644 --- a/js/src/alert/alert.js +++ b/js/src/alert/alert.js @@ -9,7 +9,7 @@ import { jQuery as $, TRANSITION_END, emulateTransitionEnd, - getSelectorFromElement, + getElementFromSelector, getTransitionDurationFromElement } from '../util/index' import Data from '../dom/data' @@ -90,12 +90,7 @@ class Alert { // Private _getRootElement(element) { - const selector = getSelectorFromElement(element) - let parent = false - - if (selector) { - parent = SelectorEngine.findOne(selector) - } + let parent = getElementFromSelector(element) if (!parent) { parent = SelectorEngine.closest(element, `.${ClassName.ALERT}`) diff --git a/js/src/carousel/carousel.js b/js/src/carousel/carousel.js index 0e1bad14ac..af4229f07c 100644 --- a/js/src/carousel/carousel.js +++ b/js/src/carousel/carousel.js @@ -9,7 +9,7 @@ import { jQuery as $, TRANSITION_END, emulateTransitionEnd, - getSelectorFromElement, + getElementFromSelector, getTransitionDurationFromElement, isVisible, makeArray, @@ -569,13 +569,7 @@ class Carousel { } static _dataApiClickHandler(event) { - const selector = getSelectorFromElement(this) - - if (!selector) { - return - } - - const target = SelectorEngine.findOne(selector) + const target = getElementFromSelector(this) if (!target || !target.classList.contains(ClassName.CAROUSEL)) { return diff --git a/js/src/collapse/collapse.js b/js/src/collapse/collapse.js index 671dc3b6cd..c1d9aa2f1b 100644 --- a/js/src/collapse/collapse.js +++ b/js/src/collapse/collapse.js @@ -10,6 +10,7 @@ import { TRANSITION_END, emulateTransitionEnd, getSelectorFromElement, + getElementFromSelector, getTransitionDurationFromElement, isElement, makeArray, @@ -244,15 +245,11 @@ class Collapse { if (triggerArrayLength > 0) { for (let i = 0; i < triggerArrayLength; i++) { const trigger = this._triggerArray[i] - const selector = getSelectorFromElement(trigger) + const elem = getElementFromSelector(trigger) - if (selector !== null) { - const elem = SelectorEngine.findOne(selector) - - if (!elem.classList.contains(ClassName.SHOW)) { - trigger.classList.add(ClassName.COLLAPSED) - trigger.setAttribute('aria-expanded', false) - } + if (elem && !elem.classList.contains(ClassName.SHOW)) { + trigger.classList.add(ClassName.COLLAPSED) + trigger.setAttribute('aria-expanded', false) } } } @@ -320,8 +317,7 @@ class Collapse { makeArray(SelectorEngine.find(selector, parent)) .forEach(element => { - const selector = getSelectorFromElement(element) - const selected = selector ? SelectorEngine.findOne(selector) : null + const selected = getElementFromSelector(element) this._addAriaAndCollapsedClass( selected, diff --git a/js/src/dropdown/dropdown.js b/js/src/dropdown/dropdown.js index 8607afe7f5..f30c3924b3 100644 --- a/js/src/dropdown/dropdown.js +++ b/js/src/dropdown/dropdown.js @@ -7,7 +7,7 @@ import { jQuery as $, - getSelectorFromElement, + getElementFromSelector, isElement, makeArray, noop, @@ -442,14 +442,7 @@ class Dropdown { } static _getParentFromElement(element) { - let parent - const selector = getSelectorFromElement(element) - - if (selector) { - parent = SelectorEngine.findOne(selector) - } - - return parent || element.parentNode + return getElementFromSelector(element) || element.parentNode } static _dataApiKeydownHandler(event) { diff --git a/js/src/modal/modal.js b/js/src/modal/modal.js index 4c430a01f5..a0e460f228 100644 --- a/js/src/modal/modal.js +++ b/js/src/modal/modal.js @@ -9,7 +9,7 @@ import { jQuery as $, TRANSITION_END, emulateTransitionEnd, - getSelectorFromElement, + getElementFromSelector, getTransitionDurationFromElement, isVisible, makeArray, @@ -549,8 +549,7 @@ class Modal { */ EventHandler.on(document, Event.CLICK_DATA_API, Selector.DATA_TOGGLE, function (event) { - const selector = getSelectorFromElement(this) - const target = SelectorEngine.findOne(selector) + const target = getElementFromSelector(this) if (this.tagName === 'A' || this.tagName === 'AREA') { event.preventDefault() diff --git a/js/src/tab/tab.js b/js/src/tab/tab.js index e882ff1d2a..c9f4869ef6 100644 --- a/js/src/tab/tab.js +++ b/js/src/tab/tab.js @@ -9,7 +9,7 @@ import { jQuery as $, TRANSITION_END, emulateTransitionEnd, - getSelectorFromElement, + getElementFromSelector, getTransitionDurationFromElement, makeArray, reflow @@ -85,10 +85,9 @@ class Tab { return } - let target let previous + const target = getElementFromSelector(this._element) const listElement = SelectorEngine.closest(this._element, Selector.NAV_LIST_GROUP) - const selector = getSelectorFromElement(this._element) if (listElement) { const itemSelector = listElement.nodeName === 'UL' || listElement.nodeName === 'OL' ? Selector.ACTIVE_UL : Selector.ACTIVE @@ -113,10 +112,6 @@ class Tab { return } - if (selector) { - target = SelectorEngine.findOne(selector) - } - this._activate( this._element, listElement diff --git a/js/src/util/index.js b/js/src/util/index.js index 5788c8749e..537b391dcc 100644 --- a/js/src/util/index.js +++ b/js/src/util/index.js @@ -28,20 +28,32 @@ const getUID = prefix => { return prefix } -const getSelectorFromElement = element => { +const getSelector = element => { let selector = element.getAttribute('data-target') if (!selector || selector === '#') { const hrefAttr = element.getAttribute('href') - selector = hrefAttr && hrefAttr !== '#' ? hrefAttr.trim() : '' + selector = hrefAttr && hrefAttr !== '#' ? hrefAttr.trim() : null } - try { + return selector +} + +const getSelectorFromElement = element => { + const selector = getSelector(element) + + if (selector) { return document.querySelector(selector) ? selector : null - } catch (error) { - return null } + + return null +} + +const getElementFromSelector = element => { + const selector = getSelector(element) + + return selector ? document.querySelector(selector) : null } const getTransitionDurationFromElement = element => { @@ -169,6 +181,7 @@ export { TRANSITION_END, getUID, getSelectorFromElement, + getElementFromSelector, getTransitionDurationFromElement, triggerTransitionEnd, isElement, diff --git a/js/src/util/index.spec.js b/js/src/util/index.spec.js index bf450ee365..b667c270ea 100644 --- a/js/src/util/index.spec.js +++ b/js/src/util/index.spec.js @@ -64,6 +64,54 @@ describe('Util', () => { expect(Util.getSelectorFromElement(testEl)).toBeNull() }) + + it('should return null if no selector', () => { + fixtureEl.innerHTML = '
' + + const testEl = fixtureEl.querySelector('div') + + expect(Util.getSelectorFromElement(testEl)).toBeNull() + }) + }) + + describe('getElementFromSelector', () => { + it('should get element from data-target', () => { + fixtureEl.innerHTML = [ + '
', + '
' + ].join('') + + const testEl = fixtureEl.querySelector('#test') + + expect(Util.getElementFromSelector(testEl)).toEqual(fixtureEl.querySelector('.target')) + }) + + it('should get element from href if no data-target set', () => { + fixtureEl.innerHTML = [ + '', + '
' + ].join('') + + const testEl = fixtureEl.querySelector('#test') + + expect(Util.getElementFromSelector(testEl)).toEqual(fixtureEl.querySelector('.target')) + }) + + it('should return null if element not found', () => { + fixtureEl.innerHTML = '' + + const testEl = fixtureEl.querySelector('#test') + + expect(Util.getElementFromSelector(testEl)).toBeNull() + }) + + it('should return null if no selector', () => { + fixtureEl.innerHTML = '
' + + const testEl = fixtureEl.querySelector('div') + + expect(Util.getElementFromSelector(testEl)).toBeNull() + }) }) describe('getTransitionDurationFromElement', () => {