This repository was archived by the owner on Oct 2, 2019. It is now read-only.
fix: close drop-down regardless of stopPropagation() calls
#2003
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As of now, an event-handler on the `document` is responsible for closing the drop-down. A click-event stops bubbling when `stopPropagation()` is called, therefor it breaks the closing of the drop-down. Registering the event-listener in capture phase fixes this bug.
Jefiozie
reviewed
Jun 7, 2017
| // | ||
| // Using the capture phase avoids problems that araise when event.stopPropatagion() | ||
| // is called before the event reaches the `document`. | ||
| $window.document.addEventListener('click', onDocumentClick, true); |
There was a problem hiding this comment.
Why should we not use $document.addEventListener instead of $window.document?
There was a problem hiding this comment.
In uiSelectchoicesDirective.js#L51 $window.document is used, so i wanted to be consistent.
Also it is not possible to add a native addEventListener to the angular $document, which is a jQuery window.document wrapper.
jQuery's aim is browser compability and capture phase is not supported by older IE's. Because Angular only supports IE9+ this should not be an issue.
|
@Jefiozie i responded to your question. |
Jefiozie
approved these changes
Jun 12, 2017
|
Thanks for your quick review and approval ! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bug description
As of now, a click event-handler on the
$documentis responsible for closing an open drop-down.The click-event stops bubbling when
stopPropagation()is called anywhere on the DOM before the$document. This breaks the closing of the drop-down.Proposed solution
Registering the event-listener in the capture phase instead of the bubble phase fixes this bug.
Plunkers
Failing use-case: http://plnkr.co/edit/MWOhIKdUNpoQK5OWTC5G
Fixed use-case: http://plnkr.co/edit/jC2R9QJGUrbzljr0YR1C
Additional information
I tested the examples and they're still working with my proposed solution. Tests are also succeeding, including the one i added.
Angular version used downstream: v1.5.11
angular-ui version used downstream: 0.19.4