-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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(tabs): detach tab portal when tab hides from view #8486
Conversation
src/lib/tabs/tab-body.ts
Outdated
if (this._centeringSub && !this._centeringSub.closed) { | ||
this._centeringSub.unsubscribe(); | ||
} | ||
this._centeringSub.unsubscribe(); |
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.
It probably still need to check whether _centeringSub
exists because some people, in their unit tests, destroy a component before calling detectChanges
fixture.whenStable().then(() => { | ||
expect(fixture.nativeElement.textContent).not.toContain('Pizza, fries'); | ||
expect(fixture.nativeElement.textContent).toContain('Peanuts'); | ||
}); |
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.
Expand this to change the tab and re-assert the the others are gone?
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
Marking as P1 since this fixes a regression |
@josephperrott Thanks for this pull request, but shouldn't the tab portal not be detached until the animation is done (after centering rather than before)? The section on "Navigational transitions" in the Material Design Guidelines specifies that in a sibling to sibling transition
Since this merge, it seems that the "moves off-screen" part has been effectively removed. |
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. |
No description provided.