Skip to content

[DashboardLayout] Left-align header title in mobile viewport#4346

Merged
apedroferreira merged 9 commits intomui:masterfrom
apedroferreira:fix-layout-header-overlap
Nov 8, 2024
Merged

[DashboardLayout] Left-align header title in mobile viewport#4346
apedroferreira merged 9 commits intomui:masterfrom
apedroferreira:fix-layout-header-overlap

Conversation

@apedroferreira
Copy link
Collaborator

@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 type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or 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
Collaborator

  • 🤔 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.

@apedroferreira
Copy link
Collaborator 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
Contributor

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
Contributor

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

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.

Customisation toolbar overlap

5 participants