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

Fix popovers hide() being called twice #2195

Merged
merged 5 commits into from
Apr 28, 2022

Conversation

RasmusKjeldgaard
Copy link
Collaborator

Which issue does this PR close?

This PR closes #2180

What is the new behavior?

The hide functionality has been refactored a bit to ensure that the element is in fact showing, before doing hide logic.

Furthermore, the backdropClick listener was removed, because it was not functional, as described in the commit message:

The event listener was not called because dropdowns onBlur event was
always fired first, closing the dropdown including the popover
component. In fact it was only called when clicking inside the dropdown
because onBlur is not fired then - and thus it is useless at this point.

Components using popover (dropdown only right now) should take care of
calling hide() at the appropriate time themselves, e.g. in onBlur.

Does this PR introduce a breaking change?

  • Yes
  • No

Are there any additional context?

Checklist:

The following tasks should be carried out in sequence in order to follow the process of contributing correctly.

Reminders

  • Make sure you have implemented tests following the guidelines in: "The good: Test".
  • Make sure you have updated the cookbook with examples and showcases (for bug fixes, enhancements & new components).

Review

  • Do a self-review.
  • Request that the changes are code-reviewed
  • Request that the changes are UX reviewed (only necessary if your PR introduces visual changes)

When the pull request has been approved it will be automatically merged to stable via automerge.

The event listener was not called because dropdowns onBlur event was
always fired first, closing the dropdown including the popover
component. In fact it was only called when clicking inside the dropdown
because onBlur is not fired then - and thus it is useless at this point.

Components using popover (dropdown only right now) should take care of
calling hide() at the appropriate time themselves, e.g. in onBlur.
@github-actions github-actions bot temporarily deployed to pr-2180-popover-hide-called-twice April 27, 2022 11:12 Inactive
@RasmusKjeldgaard
Copy link
Collaborator Author

@andershessellund has checked that this works in the target application

Copy link
Contributor

@MadsBuchmann MadsBuchmann left a comment

Choose a reason for hiding this comment

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

teeny tiny comment - otherwise lgtm!

@github-actions github-actions bot temporarily deployed to pr-2180-popover-hide-called-twice April 27, 2022 13:21 Inactive
@RasmusKjeldgaard RasmusKjeldgaard enabled auto-merge (squash) April 27, 2022 15:24
@RasmusKjeldgaard RasmusKjeldgaard merged commit 4e77187 into main Apr 28, 2022
@RasmusKjeldgaard RasmusKjeldgaard deleted the bug/2180-popover-hide-called-twice branch April 28, 2022 12:26
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.

[BUG] popovers hide() function is called twice on dropdown-item click
2 participants