feat(react-nav): add motion to components #34632
Conversation
* master: feat(react-nav): release to stable (microsoft#34631)
|
Pull request demo site: URL |
marcosmoura
force-pushed
the
feat/react-nav/add-motion-to-components
branch
from
ee2fdd6 to
bfd97a8
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| "@fluentui/react-shared-contexts": "^9.23.1", | ||
| "@fluentui/react-tabster": "^9.24.8", | ||
| "@fluentui/react-theme": "^9.1.24", | ||
| "@fluentui/react-tooltip": "^9.6.9", |
There was a problem hiding this comment.
Why do we need this? It is not referenced in the PR, or am I missing something?
There was a problem hiding this comment.
React tooltip is part of the Nav package for a long time. The diff here is simply putting the packages in alphabetical order.
| @@ -12,7 +11,7 @@ import type { NavCategoryProps, NavCategoryState } from './NavCategory.types'; | |||
| * @param props - props from this instance of NavCategory | |||
| * @param ref - reference to root HTMLDivElement of NavCategory | |||
| */ | |||
| export const useNavCategory_unstable = (props: NavCategoryProps, ref: React.Ref<HTMLDivElement>): NavCategoryState => { | |||
| export const useNavCategory_unstable = (props: NavCategoryProps): NavCategoryState => { | |||
There was a problem hiding this comment.
What's the reason behind this change? Is it not used anywhere? If so, we need to cleanup a JSDoc above.
There was a problem hiding this comment.
That's true about the JSDoc. The issue here is that ref is never gonna be used, as the component is "renderless". We may need to keep it, to not change the component contract and add an eslint comment.
| const getExpandIcon = (state: NavCategoryItemState) => { | ||
| assertSlots<NavCategoryItemSlots>(state); | ||
|
|
||
| if (state.expandIcon) { |
There was a problem hiding this comment.
nit, but early returns might be a bit more readable:
if (!state.expandIcon) {
return null
}
if (state.expandIconMotion) {
return (
<state.expandIconMotion>
<state.expandIcon />
</state.expandIconMotion>
);
}
return <state.expandIcon />;
| @@ -43,6 +71,7 @@ export const useNavCategoryItem_unstable = ( | |||
| root: 'button', | |||
| icon: 'span', | |||
| expandIcon: 'span', | |||
| expandIconMotion: ExpandIconMotion as React.FC<PresenceMotionSlotProps>, | |||
There was a problem hiding this comment.
typecasting might be redundant after React 18 PR support is merged
There was a problem hiding this comment.
Yep. This was made before the merge. Gonna fix it!
| import { NavDensity } from '../Nav/Nav.types'; | ||
|
|
||
| export type NavSubItemGroupCollapseMotionParams = { | ||
| items?: number; |
There was a problem hiding this comment.
pls add JSDoc description
| import { useNavCategoryContext_unstable } from '../NavCategoryContext'; | ||
| import { | ||
| PresenceMotionSlotProps, |
There was a problem hiding this comment.
| PresenceMotionSlotProps, | |
| type PresenceMotionSlotProps, |
|
|
||
| return { | ||
| open, | ||
| components: { | ||
| root: 'div', | ||
| collapseMotion: NavGroupMotion as React.FC<PresenceMotionSlotProps>, |
There was a problem hiding this comment.
the same as above, not sure if that still relevant
Previous Behavior
Components were opening and closing without any motion (static).
New Behavior
Components now have motion when opening and closing (smooth animations)