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

MenuItem: Update of 'expanded' and 'label' properties not working #14893

Closed
marcioluish opened this issue Feb 27, 2024 · 12 comments · Fixed by #15042 or #15090
Closed

MenuItem: Update of 'expanded' and 'label' properties not working #14893

marcioluish opened this issue Feb 27, 2024 · 12 comments · Fixed by #15042 or #15090
Assignees
Labels
LTS-16-PORTABLE Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Milestone

Comments

@marcioluish
Copy link

Describe the bug

The following issue was opened in 12/11/2023 and closed in 12/13/2023, having the fix being checked in v17.1.0.

#14329

I honestly didn't follow up PanelMenu's new releases, but in v17.1.0 the issue was only partially fixed and now in v17.8.0, we have the same behavior as in versions above v16.0.2.

Below you can find 2 GIFs comparing v16.0.2 and v17.8.0. In these I access sub-menu items and change their label and expanded properties by clicking in the Change button.

Here's the link to Stackblitz using v16.0.2: https://stackblitz.com/edit/gegncb?embed=1&file=package.json

v16.0.2

Primeng_16_0_2

v17.8.0

Primeng_17_8_0

Environment

Angular 17.2.1 so as PrimeNg 17.8.0

Reproducer

https://stackblitz.com/edit/4sbrb2-gkf3lq?file=src%2Fapp%2Fdemo%2Fpanel-menu-multiple-demo.ts

Angular version

17.2.1

PrimeNG version

17.8.0

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

20.10.0

Browser(s)

No response

Steps to reproduce the behavior

No response

Expected behavior

No response

@marcioluish marcioluish added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Feb 27, 2024
@mehmetcetin01140 mehmetcetin01140 added this to the 17.9.0 milestone Feb 28, 2024
@cetincakiroglu cetincakiroglu added Type: Bug Issue contains a bug related to a specific component. Something about the component is not working Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Feb 28, 2024
@cetincakiroglu cetincakiroglu self-assigned this Feb 28, 2024
@cetincakiroglu cetincakiroglu removed the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Feb 28, 2024
@cetincakiroglu cetincakiroglu modified the milestones: 17.9.0, 17.10.0, 17.11.0 Feb 29, 2024
cetincakiroglu added a commit that referenced this issue Mar 13, 2024
Fixed #14893 - PanelMenu | Refactor to trigger change detection in su…
@psarno
Copy link

psarno commented Mar 14, 2024

I attempted to update PrimeNG to 17.11 and it caused a major regression in behavior of the <p-panelMenu>.

When expanding a node, it seems to briefly expand all of the children under that node before immediately collapsing them. When you go to collapse the parent, the same process occurs, where it expands and then collapses the children before collapsing the parent.

I backed it down again to 17.8 and everything is working as normal.

We aren't doing anything fancy with the menu as far as I can tell. We set the expanded property on parent nodes based on whether or not the children are selected, which is based on the route. This keeps the menus expanded properly when the page is refreshed. i.e.

expanded: this.router.url.includes('/some/route')

That's about it. But it was unusable in 17.11. I don't know if this is the specific commit that is causing us problems, but it does deal with MenuItem and expanded.

@marciohamasaki
Copy link

@cetincakiroglu the chagen/fix is not working for me as well.
Please refer to below GIF taken from stackblitz of PanelMenu primeng official website:

PNG
primeng_17_11

Stackblitz Link
https://stackblitz.com/edit/vpdt6g?file=package.json

@marcioluish
Copy link
Author

Hi @cetincakiroglu, thanks for looking into it.
However, expanded property is still not working.

@imaksp
Copy link

imaksp commented Mar 16, 2024

Hi @psarno it also broke our app, we are also simply using panel menu only on mobile view with static item array with label command & routerLink & this version has introduced infinite loop in our app, most likely due to PR which was supposed to fix this issue.
related issue:
#15057

@cetincakiroglu
Copy link
Contributor

Hi,

Re-opened and transferred in v17.12.0 milestone.

@cetincakiroglu cetincakiroglu modified the milestones: 17.11.0, 17.12.0 Mar 19, 2024
@github-actions github-actions bot added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Mar 19, 2024
cetincakiroglu added a commit that referenced this issue Mar 19, 2024
@cetincakiroglu
Copy link
Contributor

Hi,

Thanks for the feedback, I've found that there is a missing condition in the if statement in line 131 of panelMenuSub component which causes the issue of child expansion on root list item click. Rendering issue is fixed for good.

Could you please test it after the release?

@marcioluish
Copy link
Author

Hi,

Thanks for the feedback, I've found that there is a missing condition in the if statement in line 131 of panelMenuSub component which causes the issue of child expansion on root list item click. Rendering issue is fixed for good.

Could you please test it after the release?

Hi @cetincakiroglu thanks for checking this out. Yes, I'll try it as soon fix is released.
Thanks.

@marcioluish
Copy link
Author

Hi,
Thanks for the feedback, I've found that there is a missing condition in the if statement in line 131 of panelMenuSub component which causes the issue of child expansion on root list item click. Rendering issue is fixed for good.
Could you please test it after the release?

Hi @cetincakiroglu thanks for checking this out. Yes, I'll try it as soon fix is released. Thanks.

Hi @cetincakiroglu 17.12 still fails to manage expand and label MenuItem's properties.
Below is a GIF testing it:

17_12-panelMenu

And below is the modified stackblitz link of 17.12 PanelMenu's Multiple. Please have a look at it again to see if anything is wrong when trying to set those MenuItem's properties.
https://stackblitz.com/edit/sg4yxy?file=package.json

Desirable behavior would be the same that we had in 16.0.2 (first GIF of this card)

Thanks again.

@cetincakiroglu
Copy link
Contributor

Hi,
Thanks for the feedback, I've found that there is a missing condition in the if statement in line 131 of panelMenuSub component which causes the issue of child expansion on root list item click. Rendering issue is fixed for good.
Could you please test it after the release?

Hi @cetincakiroglu thanks for checking this out. Yes, I'll try it as soon fix is released. Thanks.

Hi @cetincakiroglu 17.12 still fails to manage expand and label MenuItem's properties. Below is a GIF testing it:

17_12-panelMenu 17_12-panelMenu

And below is the modified stackblitz link of 17.12 PanelMenu's Multiple. Please have a look at it again to see if anything is wrong when trying to set those MenuItem's properties. https://stackblitz.com/edit/sg4yxy?file=package.json

Desirable behavior would be the same that we had in 16.0.2 (first GIF of this card)

Thanks again.

Hi,

I guess the problem is that the change detection is not triggered. I can make it work a little bit refactoring https://stackblitz.com/edit/sg4yxy-rnqdjm?file=package.json,src%2Fapp%2Fdemo%2Fpanel-menu-multiple-demo.ts

@marcioluish
Copy link
Author

marcioluish commented Mar 29, 2024

Hi,

I guess the problem is that the change detection is not triggered. I can make it work a little bit refactoring https://stackblitz.com/edit/sg4yxy-rnqdjm?file=package.json,src%2Fapp%2Fdemo%2Fpanel-menu-multiple-demo.ts

@cetincakiroglu thanks for the response.
To then trigger MenuItem's change detection (it could be changes on expanded or label properties), it'll be needed now to update the whole items property/array (parent component) and not just one of its elements (children MenuItems)?

@marcioluish
Copy link
Author

Hi,

I guess the problem is that the change detection is not triggered. I can make it work a little bit refactoring https://stackblitz.com/edit/sg4yxy-rnqdjm?file=package.json,src%2Fapp%2Fdemo%2Fpanel-menu-multiple-demo.ts

Also @cetincakiroglu, I noticed that when manipulating expanded property and triggering change detection (by assigning new values to MenuItem's items array), we have a little issue between angledownicon and anglerighticon, where collapsed items still show the angledownicon:

image

Is this something that can be prevented without explicitly using html manipulation?

@cetincakiroglu
Copy link
Contributor

Hi,

I guess the problem is that the change detection is not triggered. I can make it work a little bit refactoring https://stackblitz.com/edit/sg4yxy-rnqdjm?file=package.json,src%2Fapp%2Fdemo%2Fpanel-menu-multiple-demo.ts

Also @cetincakiroglu, I noticed that when manipulating expanded property and triggering change detection (by assigning new values to MenuItem's items array), we have a little issue between angledownicon and anglerighticon, where collapsed items still show the angledownicon:

image

Is this something that can be prevented without explicitly using html manipulation?

Hi, thanks for letting us know. We'll check the issue again before the 17.14.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LTS-16-PORTABLE Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Projects
None yet
6 participants