Skip to content

Commit

Permalink
Rebase scrollbar fix (#438)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
cibernox authored Dec 14, 2018
1 parent 3c18048 commit 531758e
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 21 deletions.
1 change: 1 addition & 0 deletions addon/components/basic-dropdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
9 changes: 7 additions & 2 deletions addon/components/basic-dropdown/content.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions addon/templates/components/basic-dropdown.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
44 changes: 43 additions & 1 deletion tests/integration/components/basic-dropdown/content-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,54 @@ module('Integration | Component | basic-dropdown/content', function(hooks) {
await render(hbs`
<div id="destination-el"></div>
<div id="other-div"></div>
{{#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`
<div id="destination-el"></div>
<div id="other-div"></div>
{{#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`
<div id="destination-el"></div>
<div id="other-div"></div>
{{#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 = {
Expand Down
24 changes: 6 additions & 18 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit 531758e

Please sign in to comment.