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

Add @rootEventType argument and set mousedown as default #1819

Merged
merged 2 commits into from
Jul 31, 2024

Conversation

ArnaudWeyts
Copy link
Contributor

Summary

  • Adds the @rootEventType to be passed on to BasicDropdown
  • Sets the default value of @rootEventType to mousedown
  • Add documentation

Description

I noticed that we set eventType to a default of mousedown for the ember-basic-dropdown trigger (see https://github.com/cibernox/ember-power-select/blob/3aec36878c8b2aa9cf335b7a2220ae448c359fa1/ember-power-select/src/components/power-select.hbs#L46C1-L46C47), but we don't set it for rootEventType, which is the event used when clicking outside of the dropdown to open/close. I believe these should be in sync and the default of mousedown makes sense (it's also how a default HTML select behaves).

We ran into this issue because the click event doesn't propagate when the original element is removed from the DOM. Causing selects to not close properly.

@ArnaudWeyts
Copy link
Contributor Author

Also opened an MR in ember-basic-dropdown to add missing docs: cibernox/ember-basic-dropdown#903

Copy link
Collaborator

@mkszepp mkszepp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@@ -98,6 +98,7 @@ export interface PowerSelectArgs {
triggerClass?: string;
ariaInvalid?: string;
eventType?: string;
rootEventType?: string;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should change to rootEventType?: 'click' | 'mousedown';, because only this two types are allowed.

But the better way is that we export this types in ebd, so this is always syned.
I have also discoverd that in ebd we have one time delared as string and one strict to click/mousedown. I will look to fix it in ebd and than, when we release the ebd we do update the type also here

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type changed in #1846

@mkszepp mkszepp merged commit 3fbf150 into cibernox:master Jul 31, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants