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] Fix app bar items alignment #4437

Merged
merged 7 commits into from
Nov 19, 2024

Conversation

bharatkashyap
Copy link
Member

Before After
Screenshot 2024-11-18 at 6 53 12 PM Screenshot 2024-11-18 at 6 43 41 PM

@bharatkashyap bharatkashyap added design: ui Design component: layout This is the name of the generic UI component, not the React module! labels Nov 18, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 18, 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 18, 2024
Copy link
Member

@apedroferreira apedroferreira left a comment

Choose a reason for hiding this comment

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

Requesting changes just for adding alignItems to the stack that wraps the toolbar slots too.
Thanks for looking into this! I had missed it as I hasn't tested it with the account button in the top bar.

flexWrap: 'wrap',
justifyContent: 'space-between',
width: '100%',
alignItems: 'center',
Copy link
Member

Choose a reason for hiding this comment

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

I don't remember exactly why I had added the padding but just retested and it should be fine to remove...

Copy link
Member

Choose a reason for hiding this comment

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

Also for justifyContent and alignItems I guess we could just use the props of Stack directly, but it shouldn't be a big difference.

@@ -14,7 +14,7 @@ import { ThemeSwitcher } from './ThemeSwitcher';
*/
function ToolbarActions() {
return (
<Stack direction="row">
<Stack direction="row" alignItems="center">
Copy link
Member

@apedroferreira apedroferreira Nov 18, 2024

Choose a reason for hiding this comment

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

Besides this, can we also add this prop in the DashboardLayout line 377, in the Stack that wraps the actions and account slots?
(we actually have to so that they're aligned too)

pnpm-lock.yaml Outdated
@@ -4036,7 +4036,7 @@ packages:
engines: {node: '>=18'}
peerDependencies:
'@testing-library/dom': ^10.0.0
'@types/react': ^18.3.11
'@types/react': ^18.3.12
Copy link
Member

Choose a reason for hiding this comment

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

There should be no changes here, right?

@bharatkashyap bharatkashyap merged commit 94f9652 into mui:master Nov 19, 2024
14 checks passed
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! design: ui Design
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants