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

Component: TabMenu incorrectly checks if routerLink is active #11999

Closed
atheros opened this issue Oct 2, 2022 · 4 comments
Closed

Component: TabMenu incorrectly checks if routerLink is active #11999

atheros opened this issue Oct 2, 2022 · 4 comments
Assignees
Labels
Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Milestone

Comments

@atheros
Copy link
Contributor

atheros commented Oct 2, 2022

Describe the bug

TabMenu incorrectly checks if routerLink is active.

Environment

Web browser

Reproducer

No response

Angular version

14.1

PrimeNG version

14.1.2

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

N/A

Browser(s)

No response

Steps to reproduce the behavior

Have two tabs with nested linkes (eg. /product and /product/details)
Add to each of the items routerLinkActiveOptions: {exact: true}
Test, when navingating to nested link (/products/details), both tabs are highlighted as active.

The result is p-menulink-active is correctly assigned to only one item, however both tabs (

  • elements) have p-highlight class set.

    Expected behavior

    Only one of the tabs is highlighted, according to their configuration.

    I did some digging. The problem seem to be the TabMenu.isActive() method, that uses a depricated signature of Router.isActive (at least in Angular 14), and calling it with exact false.

    A very simple solution would be to replace the false argument with item.routerLinkActiveOptions??false.

    Related issues: Tabmenu highlights wrong menuitem when has routerLinkActiveOptions set as exact #11410, TabMenu tab still active even when the routerLinkActiveOptions containc exact true. #11495

  • @atheros atheros added the Type: Bug Issue contains a bug related to a specific component. Something about the component is not working label Oct 2, 2022
    @atheros
    Copy link
    Contributor Author

    atheros commented Oct 2, 2022

    As a workaround, until a permanent fix is deployed, you can use this:

    import {TabMenu} from 'primeng/tabmenu';
    import {MenuItem} from 'primeng/api';
    
    export function patchTabMenu() {
        TabMenu.prototype.isActive = function (item: MenuItem): boolean {
            if (item.routerLink){
                const routerLink = Array.isArray(item.routerLink) ? item.routerLink : [item.routerLink];
                const router = (this as any).router;
                return router.isActive(router.createUrlTree(routerLink, {relativeTo: (this as any).route}).toString(),
                    item.routerLinkActiveOptions?.exact ?? item.routerLinkActiveOptions ?? false);
            }
    
            return item === this.activeItem;
        };
    }

    I call it from app.module.ts, then TabMenu work as expected.

    When setting up menu items, set routerLinkActiveOptions to either {exact: true} or IsActiveMatchOptions object.

    @majkers
    Copy link

    majkers commented Nov 9, 2022

    I think it is safer to use:

    router.createUrlTree(routerLink, {
                        relativeTo: (this as any).route,
                        queryParams: item.queryParams
                    });
    

    Reason why: I am using tabMenu and the first item in it has a routerLink more or less like this: /aaa?part=navbar&search=about&year=2021 so with queryParams and exact set to false in options. The other item has routerLink like /aaa/bbb/2 so it is dynamic like this /aaa/bbb/:id and exact set to true in options. So when used like this:

    router.createUrlTree(routerLink, {
                        relativeTo: (this as any).route
                    });
    

    for currentUrl with /aaa/bbb/2 and for the first item the output of method above would be like /aaa so isActive would result with true.

    @luisedo97
    Copy link

    As a workaround, until a permanent fix is deployed, you can use this:

    import {TabMenu} from 'primeng/tabmenu';
    import {MenuItem} from 'primeng/api';
    
    export function patchTabMenu() {
        TabMenu.prototype.isActive = function (item: MenuItem): boolean {
            if (item.routerLink){
                const routerLink = Array.isArray(item.routerLink) ? item.routerLink : [item.routerLink];
                const router = (this as any).router;
                return router.isActive(router.createUrlTree(routerLink, {relativeTo: (this as any).route}).toString(),
                    item.routerLinkActiveOptions?.exact ?? item.routerLinkActiveOptions ?? false);
            }
    
            return item === this.activeItem;
        };
    }

    I call it from app.module.ts, then TabMenu work as expected.

    When setting up menu items, set routerLinkActiveOptions to either {exact: true} or IsActiveMatchOptions object.

    Very good solution!, since I only use it once in a component, I call it from there.

    cetincakiroglu added a commit that referenced this issue Dec 29, 2022
    TabMenu route link matching fixed #11999
    @cetincakiroglu cetincakiroglu changed the title Component: TabMenu Component: TabMenu incorrectly checks if routerLink is active Dec 29, 2022
    @cetincakiroglu cetincakiroglu added this to the 15.0.1 milestone Dec 29, 2022
    @aturoczy
    Copy link

    aturoczy commented Jan 7, 2023

    Seems like this change could introduce a breaking change, and the whole site do not render after the upgrade. (Downgrade to 15.0.0 helped)

    ERROR TypeError: pathCompareMap[options.paths] is not a function at containsTree (router.mjs:216:41) at Router.isActive (router.mjs:5545:16) at TabMenu.isActive (primeng-tabmenu.mjs:53:32) at TabMenu_li_7_Template (primeng-tabmenu.mjs:146:93) at executeTemplate (core.mjs:11242:9) at refreshView (core.mjs:11127:13) at refreshEmbeddedViews (core.mjs:12142:17) at refreshView (core.mjs:11151:9) at refreshComponent (core.mjs:12188:13) at refreshChildComponents (core.mjs:10918:9)

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants