Skip to content

fix(drawer): unsubscribe #13378

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 6, 2018
Merged

fix(drawer): unsubscribe #13378

merged 1 commit into from
Dec 6, 2018

Conversation

@erhise
Copy link
Contributor

@erhise erhise commented Oct 1, 2018

make sure subscription on fromEvent observable is unsubscribed when the
component is destroyed

@erhise erhise requested a review from mmalerba as a code owner October 1, 2018 15:42
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Oct 1, 2018
crisbeto
crisbeto reviewed Oct 1, 2018
View reviewed changes
@erhise erhise force-pushed the drawer-component-fix branch from be2df73 to 4ab8a42 Compare October 6, 2018 21:33
crisbeto
crisbeto approved these changes Oct 12, 2018
View reviewed changes
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM

@crisbeto crisbeto added pr: lgtm action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release merge: fix commit message When the PR is merged, rewrites/fixups of the commit messages are needed labels Oct 12, 2018
@mmalerba mmalerba changed the title (fix): make sure no leaking subscription fix(sidenav): make sure no leaking subscription Oct 12, 2018
@ngbot
Copy link

ngbot bot commented Oct 19, 2018

Hi @erhise! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

@erhise erhise force-pushed the drawer-component-fix branch from 4ab8a42 to b983d65 Compare October 30, 2018 15:27
@erhise erhise changed the title fix(sidenav): make sure no leaking subscription fix(drawer): unsubscribe Oct 30, 2018
@vivian-hu-zz
Copy link
Contributor

vivian-hu-zz commented Nov 7, 2018

@erhise, this PR has passed google test, can you resolve the conflict and push again

@erhise
Copy link
Contributor Author

erhise commented Nov 12, 2018

@vivian-hu the merge conflict was resolved but the ci-tests seem to have failed due to other issue? Is it possible to do retry-build? Otherwise let me know and I can push again.

@josephperrott
Copy link
Member

josephperrott commented Nov 20, 2018

Go ahead and rebase and push again since our CI on master is currently green.

@josephperrott josephperrott removed the action: merge The PR is ready for merge by the caretaker label Nov 20, 2018
@erhise
fix(drawer): unsubscribe
35a5b47 
make sure subscription on fromEvent observable is unsubscribed when the
component is destroyed
@erhise erhise force-pushed the drawer-component-fix branch from b983d65 to 35a5b47 Compare November 25, 2018 12:28
@sheiidan
Copy link

sheiidan commented Nov 27, 2018

Can you please push this fix? 10x

This was referenced Nov 27, 2018
Closed
Open
@mmalerba mmalerba added the action: merge The PR is ready for merge by the caretaker label Dec 3, 2018
@mmalerba mmalerba merged commit f7dcc27 into angular:master Dec 6, 2018
@erhise erhise deleted the drawer-component-fix branch December 8, 2018 14:53
mmalerba pushed a commit that referenced this pull request Dec 10, 2018
@erhise @mmalerba
fix(drawer): ensure all observables are unsubscribed (#13378)
beed6be 
make sure subscription on fromEvent observable is unsubscribed when the
component is destroyed
josephperrott pushed a commit to josephperrott/components that referenced this pull request Jan 14, 2019
@erhise @josephperrott
fix(drawer): ensure all observables are unsubscribed (angular#13378)
bd777c4 
make sure subscription on fromEvent observable is unsubscribed when the
component is destroyed
@angular-automatic-lock-bot
Copy link

angular-automatic-lock-bot bot commented Sep 10, 2019

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@crisbeto crisbeto crisbeto approved these changes

@mmalerba mmalerba Awaiting requested review from mmalerba

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement merge: fix commit message When the PR is merged, rewrites/fixups of the commit messages are needed target: patch This PR is targeted for the next patch release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@erhise @vivian-hu-zz @josephperrott @sheiidan @crisbeto @googlebot @mmalerba