[account] [docs] Add <Account /> in sidebarFooter#4255
[account] [docs] Add <Account /> in sidebarFooter#4255bharatkashyap merged 87 commits intomui:masterfrom
<Account /> in sidebarFooter#4255Conversation
…into enh/account-3
…into enh/account-3
| Feel free to remind me to review this one once the others are merged! |
| I am not sure if the following has been addressed in this pull request, but there appears to be a small space on the far right when the
|
github-actions bot added the PR: out-of-date github-actions bot removed the PR: out-of-date 
Gets addressed in this PR: |
Should be ready to review now! |
apedroferreira left a comment
apedroferreira left a comment There was a problem hiding this comment.
Just some visual things and figuring out how we should type slots!
| ); | ||
| } | ||
| | ||
| function SidebarFooterAccount() { |
There was a problem hiding this comment.
This needs some logic to handle the mini-drawer variant, not to overflow.
The mini prop should always be available in this slot, it's just a boolean.
There was a problem hiding this comment.
Missed that! I've modified the demo to be able to handle the mini state
| <Account | ||
| slots={{ | ||
| preview: AccountSidebarPreview, | ||
| popoverContent: SidebarFooterAccountPopover, |
There was a problem hiding this comment.
In these docs demos, opening and closing the popover automatically scrolls unless they have the disableAutoFocus prop, maybe we could have that somehow?
There was a problem hiding this comment.
Great find! I've added disableAutoFocus as true by default on the Account popover, but overridable by users through the slotProps
There was a problem hiding this comment.
Ok, I'm not 100% sure if we'd need it in a normal app though, maybe it's just a problem in the demo iframes.
| @@ -0,0 +1,247 @@ | |||
| import * as React from 'react'; | |||
There was a problem hiding this comment.
The 3 dots icon doesn't seem vertically centered.
There was a problem hiding this comment.
Also the popover isn't pointing to / coming from the 3 dots icon, I guess it should?
There was a problem hiding this comment.
Agree, I've tweaked the demo to reflect this in both states:
There was a problem hiding this comment.
For the expanded variant, could there be an option to choose whether the popup appears on the right or the top side?
There was a problem hiding this comment.
You should be able to customize the popover's position (as well as the placement of the arrow) using the slotProps - the demo on the docs will showcase this using the transformOrigin and anchorOrigin props
| | ||
| {{"demo": "DashboardLayoutSlots.js", "height": 400, "iframe": true}} | ||
| | ||
| Through this, you can also modify the position of the `<Account />` component and use it inside the sidebar: |
There was a problem hiding this comment.
In #4340 I'm adding an Examples section here, I guess that we can just have this under that section once it gets added.
| * @default null | ||
| */ | ||
| sidebarFooter?: React.JSXElementConstructor<SidebarFooterProps>; | ||
| sidebarFooter?: React.ElementType; |
There was a problem hiding this comment.
So the SignInPage slots are typed as React.JSXElementConstructor<Props> but the slots in Account are React.ElementType?
Shouldn't it be consistent all around? What do the other MUI component use?
Also the SidebarFooterProps have a mini boolean prop that can be useful, so we need the types to be aware of it...
There was a problem hiding this comment.
I've updated the sidebarFooter type as well as the types of the Account slots; agree on the need for consistency - we can probably discuss this and raise a PR separately to bring consistency across all other components
…ap/mui-toolpad into feat/account-sidebar-variant
apedroferreira left a comment
apedroferreira left a comment There was a problem hiding this comment.
Awesome!! Thanks for the changes!
| /** | ||
| * To override the default `sx` value on the `Stack` component in the "expanded" variant | ||
| */ | ||
| sx?: SxProps; |
There was a problem hiding this comment.
So it applies to the expanded variant only? Probably ideally it would apply to any variant to be less confusing.
If you're not using it yet maybe we can skip this prop for now?
There was a problem hiding this comment.
Fair enough, we can leave it out for now, adding it will not be a breaking change later
…ap/mui-toolpad into feat/account-sidebar-variant
4 participants
Account#4102Summary of grooming discussion
Props:
hideToolbarAccountprop, users can pass a component which returnsnullinto thetoolbarAccountslot when using<Account />inside the sidebar footerSlots:
export composable components:
=> for the sidebar version =
scenarios:
https://deploy-preview-4255--mui-toolpad-docs.netlify.app/toolpad/core/react-dashboard-layout