-
Notifications
You must be signed in to change notification settings - Fork 47
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
pkp/pkp-lib#9890 Update the design for the focused item state in SideMenu #408
Conversation
72e69ee
to
2682b2b
Compare
…idemenu component
…item.command function to handle click events
… in SideMenu component
46fa99e
to
5ee2c9a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great, only one question to better understand how the dynamic changes of the menu works.
|
||
watchEffect(() => { | ||
Object.assign(items, convertLinksToArray(linksRef.value)); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because of some runtime menu changes, right? Can you just explain bit more how it works?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Jarda, sorry I wasn't able to provide explanation prior merging, I didn't notice you had a question here.
The props coming from php are not propagated down to the useSideMenu API so this change was necessary. An example of this runtime change is when enabling the Announcements feature where we don't reload the page and just add the link to the sidebar.
As for the watchEffect
, when props.links
get updated, then it was necessary to update the items
array so the useSideMenu
can propagate the changes down to the PanelMenu component. If you have other suggestion to make this work better, then please let me know and I can get the changes on a separate PR. Thanks!
No description provided.