BA-3031 Add slot props and customization options to navigation compon… #331
Conversation
samueljansem
requested review from
anicioalexandre and
priscilladeroode
as code owners
|
📝 WalkthroughWalkthroughNavigation components and types now accept and forward new customization props ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/components/modules/navigations/web/NavCentered/types.ts`:
- Around line 9-10: NavCenteredProps declares slotProps but NavigationLayout's
render path doesn't forward slotProps to the NavCentered component while it does
for NavHorizontal/NavMini/NavVertical; update the NavigationLayout/index.tsx
render branch that returns <NavCentered ...> to include the slotProps prop (pass
the same slotProps variable used for other navs), or if the centered layout
truly shouldn't accept slotProps remove slotProps from the NavCenteredProps type
in packages/components/modules/navigations/web/NavCentered/types.ts and any
related usages to keep the API consistent.
In `@packages/components/modules/navigations/web/NavigationLayout/index.tsx`:
- Around line 50-55: NavCentered is missing the slotProps prop—add
slotProps={slotProps} to the NavCentered JSX so it receives the same slotProps
passed to NavHorizontal, NavMini, and NavVertical; locate the NavCentered
component usage in NavigationLayout (the instance with navData, openNav,
onCloseNav, VerticalDrawerProps) and include slotProps to match NavCenteredProps
type and ensure consistent behavior.
🧹 Nitpick comments (2)
packages/components/modules/navigations/web/__shared__/VerticalDrawer/index.tsx (1)
26-30: Consider adding dependency array entry foronCloseNav.The effect depends on
pathnamebut callsonCloseNav. WhileonCloseNavis likely stable, the exhaustive-deps rule would flag this. If linting is enforced, consider wrappingonCloseNavin a ref or adding it to deps with appropriate memoization upstream.packages/components/modules/navigations/web/NavVertical/index.tsx (1)
51-51: Consider mergingsxprop for consistency with NavMini.In
NavMini,NavToggleButtonmerges thesxprop explicitly to allow custom positioning overrides:sx={{ top: 22, left: NAV_WIDTH.MINI - 12, ...NavToggleButtonProps?.sx }}Here, the spread
{...NavToggleButtonProps}relies onNavToggleButton's internalsxmerge. This works becauseNavToggleButtonalready spreadssxinternally, but the pattern is inconsistent between components.
…mponent customization
|
2 participants
Why These Changes Were Needed
Projects using this component library often need to customize specific inner elements (nav items styling, drawer behavior, toggle button appearance) to match their design systems. Previously, the only option was to recreate entire navigation components from scratch. These props enable granular customization while preserving default behavior.
Changes Made
DrawerPropsto allow full Drawer customization with proper PaperProps.sx mergingVerticalDrawerPropsandNavToggleButtonPropsVerticalDrawerPropsandNavToggleButtonProps, fixedLogoIconpassthroughVerticalDrawerPropsVerticalDrawerPropsVerticalDrawerPropsandNavToggleButtonProps, exposing them as the public APIUsage Examples
Jira Issue: https://silverlogic.atlassian.net/browse/BA-3028
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.