Skip to content

[DashboardLayout] Highlight and text alignment for mini - #4844

Merged
bharatkashyap merged 3 commits into
mui:masterfrom
bharatkashyap:fix/dashboard-layout-mini-item
Apr 17, 2025
Merged

[DashboardLayout] Highlight and text alignment for mini#4844
bharatkashyap merged 3 commits into
mui:masterfrom
bharatkashyap:fix/dashboard-layout-mini-item

Conversation

@bharatkashyap

@bharatkashyap bharatkashyap commented Apr 14, 2025

Copy link
Copy Markdown
Collaborator
Before After
Screen.Recording.2025-04-16.at.2.08.56.PM.mov

@bharatkashyap bharatkashyap added the design: ui Visual design. label Apr 14, 2025
@mui-bot

mui-bot commented Apr 14, 2025

Copy link
Copy Markdown

Netlify deploy preview

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

Generated by 🚫 dangerJS against e360c1c

position: 'absolute',
bottom: -18,
left: '50%',
left: '45%',

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.

I wouldn't change this, this is what is centering the text and it goes together with transform: 'translateX(-50%)' below.
And also the font size shouldn't be what is causing the issue and it shouldn't be changed unless necessary.

Can we not just set a minimum width somewhere so that the scrollbar doesn't make things narrower?
The current changes are only trying to disguise the problem but the content is not centered.
I can take a deeper look if it helps.

@bharatkashyap

bharatkashyap commented Apr 16, 2025

Copy link
Copy Markdown
Collaborator Author

@apedroferreira I've changed the solution to use the MINI_DRAWER_WIDTH for the ul and passing scrollbar-gutter: stable to the nav when isMini is set, everything being the same in all other modes.

Screen.Recording.2025-04-16.at.2.08.56.PM.mov

@apedroferreira apedroferreira 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.

This seems a lot better, thanks a lot for the changes!

@bharatkashyap
bharatkashyap merged commit 039fe71 into mui:master Apr 17, 2025
@apedroferreira

apedroferreira commented Apr 17, 2025

Copy link
Copy Markdown
Collaborator

Looks like there's a horizontal scrollbar when the sidebar is in mini mode now: https://deploy-preview-4809--mui-toolpad-docs.netlify.app/toolpad/core/react-dashboard-layout

Maybe it'll be fixed if we remove a few pixels from the width?

@bharatkashyap

Copy link
Copy Markdown
Collaborator Author

Looks like there's a horizontal scrollbar when the sidebar is in mini mode now: https://deploy-preview-4809--mui-toolpad-docs.netlify.app/toolpad/core/react-dashboard-layout

Maybe it'll be fixed if we remove a few pixels from the width?

We could do that or hide scrollbars on the container nav of the list, which approach do you think makes more sense? Either make sense to me, although the overflow-x: hidden on the container nav seems to be more robust since we never want horizontal scrollbars to appear in the sidebar

@apedroferreira

Copy link
Copy Markdown
Collaborator

Looks like there's a horizontal scrollbar when the sidebar is in mini mode now: https://deploy-preview-4809--mui-toolpad-docs.netlify.app/toolpad/core/react-dashboard-layout
Maybe it'll be fixed if we remove a few pixels from the width?

We could do that or hide scrollbars on the container nav of the list, which approach do you think makes more sense? Either make sense to me, although the overflow-x: hidden on the container nav seems to be more robust since we never want horizontal scrollbars to appear in the sidebar

I would try to figure out why there is an overflow now - if it's related with having set a width in the items, I would try to fix something related to that. I don't mind taking a look soon.

@bharatkashyap

Copy link
Copy Markdown
Collaborator Author

Looks like there's a horizontal scrollbar when the sidebar is in mini mode now: https://deploy-preview-4809--mui-toolpad-docs.netlify.app/toolpad/core/react-dashboard-layout
Maybe it'll be fixed if we remove a few pixels from the width?

We could do that or hide scrollbars on the container nav of the list, which approach do you think makes more sense? Either make sense to me, although the overflow-x: hidden on the container nav seems to be more robust since we never want horizontal scrollbars to appear in the sidebar

I would try to figure out why there is an overflow now - if it's related with having set a width in the items, I would try to fix something related to that. I don't mind taking a look soon.

The scrollbar is due to providing an explicit width to the list now - reducing the width makes the scrollbar go away but I haven't tested this fix rigorously over all device sizes. I can open a PR with that fix and you can take it forward/review it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

design: ui Visual design.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sidebar selected item styling misalignment when scrollbar appears in collapsed state

3 participants