Skip to content
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

routerLinkActive not updating when routerLink changed #18469

Closed
7 tasks
Simon-Briggs opened this issue Aug 2, 2017 · 28 comments
Closed
7 tasks

routerLinkActive not updating when routerLink changed #18469

Simon-Briggs opened this issue Aug 2, 2017 · 28 comments

Comments

@Simon-Briggs
Copy link

Simon-Briggs commented Aug 2, 2017

I have a routerLink URL that can change over time, but when the URL changes, routerLinkActive is not re-evaluated.

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ x ] Bug report  
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

Change a routerLink URL to match/not match the current URL. routerLinkActive class is not applied/removed.

Expected behavior

routerLinkActive is re-evaluated when its corresponding routerLink is updated.

Minimal reproduction of the problem with instructions

Click "link a" or "link b" to make them active, then click the switchLinks button to change the URLs. Nothing changes.
http://plnkr.co/edit/DHn4my9uAKJSIQSTIm1g?p=preview

What is the motivation / use case for changing the behavior?

I have multiple departments managed by the same UI, with department name as a variable in the URL. When the user switches departments, and the on screen links or URL is changed, the UI doesn't update the routerLinkActive.

I think it's because ContentChildren.changes doesn't fire if the contents of a directive is changed, only when directives are added or removed.:
https://github.com/angular/angular/blob/master/packages/router/src/directives/router_link_active.ts#L106

Maybe it's too bad for performance to fix this?

Environment

Angular version: 4.3
Mac

Browser:

  • [x ] Chrome (desktop) version 59
  • Chrome (Android) version XX
  • Chrome (iOS) version XX
  • Firefox version XX
  • Safari (desktop) version XX
  • Safari (iOS) version XX
  • IE version XX
  • Edge version XX
@bennadel
Copy link

I just ran into this as well. I have a tab setup where the tab links to a child like:

<a routerLink="/tabs/{{ id }}/build" routerLinkActive="on">build</a>

... and if I navigate from /tabs/1/build to /tabs/2/build, the "build" tab active-class turns off, despite the fact that the URL changes to reflect the new id. Note that it doesn't matter how the navigation takes place: performing it programmatically or just manually editing the URL -- the effect is the same.

@bennadel
Copy link

I think I see what is happening. I added a break-point in the algorithm that determines if the routerLink should be active; and, it looks like the local urlTree isn't actually updated at the point that the NavigationEnd event is evaluated. In the following screenshot, I've navigated to ..../1/screens/1/....; but, the routerLink urlTree is still checking against the previous url, ..../1/screens/2/....:

2017-11-18_06-16-07

Perhaps there is a race condition with the QueryList<RouterLink> in the RouterLinkActive directive - in which the DOM isn't updated yet, at the time the NavigationEnd event has fired.

@bennadel
Copy link

I have a hack that seems to work. After looking at the source code, it looks like this.update() is also called any time the inputs are changed. So, if you add an routerLinkActiveOptions property and give it a throw-away value that is changed based on the URL, then the inputs will technically change. Example:

<a
routerLink="/foo/:id/bar"
routerLinkActive="on"
[routerLinkActiveOptions]="{ exact: false, __change_detection_hack__: id }"> Link </a>

Here, the __change_detection_hack__ key will change when the id changes (ie, after navigation has completed and the template is reconciled), which will cause the inputs to change, which will cause the this.update() method to run. And, at that point, the QueryList<RouterLink> should be in the proper state.

It's janky, but it works as a stop-gap.

@bennadel
Copy link

I wrote my example up, in case it helps anyone else - https://www.bennadel.com/blog/3375-forcing-routerlinkactive-to-update-using-an-inputs-hack-in-angular-5-0-2.htm

@jorroll
Copy link
Contributor

jorroll commented Jan 11, 2018

Thanks for sharing your temporary fix!

@ngbot ngbot bot added this to the Backlog milestone Jan 23, 2018
@simeyla
Copy link

simeyla commented Feb 9, 2018

Great fix @bennadel . Is this the only way anybody has found? Is there nothing like router.refresh() or router.updateChangeDetection() that would have the same effect? And do you know anything about when this is going to be 'fixed' for real.

Also wanted to make sure the following similar scenario will be handled - and can confirm that your fix works works even if you have a dynamic routerLink from an observable.

I have several tabs with a shared router outlet for the 'current customer' - eg. 'Order / History / Notes'.
The order tab shows a single order chosen from a list so I have an orderRouteLink$ observable which emits something like [order, '50001'] when the current order changes. Of course the same thing happens here and the link isn't made active when switching the [routerLink] to the already-active current route.

Fortunately I was able to push (orderRouteLink$ | async) into your hack and it correctly triggers an update every time orderRouteLink$ emits a new route.

        <a mat-tab-link
           [routerLink]="orderRouteLink$ | async"
           [routerLinkActiveOptions]="{ exact: false, __change_detection_hack__: (orderRouteLink$ | async) }"

           routerLinkActive #rla="routerLinkActive"
           [active]="rla.isActive">
            Order #{{ dataService.orderId.value}}
        </a>

orderRouteLink$ is just a BehaviorSubject:

   orderRouteLink$ = new BehaviorSubject<any[] | string>([]);

@scott-ho
Copy link

scott-ho commented Aug 8, 2018

any progress?

@scott-ho
Copy link

scott-ho commented Aug 8, 2018

I have to write a dirty directive as a work around to force the update

import {
  AfterContentInit,
  ContentChildren,
  Directive,
  Input,
  OnChanges,
  QueryList,
  SimpleChanges,
} from '@angular/core';
import { RouterLinkActive } from '@angular/router';

@Directive({
  selector: '[routerLink]',
})
export class RouterLinkDirective implements OnChanges, AfterContentInit {
  @Input() routerLink: any[] | string;

  @ContentChildren(RouterLinkActive, { descendants: true })
  linkActives !: QueryList<RouterLinkActive>;

  ngOnChanges(changes: SimpleChanges) {
    if (changes.routerLink && this.linkActives && this.linkActives.first) {
      this.linkActives.first.ngOnChanges(null);
    }
  }

  ngAfterContentInit() {
    this.linkActives.changes.subscribe(_ => this.linkActives.first.ngOnChanges(null));
  }
}

LeResKP added a commit to LeResKP/git-ng-web that referenced this issue Sep 24, 2018
@vvolodin
Copy link

Can't believe that this issue is more than a year old. Such basic functionality and it's broken.
Thanks a lot @bennadel for the workaround, it works great!

@nionita
Copy link

nionita commented Oct 7, 2018

Hi, I am a beginner in TS and Angular, so please be patient if I write complete nonsense, but while trying to understand what the problem is with routerLinkActive I found this in file router/src/directives/router_link_active.ts:

private update(): void { if (!this.links || !this.linksWithHrefs || !this.router.navigated) return; ...

So if one of the lists is empty, I return. I must have both this.links and this.linksWithHrefs non-empty to not return, i.e. to go on and really update.

Isn't this already a bug?

@Wildcorsair
Copy link

I am using Angular 6.0.3 and this solve is working for me. Thank you @bennadel.

@arjunpanicker
Copy link

Thank you so much @bennadel.

I was facing this issue in Angular 5 also and did not know what to do. You saved my day.

@leomayer
Copy link

Since I was facing the same issue like #9825 - which I did not know I made a code sample on stackblitz

As seen in the sample as long as userName is unset the link is not evaluted properly. A reevaluation does NOT help.

@mackelito
Copy link

Another hack for this would be to add markForCheck() in the component on router NavigationEnd

import { Router, NavigationEnd } from '@angular/router';
import { Component, Input, OnInit, ChangeDetectionStrategy, ChangeDetectorRef } from '@angular/core';

  constructor(
    private router: Router,
    private cd: ChangeDetectorRef
  ) {}

  ngOnInit() {
    this.router.events.pipe(
      filter(evt => evt instanceof NavigationEnd)
    ).subscribe(() => {
      this.cd.markForCheck();
    });
  }

@miki995
Copy link

miki995 commented Feb 11, 2019

Hello Angular guys, @vsavkin, @petebacondarwin, @vicb, @IgorMinar, @tbosch, @gkalpak.

Are you going to fix this once for all?

Developers should not do hacks for such a basic functionality :D

Best, Miroslav

@bdairy
Copy link

bdairy commented Apr 23, 2019

@scott-ho this.linkActives is allways undefined,, any idea??

<a
*ngFor="let item of (menuItems)"
[routerLink]="['/' + item.state]"
routerLinkActive="active" >{{item.name}}

@chriszrc
Copy link

ngOnInit() {
this.router.events.pipe(
filter(evt => evt instanceof NavigationEnd)
).subscribe(() => {
this.cd.markForCheck();
});
}

@mackelito probably best to use a "take(1)" in the rxjs pipe here as well, since we really only to check this once, since after the first load the routerLinkActive directive works just fine-

@ah3eyy
Copy link

ah3eyy commented Jun 9, 2019

a simple solution is to subscribe to query.params and route params

this.route.queryParams.subscribe(queryParams => {
// do something with the query params
});

this.route.params.subscribe(routeParams => {
  this.loading = true;
  this.loadingproducts(routeParams.id);
});

the above work for me perfectly

@JamesDougherty
Copy link

Hello - Any update on this issue? It's been over two years since the initial report and there is still no fix from what I can tell.

I was using the router.isActive method and I believe these are related. I would have to click the link twice for it to activate since the router wasn't updated until the navigation ended, which caused the router to report the previous link as being active. For instance, if I had links A, B and C (with A being active) and I clicked C then A would stay active, but page C would load fine. If I were to then click B then C would become active and page B would load fine. It was very odd.

I needed an immediate update since this was used for a side navigation component. Each side navigation item is a component as well. So to solve this I send the current route to the layout service as soon as an item is clicked. Then instead of calling router.isActive I just simply compare the current route stored in the service to the route I pass the main side navigation component.

It's not the most ideal, but it worked for my situation where the others did not. It's simple, can be easily modified to survive a browser refresh, etc.

@bgever
Copy link

bgever commented Dec 9, 2019

A false positive of this bug could be that when you pass in a router link of type any[] as the url arg to isActive() it will always return true when the exact arg is set to false.

In that case, make sure to first convert the url arg to an UrlTree object with router.createUrlTree(). LFMF. 😅

@BeejeeDS
Copy link

Still having this issue in Angular 9

@maartentibau
Copy link
Contributor

If I got the time I'm gonna try to dig into this issue, it's frustration that after almost 2 years this is still in the code.

Now you can work around it with just some proper CSS. Meaning that your element has a normal state which has a class and the "active" state needs 2 classes to be present on the elment.

Like this

<a class="nav-link" routerLink="/user/bob" routerLinkActive="nav-link-active">Bob</a>
.nav-link {
  color: blue;

  &.nav-link-active {
    color: red;
  }
}

@atscott
Copy link
Contributor

atscott commented May 27, 2020

Confirmed in v9 with repro: https://stackblitz.com/edit/angular-ivy-2wajtk?file=src%2Fapp%2Fapp.module.ts

Duplicate of #13865, though this one is a more general case so I'll close 13865 and keep this open.

@MurhafSousli
Copy link

MurhafSousli commented Jun 10, 2020

You can use the following as well

// MatTabNav reference
@ViewChild(MatTabNav) nav: MatTabNav;

this.subscriber = this.router.events.pipe(
  filter((event: RouterEvent) => event instanceof NavigationEnd),
  tap(() => this.nav.updateActiveLink())
).subscribe();

@cumtwwei
Copy link

cumtwwei commented Jun 14, 2020

i use angular9 with ng-zorro, and the routerLink not work some time.
finally, remove tag '<a>', only use 'nz-menu-item' with '<li>'.
like blow:

<li nz-menu-item *ngFor="let item of subItem.sub" [nzSelected]="item.active" [routerLink]="[item.url]" [queryParams]="item.queryParams" routerLinkActive="active" [routerLinkActiveOptions]="{ exact: false, __change_detection_hack__: (item.url) }" (click)="printUrl(subItem.url);"> <span class="sub-menu-title"> <i class="icon circle"></i>{{item.title}} </span> </li>

@AffeCode
Copy link

AffeCode commented Aug 5, 2020

I managed to get it working in Angular 10 with ng-zorro without any of the hacks presented here. Instead of having links that will change value sometimes I created an observable emitting an array of objects with the information needed to generate my links, and then I iterate over that array in the html. Then it hightlights the links correctly after they change. I suspect that it works because I don't use a trackBy function, so the links are completely re-rendered every time the observable emits.
Here's a code sample:

<li *ngFor="let item of items$ | async" nz-menu-item nzMatchRouter>
   <a [routerLink]="item.url">
      <i nz-icon [nzType]="item.icon" nzTheme="outline"></i>
      <span>
         {{ item.label }}
      </span>
   </a>
</li>

@JamesDougherty
Copy link

@DK8ALREC That's great you found another solution and I'm sure it will work great for those already using ng-zorro. However, for those not using ng-zorro it would be a massive dependency for something so trivial. It would be nice if the Angular team can just get an official patch for this already.

On a side note, this issue hit it's 3rd birthday a couple of days ago! Happy 3rd birthday #18469. 🥳 🎈 🍰

I managed to get it working in Angular 10 with ng-zorro without any of the hacks presented here. Instead of having links that will change value sometimes I created an observable emitting an array of objects with the information needed to generate my links, and then I iterate over that array in the html. Then it hightlights the links correctly after they change. I suspect that it works because I don't use a trackBy function, so the links are completely re-rendered every time the observable emits.

atscott added a commit to atscott/angular that referenced this issue Aug 5, 2020
…nks change

This commit introduces a new subscription in the `routerLinkActive` directive which triggers an update
when any of its associated routerLinks have changes. `RouterLinkActive` not only needs to know when
links are added or removed, but it also needs to know about if a link it already knows about
changes in some way.

Quick note that `from...mergeAll` is used instead of just a simple
`merge` (or `scheduled...mergeAll`) to avoid introducing new rxjs
operators in order to keep bundle size down.

Fixes angular#18469
atscott added a commit to atscott/angular that referenced this issue Aug 5, 2020
…nks change

This commit introduces a new subscription in the `routerLinkActive` directive which triggers an update
when any of its associated routerLinks have changes. `RouterLinkActive` not only needs to know when
links are added or removed, but it also needs to know about if a link it already knows about
changes in some way.

Quick note that `from...mergeAll` is used instead of just a simple
`merge` (or `scheduled...mergeAll`) to avoid introducing new rxjs
operators in order to keep bundle size down.

Fixes angular#18469
atscott added a commit to atscott/angular that referenced this issue Aug 5, 2020
…nks change

This commit introduces a new subscription in the `routerLinkActive` directive which triggers an update
when any of its associated routerLinks have changes. `RouterLinkActive` not only needs to know when
links are added or removed, but it also needs to know about if a link it already knows about
changes in some way.

Quick note that `from...mergeAll` is used instead of just a simple
`merge` (or `scheduled...mergeAll`) to avoid introducing new rxjs
operators in order to keep bundle size down.

Fixes angular#18469
atscott added a commit to atscott/angular that referenced this issue Aug 5, 2020
…nks change

This commit introduces a new subscription in the `routerLinkActive` directive which triggers an update
when any of its associated routerLinks have changes. `RouterLinkActive` not only needs to know when
links are added or removed, but it also needs to know about if a link it already knows about
changes in some way.

Quick note that `from...mergeAll` is used instead of just a simple
`merge` (or `scheduled...mergeAll`) to avoid introducing new rxjs
operators in order to keep bundle size down.

Fixes angular#18469
atscott added a commit to atscott/angular that referenced this issue Aug 5, 2020
…nks change

This commit introduces a new subscription in the `routerLinkActive` directive which triggers an update
when any of its associated routerLinks have changes. `RouterLinkActive` not only needs to know when
links are added or removed, but it also needs to know about if a link it already knows about
changes in some way.

Quick note that `from...mergeAll` is used instead of just a simple
`merge` (or `scheduled...mergeAll`) to avoid introducing new rxjs
operators in order to keep bundle size down.

Fixes angular#18469
atscott added a commit to atscott/angular that referenced this issue Aug 6, 2020
…nks change

This commit introduces a new subscription in the `routerLinkActive` directive which triggers an update
when any of its associated routerLinks have changes. `RouterLinkActive` not only needs to know when
links are added or removed, but it also needs to know about if a link it already knows about
changes in some way.

Quick note that `from...mergeAll` is used instead of just a simple
`merge` (or `scheduled...mergeAll`) to avoid introducing new rxjs
operators in order to keep bundle size down.

Fixes angular#18469
atscott added a commit to atscott/angular that referenced this issue Aug 17, 2020
…nks change

This commit introduces a new subscription in the `routerLinkActive` directive which triggers an update
when any of its associated routerLinks have changes. `RouterLinkActive` not only needs to know when
links are added or removed, but it also needs to know about if a link it already knows about
changes in some way.

Quick note that `from...mergeAll` is used instead of just a simple
`merge` (or `scheduled...mergeAll`) to avoid introducing new rxjs
operators in order to keep bundle size down.

Fixes angular#18469
atscott added a commit that referenced this issue Aug 17, 2020
…nks change (#38349)

This commit introduces a new subscription in the `routerLinkActive` directive which triggers an update
when any of its associated routerLinks have changes. `RouterLinkActive` not only needs to know when
links are added or removed, but it also needs to know about if a link it already knows about
changes in some way.

Quick note that `from...mergeAll` is used instead of just a simple
`merge` (or `scheduled...mergeAll`) to avoid introducing new rxjs
operators in order to keep bundle size down.

Fixes #18469

PR Close #38349
atscott added a commit to atscott/angular that referenced this issue Aug 18, 2020
…nks change

This commit introduces a new subscription in the `routerLinkActive` directive which triggers an update
when any of its associated routerLinks have changes. `RouterLinkActive` not only needs to know when
links are added or removed, but it also needs to know about if a link it already knows about
changes in some way.

Quick note that `from...mergeAll` is used instead of just a simple
`merge` (or `scheduled...mergeAll`) to avoid introducing new rxjs
operators in order to keep bundle size down.

Fixes angular#18469
atscott added a commit to atscott/angular that referenced this issue Aug 18, 2020
…nks change

This commit introduces a new subscription in the `routerLinkActive` directive which triggers an update
when any of its associated routerLinks have changes. `RouterLinkActive` not only needs to know when
links are added or removed, but it also needs to know about if a link it already knows about
changes in some way.

Quick note that `from...mergeAll` is used instead of just a simple
`merge` (or `scheduled...mergeAll`) to avoid introducing new rxjs
operators in order to keep bundle size down.

Fixes angular#18469
atscott added a commit that referenced this issue Aug 18, 2020
…nks change (#38511)

This commit introduces a new subscription in the `routerLinkActive` directive which triggers an update
when any of its associated routerLinks have changes. `RouterLinkActive` not only needs to know when
links are added or removed, but it also needs to know about if a link it already knows about
changes in some way.

Quick note that `from...mergeAll` is used instead of just a simple
`merge` (or `scheduled...mergeAll`) to avoid introducing new rxjs
operators in order to keep bundle size down.

Fixes #18469

PR Close #38511
atscott added a commit that referenced this issue Aug 18, 2020
…nks change (#38511)

This commit introduces a new subscription in the `routerLinkActive` directive which triggers an update
when any of its associated routerLinks have changes. `RouterLinkActive` not only needs to know when
links are added or removed, but it also needs to know about if a link it already knows about
changes in some way.

Quick note that `from...mergeAll` is used instead of just a simple
`merge` (or `scheduled...mergeAll`) to avoid introducing new rxjs
operators in order to keep bundle size down.

Fixes #18469

PR Close #38511
subratpalhar92 pushed a commit to SUBRATPALHAR-ALL-JAVASCRIPT/angular that referenced this issue Aug 29, 2020
…nks change (angular#38349)

This commit introduces a new subscription in the `routerLinkActive` directive which triggers an update
when any of its associated routerLinks have changes. `RouterLinkActive` not only needs to know when
links are added or removed, but it also needs to know about if a link it already knows about
changes in some way.

Quick note that `from...mergeAll` is used instead of just a simple
`merge` (or `scheduled...mergeAll`) to avoid introducing new rxjs
operators in order to keep bundle size down.

Fixes angular#18469

PR Close angular#38349
subratpalhar92 pushed a commit to SUBRATPALHAR-ALL-JAVASCRIPT/angular that referenced this issue Aug 29, 2020
…nks change (angular#38511)

This commit introduces a new subscription in the `routerLinkActive` directive which triggers an update
when any of its associated routerLinks have changes. `RouterLinkActive` not only needs to know when
links are added or removed, but it also needs to know about if a link it already knows about
changes in some way.

Quick note that `from...mergeAll` is used instead of just a simple
`merge` (or `scheduled...mergeAll`) to avoid introducing new rxjs
operators in order to keep bundle size down.

Fixes angular#18469

PR Close angular#38511
subratpalhar92 pushed a commit to SUBRATPALHAR-ALL-JAVASCRIPT/angular that referenced this issue Aug 31, 2020
…nks change (angular#38349)

This commit introduces a new subscription in the `routerLinkActive` directive which triggers an update
when any of its associated routerLinks have changes. `RouterLinkActive` not only needs to know when
links are added or removed, but it also needs to know about if a link it already knows about
changes in some way.

Quick note that `from...mergeAll` is used instead of just a simple
`merge` (or `scheduled...mergeAll`) to avoid introducing new rxjs
operators in order to keep bundle size down.

Fixes angular#18469

PR Close angular#38349
subratpalhar92 pushed a commit to SUBRATPALHAR-ALL-JAVASCRIPT/angular that referenced this issue Aug 31, 2020
…nks change (angular#38511)

This commit introduces a new subscription in the `routerLinkActive` directive which triggers an update
when any of its associated routerLinks have changes. `RouterLinkActive` not only needs to know when
links are added or removed, but it also needs to know about if a link it already knows about
changes in some way.

Quick note that `from...mergeAll` is used instead of just a simple
`merge` (or `scheduled...mergeAll`) to avoid introducing new rxjs
operators in order to keep bundle size down.

Fixes angular#18469

PR Close angular#38511
profanis pushed a commit to profanis/angular that referenced this issue Sep 5, 2020
…nks change (angular#38349)

This commit introduces a new subscription in the `routerLinkActive` directive which triggers an update
when any of its associated routerLinks have changes. `RouterLinkActive` not only needs to know when
links are added or removed, but it also needs to know about if a link it already knows about
changes in some way.

Quick note that `from...mergeAll` is used instead of just a simple
`merge` (or `scheduled...mergeAll`) to avoid introducing new rxjs
operators in order to keep bundle size down.

Fixes angular#18469

PR Close angular#38349
profanis pushed a commit to profanis/angular that referenced this issue Sep 5, 2020
…nks change (angular#38511)

This commit introduces a new subscription in the `routerLinkActive` directive which triggers an update
when any of its associated routerLinks have changes. `RouterLinkActive` not only needs to know when
links are added or removed, but it also needs to know about if a link it already knows about
changes in some way.

Quick note that `from...mergeAll` is used instead of just a simple
`merge` (or `scheduled...mergeAll`) to avoid introducing new rxjs
operators in order to keep bundle size down.

Fixes angular#18469

PR Close angular#38511
@angular-automatic-lock-bot
Copy link

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 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.