Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DashboardLayout] Add sidebarFooter slot #4236

Merged
merged 5 commits into from
Oct 11, 2024

Conversation

apedroferreira
Copy link
Member

@apedroferreira apedroferreira commented Oct 10, 2024

Closes #4101
Partial item in #4147

@apedroferreira apedroferreira added the feature: Components Button, input, table, etc. label Oct 10, 2024
@apedroferreira apedroferreira self-assigned this Oct 10, 2024
@apedroferreira apedroferreira marked this pull request as ready for review October 11, 2024 14:04
@apedroferreira apedroferreira requested a review from a team October 11, 2024 14:05
@@ -124,7 +139,9 @@ function DashboardLayoutSlots(props) {
theme={demoTheme}
window={demoWindow}
>
<DashboardLayout slots={{ toolbarActions: Search }}>
<DashboardLayout
slots={{ toolbarActions: Search, sidebarFooter: SidebarFooter }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder about naming. Should we call this something like "NavigationFooter"? 🤔 I feel like it would make sense to give our surfaces a semantic name, rather one that is based on where it's positioned. Not sure though

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far I've used the word "sidebar" for this kind of interfaces and it made the most sense to me in the places it was used, where it seemed like they were specific features of the sidebar itself and not just navigation.
The hideNavigation prop in DashboardLayout is an exception but it also hides the menu icon, so it's a bit more general.
For this particular slot I think we really mean the footer of the sidebar and no other type of navigation so I feel like it makes sense to name it this way? And it might be easier for users to understand what it really means visually.

It's also just my opinion of course, but the logic I've been following for this kind of naming is intentional.

Copy link
Member

@Janpot Janpot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some musings about naming, but overall looks fine.

@apedroferreira
Copy link
Member Author

Merging but if we agree to change any naming we can still do it!

@apedroferreira apedroferreira merged commit 48502b4 into mui:master Oct 11, 2024
14 checks passed
@apedroferreira apedroferreira deleted the add-sidebar-footer-slot branch October 11, 2024 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: Components Button, input, table, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[template] Navigation Drawer Footer
2 participants