From 718d88e2b0c6f4599b1a265c80b06a92959616ea Mon Sep 17 00:00:00 2001 From: Konstantin Vyatkin Date: Mon, 4 Nov 2019 20:06:25 -0400 Subject: [PATCH 1/6] scope onMouseDown event capture --- src/scripts/choices.js | 82 +++++++++++++++++----------------------- src/scripts/lib/utils.js | 6 --- 2 files changed, 34 insertions(+), 54 deletions(-) diff --git a/src/scripts/choices.js b/src/scripts/choices.js index f81800c75..7962a8bad 100644 --- a/src/scripts/choices.js +++ b/src/scripts/choices.js @@ -36,7 +36,6 @@ import { strToEl, sortByScore, generateId, - findAncestorByAttrName, isIE11, existsInArray, cloneObject, @@ -1224,7 +1223,11 @@ class Choices { // capture events - can cancel event processing or propagation documentElement.addEventListener('keydown', this._onKeyDown, true); documentElement.addEventListener('touchend', this._onTouchEnd, true); - documentElement.addEventListener('mousedown', this._onMouseDown, true); + this.containerOuter.element.addEventListener( + 'mousedown', + this._onMouseDown, + true, + ); // passive events - doesn't call `preventDefault` or `stopPropagation` documentElement.addEventListener('click', this._onClick, { passive: true }); @@ -1269,41 +1272,27 @@ class Choices { documentElement.removeEventListener('keydown', this._onKeyDown, true); documentElement.removeEventListener('touchend', this._onTouchEnd, true); - documentElement.removeEventListener('mousedown', this._onMouseDown, true); + this.containerOuter.element.removeEventListener( + 'mousedown', + this._onMouseDown, + true, + ); - documentElement.removeEventListener('keyup', this._onKeyUp, { - passive: true, - }); - documentElement.removeEventListener('click', this._onClick, { - passive: true, - }); - documentElement.removeEventListener('touchmove', this._onTouchMove, { - passive: true, - }); - documentElement.removeEventListener('mouseover', this._onMouseOver, { - passive: true, - }); + documentElement.removeEventListener('keyup', this._onKeyUp); + documentElement.removeEventListener('click', this._onClick); + documentElement.removeEventListener('touchmove', this._onTouchMove); + documentElement.removeEventListener('mouseover', this._onMouseOver); if (this._isSelectOneElement) { - this.containerOuter.element.removeEventListener('focus', this._onFocus, { - passive: true, - }); - this.containerOuter.element.removeEventListener('blur', this._onBlur, { - passive: true, - }); + this.containerOuter.element.removeEventListener('focus', this._onFocus); + this.containerOuter.element.removeEventListener('blur', this._onBlur); } - this.input.element.removeEventListener('focus', this._onFocus, { - passive: true, - }); - this.input.element.removeEventListener('blur', this._onBlur, { - passive: true, - }); + this.input.element.removeEventListener('focus', this._onFocus); + this.input.element.removeEventListener('blur', this._onBlur); if (this.input.element.form) { - this.input.element.form.removeEventListener('reset', this._onFormReset, { - passive: true, - }); + this.input.element.form.removeEventListener('reset', this._onFormReset); } this.input.removeEventListeners(); @@ -1573,7 +1562,7 @@ class Choices { } _onMouseDown(event) { - const { target, shiftKey } = event; + const { target } = event; // If we have our mouse down on the scrollbar and are on IE11... if ( this.choiceList.element.contains(target) && @@ -1582,27 +1571,24 @@ class Choices { this._isScrollingOnIe = true; } - if ( - !this.containerOuter.element.contains(target) || - target === this.input.element - ) { + if (target === this.input.element) { return; } - const { activeItems } = this._store; - const hasShiftKey = shiftKey; - const buttonTarget = findAncestorByAttrName(target, 'data-button'); - const itemTarget = findAncestorByAttrName(target, 'data-item'); - const choiceTarget = findAncestorByAttrName(target, 'data-choice'); - - if (buttonTarget) { - this._handleButtonAction(activeItems, buttonTarget); - } else if (itemTarget) { - this._handleItemAction(activeItems, itemTarget, hasShiftKey); - } else if (choiceTarget) { - this._handleChoiceAction(activeItems, choiceTarget); + const item = target.closest('[data-button],[data-item],[data-choice]'); + if (item) { + const hasShiftKey = event.shiftKey; + const { activeItems } = this._store; + const { dataset } = item; + + if ('button' in dataset) { + this._handleButtonAction(activeItems, item); + } else if ('item' in dataset) { + this._handleItemAction(activeItems, item, hasShiftKey); + } else if ('choice' in dataset) { + this._handleChoiceAction(activeItems, item); + } } - event.preventDefault(); } diff --git a/src/scripts/lib/utils.js b/src/scripts/lib/utils.js index bf2508430..225fb5e3d 100644 --- a/src/scripts/lib/utils.js +++ b/src/scripts/lib/utils.js @@ -58,12 +58,6 @@ export const wrap = (element, wrapper = document.createElement('div')) => { return wrapper.appendChild(element); }; -/** - * @param {HTMLElement} el - * @param {string} attr - */ -export const findAncestorByAttrName = (el, attr) => el.closest(`[${attr}]`); - /** * @param {Element} startEl * @param {string} selector From b0e41d49f83cd8e6211232f640b47cf4fed61e56 Mon Sep 17 00:00:00 2001 From: Konstantin Vyatkin Date: Mon, 4 Nov 2019 20:34:26 -0400 Subject: [PATCH 2/6] supercedes #710 --- src/scripts/choices.js | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/scripts/choices.js b/src/scripts/choices.js index 7962a8bad..c7cf1aed5 100644 --- a/src/scripts/choices.js +++ b/src/scripts/choices.js @@ -1561,14 +1561,29 @@ class Choices { this._wasTap = true; } + /** + * Handles mousedown event in capture mode for containetOuter.element + * @param {MouseEvent} event + */ _onMouseDown(event) { const { target } = event; + if (!(target instanceof HTMLElement)) { + return; + } + // If we have our mouse down on the scrollbar and are on IE11... if ( - this.choiceList.element.contains(target) && - isIE11(navigator.userAgent) + isIE11(navigator.userAgent) && + this.choiceList.element.contains(target) ) { - this._isScrollingOnIe = true; + // check if click was on a scrollbar area + const firstChoice = /** @type {HTMLElement} */ (this.choiceList.element + .firstElementChild); + const isOnScrollbar = + this._direction === 'ltr' + ? event.offsetX >= firstChoice.offsetWidth + : event.offsetX < firstChoice.offsetLeft; + this._isScrollingOnIe = isOnScrollbar; } if (target === this.input.element) { @@ -1576,7 +1591,7 @@ class Choices { } const item = target.closest('[data-button],[data-item],[data-choice]'); - if (item) { + if (item instanceof HTMLElement) { const hasShiftKey = event.shiftKey; const { activeItems } = this._store; const { dataset } = item; From 54b14012dfa56b34a7dabf11c781e424c8ffe4a2 Mon Sep 17 00:00:00 2001 From: Konstantin Vyatkin Date: Mon, 4 Nov 2019 20:42:48 -0400 Subject: [PATCH 3/6] make isIE11 a const --- src/scripts/choices.js | 11 ++++++----- src/scripts/lib/utils.js | 7 ------- src/scripts/lib/utils.test.js | 13 ------------- 3 files changed, 6 insertions(+), 25 deletions(-) diff --git a/src/scripts/choices.js b/src/scripts/choices.js index c7cf1aed5..7b5768381 100644 --- a/src/scripts/choices.js +++ b/src/scripts/choices.js @@ -36,12 +36,16 @@ import { strToEl, sortByScore, generateId, - isIE11, existsInArray, cloneObject, diff, } from './lib/utils'; +/** @see {@link http://browserhacks.com/#hack-acea075d0ac6954f275a70023906050c} */ +const IS_IE11 = + '-ms-scroll-limit' in document.documentElement.style && + '-ms-ime-align' in document.documentElement.style; + /** * @typedef {import('../../types/index').Choices.Choice} Choice * @typedef {import('../../types/index').Choices.Item} Item @@ -1572,10 +1576,7 @@ class Choices { } // If we have our mouse down on the scrollbar and are on IE11... - if ( - isIE11(navigator.userAgent) && - this.choiceList.element.contains(target) - ) { + if (IS_IE11 && this.choiceList.element.contains(target)) { // check if click was on a scrollbar area const firstChoice = /** @type {HTMLElement} */ (this.choiceList.element .firstElementChild); diff --git a/src/scripts/lib/utils.js b/src/scripts/lib/utils.js index 225fb5e3d..2f4d5fbe9 100644 --- a/src/scripts/lib/utils.js +++ b/src/scripts/lib/utils.js @@ -179,13 +179,6 @@ export const dispatchEvent = (element, type, customArgs = null) => { return element.dispatchEvent(event); }; -/** - * @param {string} userAgent - * @returns {boolean} - */ -export const isIE11 = userAgent => - !!(userAgent.match(/Trident/) && userAgent.match(/rv[ :]11/)); - /** * @param {array} array * @param {any} value diff --git a/src/scripts/lib/utils.test.js b/src/scripts/lib/utils.test.js index b6d633f09..2366f5635 100644 --- a/src/scripts/lib/utils.test.js +++ b/src/scripts/lib/utils.test.js @@ -9,7 +9,6 @@ import { sanitise, sortByAlpha, sortByScore, - isIE11, existsInArray, cloneObject, dispatchEvent, @@ -202,18 +201,6 @@ describe('utils', () => { }); }); - describe('isIE11', () => { - it('returns whether the given user agent string matches an IE11 user agent string', () => { - const IE11UserAgent = - 'Mozilla/5.0 (Windows NT 6.3; Trident/7.0; rv:11.0) like Gecko'; - const firefoxUserAgent = - 'Mozilla/5.0 (Android 4.4; Mobile; rv:41.0) Gecko/41.0 Firefox/41.0'; - - expect(isIE11(IE11UserAgent)).to.equal(true); - expect(isIE11(firefoxUserAgent)).to.equal(false); - }); - }); - describe('existsInArray', () => { it('determines whether a value exists within given array', () => { const values = [ From 49f251cd0dce2cfcb8966a7d2109695095d548bb Mon Sep 17 00:00:00 2001 From: Konstantin Vyatkin Date: Mon, 4 Nov 2019 21:13:04 -0400 Subject: [PATCH 4/6] scope keydown --- src/scripts/choices.js | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/scripts/choices.js b/src/scripts/choices.js index 7b5768381..ea232faac 100644 --- a/src/scripts/choices.js +++ b/src/scripts/choices.js @@ -1225,8 +1225,12 @@ class Choices { const { documentElement } = document; // capture events - can cancel event processing or propagation - documentElement.addEventListener('keydown', this._onKeyDown, true); documentElement.addEventListener('touchend', this._onTouchEnd, true); + this.containerOuter.element.addEventListener( + 'keydown', + this._onKeyDown, + true, + ); this.containerOuter.element.addEventListener( 'mousedown', this._onMouseDown, @@ -1274,8 +1278,12 @@ class Choices { _removeEventListeners() { const { documentElement } = document; - documentElement.removeEventListener('keydown', this._onKeyDown, true); documentElement.removeEventListener('touchend', this._onTouchEnd, true); + this.containerOuter.element.removeEventListener( + 'keydown', + this._onKeyDown, + true, + ); this.containerOuter.element.removeEventListener( 'mousedown', this._onMouseDown, @@ -1302,13 +1310,13 @@ class Choices { this.input.removeEventListeners(); } + /** + * @param {KeyboardEvent} event + */ _onKeyDown(event) { const { target, keyCode, ctrlKey, metaKey } = event; - if ( - target !== this.input.element && - !this.containerOuter.element.contains(target) - ) { + if (target !== this.input.element) { return; } From cb36fbacf0e821b12b6355620869e8005132a0f3 Mon Sep 17 00:00:00 2001 From: Konstantin Vyatkin Date: Mon, 4 Nov 2019 21:28:18 -0400 Subject: [PATCH 5/6] scope mouseover --- src/scripts/choices.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/scripts/choices.js b/src/scripts/choices.js index ea232faac..061a9a98d 100644 --- a/src/scripts/choices.js +++ b/src/scripts/choices.js @@ -1242,7 +1242,7 @@ class Choices { documentElement.addEventListener('touchmove', this._onTouchMove, { passive: true, }); - documentElement.addEventListener('mouseover', this._onMouseOver, { + this.dropdown.element.addEventListener('mouseover', this._onMouseOver, { passive: true, }); @@ -1293,7 +1293,7 @@ class Choices { documentElement.removeEventListener('keyup', this._onKeyUp); documentElement.removeEventListener('click', this._onClick); documentElement.removeEventListener('touchmove', this._onTouchMove); - documentElement.removeEventListener('mouseover', this._onMouseOver); + this.dropdown.element.removeEventListener('mouseover', this._onMouseOver); if (this._isSelectOneElement) { this.containerOuter.element.removeEventListener('focus', this._onFocus); @@ -1616,13 +1616,12 @@ class Choices { event.preventDefault(); } + /** + * Handles mouseover event over this.dropdown + * @param {MouseEvent} event + */ _onMouseOver({ target }) { - const targetWithinDropdown = - target === this.dropdown || this.dropdown.element.contains(target); - const shouldHighlightChoice = - targetWithinDropdown && target.hasAttribute('data-choice'); - - if (shouldHighlightChoice) { + if (target instanceof HTMLElement && 'choice' in target.dataset) { this._highlightChoice(target); } } From d5dd9cd53bb40802d37f9227d3968c4ea0da041d Mon Sep 17 00:00:00 2001 From: Konstantin Vyatkin Date: Mon, 4 Nov 2019 21:34:05 -0400 Subject: [PATCH 6/6] fix removeEventListener for keyup --- src/scripts/choices.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scripts/choices.js b/src/scripts/choices.js index 061a9a98d..cd3f72aa8 100644 --- a/src/scripts/choices.js +++ b/src/scripts/choices.js @@ -1290,7 +1290,6 @@ class Choices { true, ); - documentElement.removeEventListener('keyup', this._onKeyUp); documentElement.removeEventListener('click', this._onClick); documentElement.removeEventListener('touchmove', this._onTouchMove); this.dropdown.element.removeEventListener('mouseover', this._onMouseOver); @@ -1300,6 +1299,7 @@ class Choices { this.containerOuter.element.removeEventListener('blur', this._onBlur); } + this.input.element.removeEventListener('keyup', this._onKeyUp); this.input.element.removeEventListener('focus', this._onFocus); this.input.element.removeEventListener('blur', this._onBlur);