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

Fix spacing for ListItemIcon and ListItemAvatar #3416

Merged
merged 6 commits into from
Feb 18, 2025

Conversation

juliawegmayr
Copy link
Contributor

@juliawegmayr juliawegmayr commented Feb 12, 2025

Description

Refactoring the Menu styling (#3346) caused bugs in the styling of ListItemIcons. Add a gap to match the design.

Acceptance criteria

Screenshots/screencasts

Before After
Screenshot 2025-02-12 at 12 38 45 Screenshot 2025-02-18 at 09 47 48
Screenshot 2025-02-12 at 12 38 53 Screenshot 2025-02-18 at 09 47 10

Open TODOs/questions

  • Add changeset

Further information

@juliawegmayr juliawegmayr self-assigned this Feb 12, 2025
@juliawegmayr juliawegmayr force-pushed the fix-menu-styling-for-addBlockDrawer branch from 2f74841 to 5fd1ebe Compare February 12, 2025 11:44
@juliawegmayr juliawegmayr marked this pull request as ready for review February 12, 2025 11:44
@auto-assign auto-assign bot requested a review from johnnyomair February 12, 2025 11:45
@juliawegmayr
Copy link
Contributor Author

@jamesricky @johnnyomair I know, that Ricky suggested to move the styling from MenuItem to ListItemIcon to avoid different styles (here: #2919 (comment)). But as far as I can tell, we need them to be different at the moment (see screenshots). 🤔

@thomasdax98 thomasdax98 added this to the v7.14.0 milestone Feb 12, 2025
Copy link
Collaborator

@johnnyomair johnnyomair left a comment

Choose a reason for hiding this comment

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

Should the text in the content scope select really have that much spacing on the left? Is it like this in the design?

@johnnyomair johnnyomair changed the title style MuiListItemIcon in MuiMenuItem Style MuiListItemIcon in MuiMenuItem Feb 13, 2025
@juliawegmayr juliawegmayr marked this pull request as draft February 17, 2025 15:27
@juliawegmayr juliawegmayr force-pushed the fix-menu-styling-for-addBlockDrawer branch from 8ce978c to 4edeb10 Compare February 18, 2025 08:51
@juliawegmayr
Copy link
Contributor Author

Should the text in the content scope select really have that much spacing on the left? Is it like this in the design?

No it shouldn't. Thank you for the hint.

I checked the design and the usage of ListItemIcon again and I found out that my assumption was wrong and that all ListItemIcons are styled incorrectly. A better solution for fixing the ListItemIcon to match the design is to add a gap. I reverted my changes, and also adapted the description of this PR with updated screenshots. (Thank you @jamesricky for the idea with the gap :) )

@juliawegmayr juliawegmayr marked this pull request as ready for review February 18, 2025 09:01
@auto-assign auto-assign bot requested a review from johnnyomair February 18, 2025 09:01
@johnnyomair johnnyomair changed the title Style MuiListItemIcon in MuiMenuItem Fix spacing for ListItemIcon and ListItemAvatar Feb 18, 2025
johnnyomair
johnnyomair previously approved these changes Feb 18, 2025
@juliawegmayr juliawegmayr force-pushed the fix-menu-styling-for-addBlockDrawer branch from ff72249 to 3e0a64a Compare February 18, 2025 14:47
@johnnyomair johnnyomair merged commit 9b190db into main Feb 18, 2025
11 checks passed
@johnnyomair johnnyomair deleted the fix-menu-styling-for-addBlockDrawer branch February 18, 2025 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants