-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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(autocomplete): reopening closed autocomplete when coming back to tab #12372
Conversation
/** | ||
* Whether the autocomplete can open the next time it is focused. Used to prevent a focused, | ||
* closed autocomplete from being reopened if the user switches to another tab and then | ||
* comes back. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarify "browser tab" to disambiguate from a tabs component?
|
||
ngOnDestroy() { | ||
if (typeof window !== 'undefined') { | ||
window.removeEventListener('blur', this._windowBlurHandler); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be a good use for the Page Lifecycle API... someday.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The browser support isn't too horrible tbh, but I think that this is case is simple enough that we can get away with the event.
* Event handler for when the window is blurred. Needs to be an | ||
* arrow function in order to preserve the context. | ||
*/ | ||
private _windowBlurHandler = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this up to live next to the other class properties?
*/ | ||
private _windowBlurHandler = () => { | ||
this._canOpenOnNextFocus = | ||
document.activeElement !== this._element.nativeElement || this.panelOpen; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment that explains the rationale behind this logic?
a2bab41
to
b87f005
Compare
Feedback has been addressed @jelbourn. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hi @crisbeto! This PR has merge conflicts due to recent upstream merges. |
Fixes a closed autocomplete being reopened, if the user moves to another tab and coming back to the current one, while the input is still focused. Fixes angular#12337.
b87f005
to
2094132
Compare
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes a closed autocomplete being reopened, if the user moves to another tab and coming back to the current one, while the input is still focused.
Fixes #12337.