Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using the browser scrollbar closes the dropdown #409

Open
blagh opened this issue May 15, 2018 · 4 comments
Open

Using the browser scrollbar closes the dropdown #409

blagh opened this issue May 15, 2018 · 4 comments
Assignees

Comments

@blagh
Copy link

blagh commented May 15, 2018

Although you can still scroll the page with a mouse scrollwheel for example, it's a bit of a usability issue if you can't also use the scrollbars built into the browser.

I've added a gif to illustrate -- this is more of a problem with bigger dropdowns.

@cibernox
Copy link
Owner

I think is unavoidable really as clicking outside the component closes it. You can make the component stay open by returning false from the onClose action (see http://ember-basic-dropdown.com/docs/dropdown-events).

That action will receive the mousedown event as second argument, but i don't think there is a way to detect the user has clicked in a scroll bar. If I'm wrong please tell me.
I don't know if it's possible to detect that the user clicked

@blagh
Copy link
Author

blagh commented May 15, 2018

I once thought as you did, but I happened to notice that drop.js does not have this issue -- it looks to be because rather than binding to document.mousedown to detect a mouse event outside the popover, they bind to document.click. I'm speculating a little, but it seems like the scrollbar doesn't fire a document.click event because the browser has already handled it.

document.mousedown seems to be closer to the metal though. Just curious: were there edge cases that you were avoiding by using document.mousedown instead of document.click?

@cibernox
Copy link
Owner

I see. The reason why this component uses onmousedown is because it was initially extracted from Ember Power Select, and selects open and close on mousedown, not on click. Since this component is more generic than a select opening on click should, at least, be an allowed option which would also fix your complaint.

I'm on holidays right now, but it seems like a feature request.

@cibernox cibernox self-assigned this May 16, 2018
@blagh
Copy link
Author

blagh commented May 16, 2018

By all means, have a good holiday! Thanks for checking in, in the meanwhile 😁

k-fish added a commit to k-fish/ember-basic-dropdown that referenced this issue Aug 29, 2018
As mentioned in cibernox#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.
k-fish added a commit to k-fish/ember-basic-dropdown that referenced this issue Aug 29, 2018
As mentioned in cibernox#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.
cibernox added a commit that referenced this issue Dec 14, 2018
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants