feat(SideBar): introduce a new SideBar component #444
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| '.scrollbar-none': { | ||
| 'scrollbar-width': 'none', | ||
| '&::-webkit-scrollbar': { | ||
| display: 'none', | ||
| }, |
There was a problem hiding this comment.
[low]: cool 💯
Could use it on DialogBody instead of style
There was a problem hiding this comment.
Pull request overview
This pull request introduces a new SideBar component to the UI React library, providing a responsive navigation component with collapsible functionality. The implementation follows a composite pattern with value-based selection, similar to the existing TabBar component.
Changes:
- Added a new
useControllableStateutility hook to support both controlled and uncontrolled component patterns - Implemented the SideBar component with sub-components (SideBarLeading, SideBarTrailing, SideBarFooter, SideBarItem, SideBarCollapseToggle)
- Added a new scrollbar utility plugin to Tailwind configuration across all design presets
- Included comprehensive test coverage, Storybook stories, and MDX documentation
- Added i18n translations for all supported locales (en, de, es, fr, ja, ko, pt, ru, th, tr, zh)
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/ui-react/src/utils/useControllableState/* | New utility hook for controlled/uncontrolled state management with comprehensive tests |
| libs/ui-react/src/lib/Components/SideBar/* | Main SideBar component implementation with types, tests, stories, and documentation |
| libs/ui-react/src/lib/Components/index.ts | Export added for the new SideBar component |
| libs/ui-react/src/i18n/locales/*.json | i18n translations added for sidebar aria labels in all supported languages |
| libs/design-core/src/utils/createCustomPlugin.ts | New scrollbar-none utility plugin for hiding scrollbars |
| libs/design-core/src/presets/*.ts | Scrollbar plugin integrated into all design presets |
| .nx/version-plans/version-plan-1770023975751.md | Version plan file (contains incorrect description) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| icon: React.ComponentType<{ size?: IconSize; className?: string }>; | ||
| /** | ||
| * The icon component to display when the item is active. | ||
| */ | ||
| activeIcon: React.ComponentType<{ size?: IconSize; className?: string }>; |
There was a problem hiding this comment.
[low]: Not really a blocker,
Just opening the discussion, so maybe later we could defide to align on ReactNode as well for this kind of props.
As seen this week in the articles, typesafety doesn't really work for children because it accept any JSX.Element
I've made a test with defining a div:
There was a problem hiding this comment.
ah waw! Well ReactNode it is then, will add a ticket for this as we have it in multiple other places
| /** | ||
| * Props for the SideBar root component | ||
| */ | ||
| export type SideBarProps = { |
There was a problem hiding this comment.
Nice 💯
Like to have default vs controlled
| ### SideBarFooter | ||
|
|
||
| Use `SideBarFooter` to place content at the bottom of the sidebar. This is typically used to contain the `SideBarCollapseToggle`. | ||
|
|
||
| **Note:** The footer scrolls with the sidebar content. |
There was a problem hiding this comment.
[mid]: Isn't the footer is supposed to be sticky ?
If not, I believe both SidebarTrailing and SidebarFooterprovide the same behavior, so we might want to kill Footer ? And just position theSideBarCollapseToggle` at the end of the trailing
There was a problem hiding this comment.
I discussed this with Laurine: she mentioned that for wallets it shouldn’t be sticky since they don’t handle multiple items (at least for now), but that for LES it might potentially need to be sticky.
So I thought it would be wise to expose an API that is already designed to evolve. WDYT ? maybe we can just add it in the future honestly no strong opinion
There was a problem hiding this comment.
Indeed, either we keep it like it or streamline everything in the trailing for now - and add support later for sticky
|
|
||
| <DoVsDontRow> | ||
| <DoBlockItem | ||
| title='Use the `value` prop on items' |
There was a problem hiding this comment.
[low]: this one is already explicit in the docs - redundant
There was a problem hiding this comment.
Yeah it was usefull when label fallledback on value
| "goBackAriaLabel": "Назад" | ||
| }, | ||
| "sideBar": { | ||
| "navigationAriaLabel": "Боковая навигация", |
Co-authored-by: Simon.b <15988848+gamegee@users.noreply.github.com>
No description provided.