From 3548f15bd49a1d5f905650f65917182c5db4f1fe Mon Sep 17 00:00:00 2001 From: Rohit Sharma Date: Sat, 27 Mar 2021 21:38:45 +0530 Subject: [PATCH 1/3] Use cached `noop` function everywhere --- js/src/dropdown.js | 6 +++--- js/src/tooltip.js | 2 +- js/src/util/index.js | 2 +- js/tests/unit/util/index.spec.js | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/js/src/dropdown.js b/js/src/dropdown.js index 6b541ed150b3..3e1ef5fd4f5b 100644 --- a/js/src/dropdown.js +++ b/js/src/dropdown.js @@ -192,7 +192,7 @@ class Dropdown extends BaseComponent { if ('ontouchstart' in document.documentElement && !parent.closest(SELECTOR_NAVBAR_NAV)) { [].concat(...document.body.children) - .forEach(elem => EventHandler.on(elem, 'mouseover', null, noop())) + .forEach(elem => EventHandler.on(elem, 'mouseover', null, noop)) } this._element.focus() @@ -222,7 +222,7 @@ class Dropdown extends BaseComponent { // empty mouseover listeners we added for iOS support if ('ontouchstart' in document.documentElement) { [].concat(...document.body.children) - .forEach(elem => EventHandler.off(elem, 'mouseover', null, noop())) + .forEach(elem => EventHandler.off(elem, 'mouseover', null, noop)) } if (this._popper) { @@ -435,7 +435,7 @@ class Dropdown extends BaseComponent { // empty mouseover listeners we added for iOS support if ('ontouchstart' in document.documentElement) { [].concat(...document.body.children) - .forEach(elem => EventHandler.off(elem, 'mouseover', null, noop())) + .forEach(elem => EventHandler.off(elem, 'mouseover', null, noop)) } if (context._popper) { diff --git a/js/src/tooltip.js b/js/src/tooltip.js index 4fea1c9646e3..a66e1ad41e54 100644 --- a/js/src/tooltip.js +++ b/js/src/tooltip.js @@ -301,7 +301,7 @@ class Tooltip extends BaseComponent { // https://www.quirksmode.org/blog/archives/2014/02/mouse_event_bub.html if ('ontouchstart' in document.documentElement) { [].concat(...document.body.children).forEach(element => { - EventHandler.on(element, 'mouseover', noop()) + EventHandler.on(element, 'mouseover', noop) }) } diff --git a/js/src/util/index.js b/js/src/util/index.js index cc35d8a37c4c..f19d76e036e9 100644 --- a/js/src/util/index.js +++ b/js/src/util/index.js @@ -190,7 +190,7 @@ const findShadowRoot = element => { return findShadowRoot(element.parentNode) } -const noop = () => function () {} +const noop = () => {} const reflow = element => element.offsetHeight diff --git a/js/tests/unit/util/index.spec.js b/js/tests/unit/util/index.spec.js index 41c1ce2b80b4..5d144348e458 100644 --- a/js/tests/unit/util/index.spec.js +++ b/js/tests/unit/util/index.spec.js @@ -477,8 +477,8 @@ describe('Util', () => { }) describe('noop', () => { - it('should return a function', () => { - expect(typeof Util.noop()).toEqual('function') + it('should be a function', () => { + expect(typeof Util.noop).toEqual('function') }) }) From 5d3eed518ea26b1c444ad86e30bb943ff587297e Mon Sep 17 00:00:00 2001 From: Rohit Sharma Date: Sat, 27 Mar 2021 22:51:08 +0530 Subject: [PATCH 2/3] =?UTF-8?q?Dropdown=20=E2=80=94=20Don't=20use=20event?= =?UTF-8?q?=20delegation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- js/src/dropdown.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/js/src/dropdown.js b/js/src/dropdown.js index 3e1ef5fd4f5b..7d5421a56d93 100644 --- a/js/src/dropdown.js +++ b/js/src/dropdown.js @@ -192,7 +192,7 @@ class Dropdown extends BaseComponent { if ('ontouchstart' in document.documentElement && !parent.closest(SELECTOR_NAVBAR_NAV)) { [].concat(...document.body.children) - .forEach(elem => EventHandler.on(elem, 'mouseover', null, noop)) + .forEach(elem => EventHandler.on(elem, 'mouseover', noop)) } this._element.focus() @@ -222,7 +222,7 @@ class Dropdown extends BaseComponent { // empty mouseover listeners we added for iOS support if ('ontouchstart' in document.documentElement) { [].concat(...document.body.children) - .forEach(elem => EventHandler.off(elem, 'mouseover', null, noop)) + .forEach(elem => EventHandler.off(elem, 'mouseover', noop)) } if (this._popper) { @@ -435,7 +435,7 @@ class Dropdown extends BaseComponent { // empty mouseover listeners we added for iOS support if ('ontouchstart' in document.documentElement) { [].concat(...document.body.children) - .forEach(elem => EventHandler.off(elem, 'mouseover', null, noop)) + .forEach(elem => EventHandler.off(elem, 'mouseover', noop)) } if (context._popper) { From e0fd67d1333d132ea5c892cb6029668f5cc87182 Mon Sep 17 00:00:00 2001 From: Rohit Sharma Date: Thu, 8 Apr 2021 01:28:05 +0530 Subject: [PATCH 3/3] Update tests to check for `noop` to be removed --- js/tests/unit/dropdown.spec.js | 5 +++-- js/tests/unit/tooltip.spec.js | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/js/tests/unit/dropdown.spec.js b/js/tests/unit/dropdown.spec.js index b8969be7c7d7..14bf8407bf94 100644 --- a/js/tests/unit/dropdown.spec.js +++ b/js/tests/unit/dropdown.spec.js @@ -1,5 +1,6 @@ import Dropdown from '../../src/dropdown' import EventHandler from '../../src/dom/event-handler' +import { noop } from '../../src/util' /** Test helpers */ import { getFixture, clearFixture, createEvent, jQueryMock } from '../helpers/fixture' @@ -252,7 +253,7 @@ describe('Dropdown', () => { btnDropdown.addEventListener('shown.bs.dropdown', () => { expect(btnDropdown.classList.contains('show')).toEqual(true) expect(btnDropdown.getAttribute('aria-expanded')).toEqual('true') - expect(EventHandler.on).toHaveBeenCalled() + expect(EventHandler.on).toHaveBeenCalledWith(jasmine.any(Object), 'mouseover', noop) dropdown.toggle() }) @@ -260,7 +261,7 @@ describe('Dropdown', () => { btnDropdown.addEventListener('hidden.bs.dropdown', () => { expect(btnDropdown.classList.contains('show')).toEqual(false) expect(btnDropdown.getAttribute('aria-expanded')).toEqual('false') - expect(EventHandler.off).toHaveBeenCalled() + expect(EventHandler.off).toHaveBeenCalledWith(jasmine.any(Object), 'mouseover', noop) document.documentElement.ontouchstart = defaultValueOnTouchStart done() diff --git a/js/tests/unit/tooltip.spec.js b/js/tests/unit/tooltip.spec.js index f9d97e3f7ed4..399f1f22a85b 100644 --- a/js/tests/unit/tooltip.spec.js +++ b/js/tests/unit/tooltip.spec.js @@ -450,7 +450,7 @@ describe('Tooltip', () => { tooltipEl.addEventListener('shown.bs.tooltip', () => { expect(document.querySelector('.tooltip')).not.toBeNull() - expect(EventHandler.on).toHaveBeenCalled() + expect(EventHandler.on).toHaveBeenCalledWith(jasmine.any(Object), 'mouseover', noop) document.documentElement.ontouchstart = undefined done() }) @@ -889,7 +889,7 @@ describe('Tooltip', () => { tooltipEl.addEventListener('hidden.bs.tooltip', () => { expect(document.querySelector('.tooltip')).toBeNull() - expect(EventHandler.off).toHaveBeenCalled() + expect(EventHandler.off).toHaveBeenCalledWith(jasmine.any(Object), 'mouseover', noop) document.documentElement.ontouchstart = undefined done() })