Skip to content

[AvatarGroup] Fix spacing prop ignoring value 0 #45799

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

Merged

Conversation

Kartik-Murthy
Copy link
Contributor

@Kartik-Murthy Kartik-Murthy commented Apr 2, 2025

PR Description:

This PR fixes #45797 where the AvatarGroup component in MUI v7 ignores spacing={0} and defaults to the standard spacing. This occurred because 0 was incorrectly treated as false.

Changes:

  • Updated the AvatarGroup component to handle spacing={0} correctly.

  • Ensured that 0 is not treated as false, allowing no spacing between avatars.

  • Passing Infinity, -Infinity, NaN, -NaN, 1/NaN and undefined will default to medium spacing: -8px

This fix restores expected behavior for spacing={0} in the MUI v7.

Additional

Before: https://codesandbox.io/p/devbox/agitated-johnson-5yx8t9
After fix: https://codesandbox.io/p/sandbox/material-ui-cra-ts-forked-8fmqpp

@mui-bot
Copy link

mui-bot commented Apr 2, 2025

Netlify deploy preview

https://deploy-preview-45799--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 2e53b3f

@Kartik-Murthy Kartik-Murthy force-pushed the fix/avatar-group-spacing-zero branch 2 times, most recently from 60648c3 to feaffba Compare April 3, 2025 11:51
@zannager zannager added the component: avatar This is the name of the generic UI component, not the React module! label Apr 3, 2025
@zannager zannager requested a review from aarongarciah April 3, 2025 12:03
@Kartik-Murthy Kartik-Murthy force-pushed the fix/avatar-group-spacing-zero branch from feaffba to 84685fe Compare April 3, 2025 18:02
@ZeeshanTamboli ZeeshanTamboli changed the title [material-ui][AvatarGroup] Fix: Ensure AvatarGroup Respects spacing={0} in v7 [AvatarGroup] spacing prop should accept value 0 Apr 4, 2025
@ZeeshanTamboli ZeeshanTamboli changed the title [AvatarGroup] spacing prop should accept value 0 [AvatarGroup] Fix spacing prop not accepting value 0 Apr 4, 2025
@ZeeshanTamboli ZeeshanTamboli added bug 🐛 Something doesn't work package: material-ui Specific to @mui/material regression 🐛 A bug, but worse labels Apr 4, 2025
Copy link
Member

@ZeeshanTamboli ZeeshanTamboli left a comment

Choose a reason for hiding this comment

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

Left 2 comments — thanks for the contribution! Could you add a test case for when spacing is 0? It should result in marginLeft: 0px.

@ZeeshanTamboli ZeeshanTamboli added v6.x needs cherry-pick The PR should be cherry-picked to master after merge labels Apr 4, 2025
@ZeeshanTamboli ZeeshanTamboli changed the title [AvatarGroup] Fix spacing prop not accepting value 0 [AvatarGroup] Fix spacing prop ignoring value 0 Apr 4, 2025
@Kartik-Murthy Kartik-Murthy force-pushed the fix/avatar-group-spacing-zero branch from 01ae186 to 84a8888 Compare April 4, 2025 14:28
@Kartik-Murthy Kartik-Murthy force-pushed the fix/avatar-group-spacing-zero branch from 84a8888 to bb9b936 Compare April 5, 2025 05:35
- Update AvatarGroup spacing prop logic to handle 0 as a valid value.
- Ensures that 0 behaves as expected without causing layout issues.
- Restore original import order as per project conventions
@Kartik-Murthy Kartik-Murthy force-pushed the fix/avatar-group-spacing-zero branch from bb9b936 to 00280f8 Compare April 7, 2025 04:13
@Kartik-Murthy
Copy link
Contributor Author

Thanks @ZeeshanTamboli for reviewing! I’ve added the test case and implemented the changes you suggested. Let me know if anything else is needed.

@aarongarciah aarongarciah requested review from siriwatknp and removed request for aarongarciah April 7, 2025 09:32
Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

Copy link
Member

@ZeeshanTamboli ZeeshanTamboli left a comment

Choose a reason for hiding this comment

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

@Kartik-Murthy Nice work, thanks!

@ZeeshanTamboli ZeeshanTamboli enabled auto-merge (squash) April 8, 2025 06:19
@ZeeshanTamboli ZeeshanTamboli merged commit b7dc1bc into mui:master Apr 8, 2025
19 checks passed
Copy link

github-actions bot commented Apr 8, 2025

Cherry-pick PRs will be created targeting branches: v6.x

github-actions bot pushed a commit that referenced this pull request Apr 8, 2025
@oliviertassinari oliviertassinari removed the bug 🐛 Something doesn't work label Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: avatar This is the name of the generic UI component, not the React module! needs cherry-pick The PR should be cherry-picked to master after merge package: material-ui Specific to @mui/material regression 🐛 A bug, but worse v6.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[AvatarGroup] AvatarGroup spacing prop no longer accepts value 0
6 participants