-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Navigation component: Store navigation tree in context v2 #25340
Navigation component: Store navigation tree in context v2 #25340
Conversation
Size Change: +575 B (0%) Total Size: 1.2 MB
ℹ️ View Unchanged
|
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.
I've left mostly minor comments, but I'm ok with the direction.
We definitely need to document why we are keeping the children rendered, because I'm sure we'll forget the reason in no time and will have the hardest time figuring that out again. 😄
(Also appreciate that you've moved the components in their own folder! 🙇 )
packages/components/src/navigation/use-navigation-tree-nodes.js
Outdated
Show resolved
Hide resolved
packages/components/src/navigation/menu/use-navigation-tree-menu.js
Outdated
Show resolved
Hide resolved
1e266a4
to
f31277c
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.
Code LGTM.
I've also tested this by modifying the Storybook example with a button that adds and removes a new menus and items to the navigation.
This has a side effect that menus and items now need a unique identifier.
I took the liberty of pushing an update to the readme to recognize this new requirement.
Fixes: #25246
Description
Adds
items
andmenus
to the Navigation context contained in anavigationTree
object.Example usage: Change back button label to the parent menu's title
packages/components/src/navigation/menu.js
Both
items
andmenus
are objects.Keys are based on the
item
prop foritems
and they are based onmenu
prop formenus
. This makes them easy to access by their slug.Values are the props of the
NavigationItem
andNavigationMenu
items without thechildren
prop. Items also include the menu that contains the item asmenu
property.Types of changes
New feature (non-breaking change which adds functionality)
Checklist: