From 531758e92adca643130e0ac75f9bdf6d6d5a763a Mon Sep 17 00:00:00 2001 From: Miguel Camba Date: Fri, 14 Dec 2018 21:49:21 +0100 Subject: [PATCH] Rebase scrollbar fix (#438) * Add config option to fix scrollbar clicking As mentioned in #409, clicking a scrollbar will close an open basic-dropdown. This is due to mousedown being used instead of click. To not break existing behaviour, I've added a config option which can be passed to the parent container (basic-dropdown) or the content component. * Address PR comments and change property name This changes the approach from using a boolean flag for click type to passing in the click event type instead. Additionally added comment to change to 'click' in 2.0 * Change tests to use rootEventType as well Tests were still using the previous flag. * Removed remaining useClickEvent flag I missed this in an earlier pass. * Add default rootEventType to onClose test The default rootEventType is specified in the main component and passed down, it needs to be bound in this test for the test to pass. * Reinstall dependencies --- addon/components/basic-dropdown.js | 1 + addon/components/basic-dropdown/content.js | 9 +++- addon/templates/components/basic-dropdown.hbs | 1 + .../components/basic-dropdown/content-test.js | 44 ++++++++++++++++++- yarn.lock | 24 +++------- 5 files changed, 58 insertions(+), 21 deletions(-) diff --git a/addon/components/basic-dropdown.js b/addon/components/basic-dropdown.js index 2d76917d..f82f17f9 100644 --- a/addon/components/basic-dropdown.js +++ b/addon/components/basic-dropdown.js @@ -41,6 +41,7 @@ export default Component.extend({ renderInPlace: fallbackIfUndefined(false), verticalPosition: fallbackIfUndefined('auto'), // above | below horizontalPosition: fallbackIfUndefined('auto'), // auto-right | right | center | left + rootEventType: fallbackIfUndefined('mousedown'), // TODO: Change default event type to "click" in 2.0 matchTriggerWidth: fallbackIfUndefined(false), triggerComponent: fallbackIfUndefined('basic-dropdown/trigger'), contentComponent: fallbackIfUndefined('basic-dropdown/content'), diff --git a/addon/components/basic-dropdown/content.js b/addon/components/basic-dropdown/content.js index 124a6055..ba8164ed 100644 --- a/addon/components/basic-dropdown/content.js +++ b/addon/components/basic-dropdown/content.js @@ -146,7 +146,9 @@ export default Component.extend({ let dropdown = this.get('dropdown'); this.triggerElement = this.triggerElement || document.querySelector(`[data-ebd-id=${dropdown.uniqueId}-trigger]`); this.dropdownElement = document.getElementById(this.dropdownId); - document.addEventListener('mousedown', this.handleRootMouseDown, true); + const rootEventType = this.get('rootEventType'); + document.addEventListener(rootEventType, this.handleRootMouseDown, true); + if (this.get('isTouchDevice')) { document.addEventListener('touchstart', this.touchStartHandler, true); document.addEventListener('touchend', this.handleRootMouseDown, true); @@ -360,7 +362,10 @@ export default Component.extend({ this.removeScrollHandling(); this.scrollableAncestors = []; this.stopObservingDomMutations(); - document.removeEventListener('mousedown', this.handleRootMouseDown, true); + + const rootEventType = this.get('rootEventType'); + document.removeEventListener(rootEventType, this.handleRootMouseDown, true); + if (this.get('isTouchDevice')) { document.removeEventListener('touchstart', this.touchStartHandler, true); document.removeEventListener('touchend', this.handleRootMouseDown, true); diff --git a/addon/templates/components/basic-dropdown.hbs b/addon/templates/components/basic-dropdown.hbs index 97200d9c..59190cfb 100644 --- a/addon/templates/components/basic-dropdown.hbs +++ b/addon/templates/components/basic-dropdown.hbs @@ -18,6 +18,7 @@ hPosition=(readonly hPosition) renderInPlace=(readonly renderInPlace) preventScroll=(readonly preventScroll) + rootEventType=(readonly rootEventType) vPosition=(readonly vPosition) destination=(readonly destination) top=(readonly top) diff --git a/tests/integration/components/basic-dropdown/content-test.js b/tests/integration/components/basic-dropdown/content-test.js index 1152c748..cfdfb5fd 100644 --- a/tests/integration/components/basic-dropdown/content-test.js +++ b/tests/integration/components/basic-dropdown/content-test.js @@ -103,12 +103,54 @@ module('Integration | Component | basic-dropdown/content', function(hooks) { await render(hbs`
- {{#basic-dropdown/content dropdown=dropdown destination='destination-el'}}Lorem ipsum{{/basic-dropdown/content}} + {{#basic-dropdown/content rootEventType='mousedown' dropdown=dropdown destination='destination-el'}}Lorem ipsum{{/basic-dropdown/content}} `); await click('#other-div'); }); + test('Specifying the rootEventType as click will not close a component if it is opened', async function(assert) { + assert.expect(0); + this.dropdown = { + uniqueId: 'e123', + isOpen: true, + actions: { + close() { + assert.ok(true, 'The close action should not be called'); + }, + reposition() {} + } + }; + await render(hbs` +
+
+ {{#basic-dropdown/content rootEventType='click' dropdown=dropdown destination='destination-el'}}Lorem ipsum{{/basic-dropdown/content}} + `); + + await triggerEvent('#other-div', 'mousedown'); + }); + + test('Specifying the rootEventType as mousedown will close a component if it is opened', async function(assert) { + assert.expect(1); + this.dropdown = { + uniqueId: 'e123', + isOpen: true, + actions: { + close() { + assert.ok(true, 'The close action gets called'); + }, + reposition() {} + } + }; + await render(hbs` +
+
+ {{#basic-dropdown/content rootEventType='mousedown' dropdown=dropdown destination='destination-el'}}Lorem ipsum{{/basic-dropdown/content}} + `); + + await triggerEvent('#other-div', 'mousedown'); + }); + test('Clicking anywhere inside the dropdown content doesn\'t invoke the close action', async function(assert) { assert.expect(0); this.dropdown = { diff --git a/yarn.lock b/yarn.lock index e790f3f8..3061bb3c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1765,7 +1765,7 @@ broccoli-clean-css@^1.1.0: inline-source-map-comment "^1.0.5" json-stable-stringify "^1.0.0" -broccoli-concat@^3.2.2, broccoli-concat@^3.5.1, broccoli-concat@^3.7.1: +broccoli-concat@^3.2.2, broccoli-concat@^3.7.1, broccoli-concat@^3.7.3: version "3.7.3" resolved "https://registry.yarnpkg.com/broccoli-concat/-/broccoli-concat-3.7.3.tgz#0dca01311567ffb13180e6b4eb111824628e4885" dependencies: @@ -3072,9 +3072,9 @@ ember-cli-version-checker@^2.0.0, ember-cli-version-checker@^2.1.0, ember-cli-ve resolve "^1.3.3" semver "^5.3.0" -ember-cli@~3.5.0: - version "3.5.1" - resolved "https://registry.yarnpkg.com/ember-cli/-/ember-cli-3.5.1.tgz#a1c7295eed935726891d40a81e0cc389f2d15fdf" +ember-cli@~3.6.0: + version "3.6.0" + resolved "https://registry.yarnpkg.com/ember-cli/-/ember-cli-3.6.0.tgz#9d0e3ca165eb56ba904900d5133041d3f8410868" dependencies: amd-name-resolver "^1.2.0" babel-plugin-transform-es2015-modules-amd "^6.24.1" @@ -3084,7 +3084,7 @@ ember-cli@~3.5.0: broccoli-amd-funnel "^2.0.1" broccoli-babel-transpiler "^6.5.0" broccoli-builder "^0.18.14" - broccoli-concat "^3.5.1" + broccoli-concat "^3.7.3" broccoli-config-loader "^1.0.1" broccoli-config-replace "^1.1.2" broccoli-debug "^0.6.4" @@ -3114,7 +3114,7 @@ ember-cli@~3.5.0: ember-cli-preprocess-registry "^3.1.2" ember-cli-string-utils "^1.1.0" ensure-posix-path "^1.0.2" - execa "^0.10.0" + execa "^1.0.0" exit "^0.1.2" express "^4.16.3" filesize "^3.6.1" @@ -3639,18 +3639,6 @@ exec-sh@^0.3.2: version "0.3.2" resolved "https://registry.yarnpkg.com/exec-sh/-/exec-sh-0.3.2.tgz#6738de2eb7c8e671d0366aea0b0db8c6f7d7391b" -execa@^0.10.0: - version "0.10.0" - resolved "https://registry.yarnpkg.com/execa/-/execa-0.10.0.tgz#ff456a8f53f90f8eccc71a96d11bdfc7f082cb50" - dependencies: - cross-spawn "^6.0.0" - get-stream "^3.0.0" - is-stream "^1.1.0" - npm-run-path "^2.0.0" - p-finally "^1.0.0" - signal-exit "^3.0.0" - strip-eof "^1.0.0" - execa@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/execa/-/execa-1.0.0.tgz#c6236a5bb4df6d6f15e88e7f017798216749ddd8"