Component: p-menu PrimeNg 16.1.0 regression MenuItem.id no longer assigned to MenuItemContent anchor tags

[SaajRP]

Describe the bug

As a result of the code merged to menu.ts for #13372, the id defined against a MenuItem provided to p-menu is no longer assigned to the id of the anchor tag

https://github.com/primefaces/primeng/pull/13372/files#diff-6fb5eaf005117b0f3acb23845d35b567bb9e572bcdb8a957aeff8fbb791ffc80

Environment

PrimeNg 16.1.0

Reproducer

https://github.com/primefaces/primeng/pull/13372/files#diff-6fb5eaf005117b0f3acb23845d35b567bb9e572bcdb8a957aeff8fbb791ffc80

Angular version

16.X.X

PrimeNG version

16.1.0

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

18.12.0

Browser(s)

No response

Steps to reproduce the behavior

Inspect the HTML of one of the MenuItem elements, the anchor tag is not using the the defined id

Expected behavior

The anchor tag for a given MenuItem should be using the defined id

[arkancrow]

The issue is still present in the version 16.2.0.

[KeithGillette]

I think this issue is also changing the id of the MenuItem that is passed to the MenuItemCommandEvent.

[ES-Six]

The issue is still present in the version 16.3.1.

The issue about the anchor tag could be due to this line (in pMenuItemContent) that has been removed :

[attr.id]="item.id"

Indeed, this line has changed between version 16.0.2 and 16.1.0.

Before : https://github.com/primefaces/primeng/blob/16.0.2/src/app/components/menu/menu.ts#L59
After : https://github.com/primefaces/primeng/blob/16.1.0/src/app/components/menu/menu.ts#L59
Latest stable version at the moment of writing : https://github.com/primefaces/primeng/blob/16.3.1/src/app/components/menu/menu.ts#L56

This commit removed this line : cd1f6e5

So this commit might have introduced this bug.

By observing modifications that were made, we can see it's easy to correct this bug as adding this line again should correct it, like this :

<a
                *ngIf="!item?.routerLink"
                [attr.title]="item.title"
                [attr.id]="item.id" // <--- Add here
                [attr.href]="item.url || null"
                [attr.data-automationid]="item.automationId"
                [attr.tabindex]="-1"
                [attr.data-pc-section]="'action'"
                [attr.aria-hidden]="true"
                class="p-menuitem-link"
                [target]="item.target"
                [ngClass]="{ 'p-disabled': item.disabled }"
                (click)="onItemClick($event, item)"
                pRipple
            >
                <ng-container *ngTemplateOutlet="itemContent"></ng-container>
            </a>
            <a
                *ngIf="item?.routerLink"
                [routerLink]="item.routerLink"
                [attr.data-automationid]="item.automationId"
                [attr.tabindex]="-1"
                [attr.data-pc-section]="'action'"
                [attr.aria-hidden]="true"
                [attr.title]="item.title"
                [attr.id]="item.id" // <--- Add here too
                [queryParams]="item.queryParams"
                routerLinkActive="p-menuitem-link-active"
                [routerLinkActiveOptions]="item.routerLinkActiveOptions || { exact: false }"
                class="p-menuitem-link"
                [target]="item.target"
                [ngClass]="{ 'p-disabled': item.disabled }"
                (click)="onItemClick($event, item)"
                [fragment]="item.fragment"
                [queryParamsHandling]="item.queryParamsHandling"
                [preserveFragment]="item.preserveFragment"
                [skipLocationChange]="item.skipLocationChange"
                [replaceUrl]="item.replaceUrl"
                [state]="item.state"
                pRipple
            >
                <ng-container *ngTemplateOutlet="itemContent"></ng-container>
            </a>

Since these two block conditions are opposite, this will not lead to id conflict because *ngIf remove hidden elements from the DOM so only one of these two blocks with an ID will be created at a time.

[jerkovicl]

Any updates on this ? This needs to be fixed

[dalenguyen]

I created PR for this. Hopefully it will get feedback soon.

[cetincakiroglu]

Hi,

This issue is a duplicate of #13705 and it's fixed in here 44a5075

Closing the issue since it's duplicate and already fixed.

Thanks a lot for your effort and support!

[SaajRP]

@cetincakiroglu the fix done against 44a5075 for #13705

Does not actually replicate the old behaviour prior to PrimeNg 16.1.0. Prior to this the MenuItem.id was being assigned to the p-menuitem-link anchor tag element inside p-menu-content div within the menuItem li element

Looking at the fix that has been done, and confirming it in the below stackblitz instance, the MenuItem.id is now being assigned to the menuitem li element. This is a clear change in behaviour that needs to be documented in the Changelog, as for anyone upgrading from PrimeNg version <= 16.0.2 this maybe a breaking change for their application

https://stackblitz.com/edit/ykgezg?file=src%2Fapp%2Fdemo%2Fmenu-basic-demo.ts

N.b
For anyone needing some sort of identifier against the p-menuitem-link anchor tag, the MenuItem.automationId would be your best bet