Add animations to navigation component #24771
Conversation
1c2111b to
aa9d063
Compare
|
As a heads up, the animation here technically hides the level that is being animated out immediately and then slides in the new level, which is subtly different than sliding out the old level and sliding in the new level. If we want to switch to the latter, we can do so but I think the best way to do this is iterate over all the levels. /cc @jameskoster for your thoughts on this animation. (Don't worry too much about the other design as we'll be styling more within the wc nav repo.) |
| @@ -84,7 +84,7 @@ function Example() { | |||
|
|
|||
| return ( | |||
| <Navigation activeItemId={ active } data={ data } rootTitle="Home"> | |||
| { ( { level, levelItems, parentLevel, NavigationBackButton } ) => { | |||
| { ( { level, parentLevel, NavigationBackButton } ) => { | |||
There was a problem hiding this comment.
More simplification, nice!
| @@ -35,7 +35,7 @@ const NavigationMenuItem = ( props ) => { | |||
|
|
|||
| const handleClick = () => { | |||
| if ( children.length ) { | |||
| setActiveLevel( id ); | |||
| setActiveLevelId( id ); | |||
| level.id === 'root' | ||
| ? null | ||
| : items.find( ( item ) => item.id === level.parent ); | ||
| const items = mapItems( data ); |
There was a problem hiding this comment.
This calculation happens on every render. How about adding useMemo here?
There was a problem hiding this comment.
Great idea! Added in e687266.
I think there are some further optimizations that we could tackle here so a full update doesn't occur whenever the activeItemId is changed, but may need to handle in a follow-up.
…ress#24704) * remove setActiveLevel from public api * change to NavigationBackButton * return null for backButton if no parentLevel
…Press#24706) * Expose __experimentalNavigation component * fix typo * expose menut and menu-item as well
69c10d6 to
67ff7e2
Compare
|
I made a mistake when trying to rebase the feature branch and deleted the changes from #24706. Those changes now are included in this PR. Otherwise, its all looking good and ready to merge. The only issue is a failed check because |
* Navigation Component: Remove setActiveLevel child function arg (#24704) * remove setActiveLevel from public api * change to NavigationBackButton * return null for backButton if no parentLevel * Navigation Component: Expose __experimentalNavigation component (#24706) * Expose __experimentalNavigation component * fix typo * expose menut and menu-item as well * Loop over navigation levels and wrap with animate * Use map and set to organize items and levels * Simplify level and item logic * Remove unnecessary key prop * Fix parent level assignment * Add back in key prop to force animation * Use useMemo to map item data Co-authored-by: Paul Sealock <psealock@gmail.com>
Fixes #24258
Description
Testing