Skip to content

[DashboardLayout] Allow custom navigation page items - #4809

Merged
apedroferreira merged 26 commits into
mui:masterfrom
apedroferreira:custom-navigation-items
Apr 21, 2025
Merged

[DashboardLayout] Allow custom navigation page items#4809
apedroferreira merged 26 commits into
mui:masterfrom
apedroferreira:custom-navigation-items

Conversation

@apedroferreira

@apedroferreira apedroferreira commented Mar 27, 2025

Copy link
Copy Markdown
Collaborator

Closes #4413.
Closes #4607 both for the hidden and disabled cases.

Also helps with #4733 but additional state management would be needed from the user, so maybe there is a better solution for that in the future.

Also makes it possible to easily mark items as expanded, selected or disabled, or use external links with a custom href.

Check the new documentation here:

@apedroferreira apedroferreira added type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. scope: toolpad-core Abbreviated to "core" component: layout labels Mar 27, 2025
@apedroferreira apedroferreira self-assigned this Mar 27, 2025
@mui-bot

mui-bot commented Mar 27, 2025

Copy link
Copy Markdown

Netlify deploy preview

https://deploy-preview-4809--mui-toolpad-docs.netlify.app/

Generated by 🚫 dangerJS against 38cc469

@apedroferreira apedroferreira changed the title Custom navigation items [DashboardLayout] Allow custom navigation page items Mar 27, 2025
@davorian

Copy link
Copy Markdown

I'm awaiting this!!!

@apedroferreira
apedroferreira requested a review from a team April 1, 2025 14:33
@apedroferreira
apedroferreira marked this pull request as ready for review April 1, 2025 14:33
@@ -0,0 +1,31 @@
{
"props": {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤔 I would have expect this to just have a item prop. Does it need all these other props? This is just to render the default item right? I'm not sure we should expose more API on it.

@apedroferreira apedroferreira Apr 2, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

If you look at the expanded demo here I was just trying to make this type of changes as easy as possible as well as reusing the existing component. Some of these are things that some users already asked to be able to override.

Would it help if these props were additional options maybe? And the first parameter was the item?

@apedroferreira

Copy link
Copy Markdown
Collaborator Author

I've changed the renderPageItem typing to renderPageItem?: (item: NavigationPageItem, params: { mini: boolean }) => React.ReactNode; and updated the docs accordingly. The mini parameter still seems useful to render things differently in the minified sidebar.

It's using context under the hood to pass default values to the page item component without exposing all that API to the users, who can only set a few simple optional props now if they wish to override certain behaviors.

Examples in https://deploy-preview-4809--mui-toolpad-docs.netlify.app/toolpad/core/react-dashboard-layout/#custom-page-items

@apedroferreira
apedroferreira requested review from a team and Janpot April 17, 2025 21:11
@Janpot

Janpot commented Apr 18, 2025

Copy link
Copy Markdown
Member

👍

The mini parameter still seems useful to render things differently in the minified sidebar.

I suppose the alternative would be renderMiniPageItem?

@apedroferreira

Copy link
Copy Markdown
Collaborator Author

👍

The mini parameter still seems useful to render things differently in the minified sidebar.

I suppose the alternative would be renderMiniPageItem?

That should work too if we don't mind the extra property, I'll change it.

@Janpot

Janpot commented Apr 18, 2025

Copy link
Copy Markdown
Member

I'll change it.

for the record, that's not the propose of my comment. I'm fine with what there is

@apedroferreira

apedroferreira commented Apr 18, 2025

Copy link
Copy Markdown
Collaborator Author

I'll change it.

for the record, that's not the propose of my comment. I'm fine with what there is

Ok, I didn't have a huge preference between the 2 solutions but I guess that as a user I would prefer a single function prop, not to have to use 2 separate ones. So if it's ok I guess we can go with this.

Just need an approval and I can merge!

@bharatkashyap bharatkashyap left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Big improvement! Looks good in the demo/docs 👍🏻

@apedroferreira
apedroferreira merged commit c0312cc into mui:master Apr 21, 2025
@apedroferreira
apedroferreira deleted the custom-navigation-items branch April 21, 2025 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: toolpad-core Abbreviated to "core" type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hide Navigation items based on hidden props [toolpad-core] Add support for external links in dashboard navigation

5 participants