fix(navbar): update ink bar when links change #4897
Conversation
googlebot
added
the
cla: yes
| @@ -37,10 +41,14 @@ export class MdTabNavBar implements AfterContentInit, OnDestroy { | |||
| /** Combines listeners that will re-align the ink bar whenever they're invoked. */ | |||
| private _realignInkBar: Subscription = null; | |||
|
|
|||
| /** Subject that emits when the component has been destroyed. */ | |||
| private _onDestroy = new Subject<void>(); | |||
There was a problem hiding this comment.
Instead of having an internal Subject, why not save the subscription and unsubscribe on destroy instead? On a related note, I wonder whether the _tabLinks wouldn't get completed automatically on destroy.
There was a problem hiding this comment.
Both work, playing with the idea of using this subject for our components' subscriptions. It's an idea Rob is considering making "a thing" that seems to play nice with observables and lets you avoid storing the subscription in another variable.
What do you think about this @jelbourn
jelbourn
added
pr: lgtm
action: merge
andrewseguin
removed
pr: lgtm
action: merge
|
Changed the strategy to tackle another issue - align ink bar when the inner HTML changes. Using mutation observers to know when the content is changed. Can use this for when links are added/removed/moved as well as when their content has changed (and perhaps their width). @jelbourn can you re-review? |
| expect(inkBar.alignToElement).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should re-align the ink bar when the tab labels change the width', done => { |
There was a problem hiding this comment.
Why does this test use the done callback instead of fakeAsync? I actually don't see why either is necessary
There was a problem hiding this comment.
Nothing seemed to be able to trigger the mutation observer (detect changes, flush microtasks, etc). Took a cue from observe-content.spec.ts which uses done => () as well.
|
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 #4737, #4738
When the list of tab links change, the ink bar should adjust to make sure it's positioned under the right element. E.g. tabs are added/removed/moved/content changed