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] Left-align header title in mobile viewport #4346

Merged
merged 9 commits into from
Nov 8, 2024

Conversation

apedroferreira
Copy link
Member

@apedroferreira apedroferreira commented Nov 1, 2024

Closes #4111

Screenshot 2024-11-01 at 20 03 24

Previously, the DashboardLayout header title in mobile viewports was center-aligned with absolute positioning.
However, when using the toolbarActions slot to add items to the header, problems with overlapping such as the one reported in #4111 could occur.

Simply using a statically positioned, left-aligned header title, just like the tablet and desktop viewports do, should solve this problem and even make more space available for additional actions in the header.

https://deploy-preview-4346--mui-toolpad-docs.netlify.app/toolpad/core/react-dashboard-layout

@apedroferreira apedroferreira added enhancement This is not a bug, nor a new feature component: DashboardLayout labels Nov 1, 2024
@apedroferreira apedroferreira self-assigned this Nov 1, 2024
@Janpot
Copy link
Member

Janpot commented Nov 1, 2024

There's something wrong with the icon button in codesandbox:

Screenshot 2024-11-01 at 21 49 05

Trying out the overflow behavior:

Screen.Recording.2024-11-01.at.21.50.29.mov

Some observations:

  • The theme toggle stays aligned at the top (yet it does shift somewhat vertically)
  • The title/menu stays center aligned.
  • 🤔 I thought the theme toggle would (should) be part of the toolbar actions.

@bharatkashyap
Copy link
Member

  • 🤔 I thought the theme toggle would (should) be part of the toolbar actions.

That will happen with #4340

@Janpot
Copy link
Member

Janpot commented Nov 2, 2024

My expectation of overflow would differ than this implementation. I think the actions should wrap, the same way it does in the PageContainer:

Screen.Recording.2024-11-02.at.13.59.19.mov

We could even consider accepting an array of actions so that we can wrap them separately.

edit: just noticed this is currently broken. It misses a flex-wrap. I fixed it in the devtools for this video.

@oliviertassinari oliviertassinari added component: layout This is the name of the generic UI component, not the React module! and removed component: DashboardLayout labels Nov 4, 2024
@apedroferreira
Copy link
Member Author

apedroferreira commented Nov 6, 2024

My expectation of overflow would differ than this implementation. I think the actions should wrap, the same way it does in the PageContainer:

Screen.Recording.2024-11-02.at.13.59.19.mov
We could even consider accepting an array of actions so that we can wrap them separately.

edit: just noticed this is currently broken. It misses a flex-wrap. I fixed it in the devtools for this video.

Good feedback! Hadn't checked that.
It made it so it wraps now, the marginLeft trick seems to do it!

Screen.Recording.2024-11-05.at.18.42.01.mov

We could even consider accepting an array of actions so that we can wrap them separately.

I gave it a brief attempt but wrapping and aligning multiple items to the right doesn't seem as straightforward, plus the account button is already a separate slot and not sure if we want it to wrap separately from the actions. So I guess we could keep that for later?

@prakhargupta1
Copy link
Member

I just noticed a different approach to handle this and thought of sharing. Here the app name gets trimmed.

Screen.Recording.2024-11-06.at.10.12.11.AM.mov

@Janpot
Copy link
Member

Janpot commented Nov 6, 2024

Here the app name gets trimmed

This doesn't feel like a fair comparison. that app doesn't optimize for mobile.

@prakhargupta1
Copy link
Member

This doesn't feel like a fair comparison. that app doesn't optimize for mobile.

Hmm agree that's not the mobile view.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 7, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 7, 2024
@apedroferreira apedroferreira merged commit 8b2f4cc into mui:master Nov 8, 2024
14 checks passed
@apedroferreira apedroferreira deleted the fix-layout-header-overlap branch November 8, 2024 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: layout This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Customisation toolbar overlap
5 participants