mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.75k stars 32.24k forks source link

[AvatarGroup] `spacing` prop does nothing #44172

Open hon2a opened 4 days ago

hon2a commented 4 days ago

Steps to reproduce

Link to live example: https://codesandbox.io/p/sandbox/cocky-forest-clhrvg

Steps

Attempt to use spacing in AvatarGroup, provided enum (small / medium) or number.

Current behavior

Spacing stays the same regardless of the provided value.

Expected behavior

Provided spacing should get applied.

Context

No response

Your environment

npx @mui/envinfo ``` System: OS: macOS 14.6.1 CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz Memory: 20.78 GB / 64.00 GB Shell: 5.9 - /bin/zsh Binaries: Node: 17.7.1 - ~/.nvm/versions/node/v17.7.1/bin/node Yarn: 1.22.19 - ~/.nvm/versions/node/v17.7.1/bin/yarn npm: 8.5.2 - ~/.nvm/versions/node/v17.7.1/bin/npm pnpm: 7.17.0 - ~/.nvm/versions/node/v17.7.1/bin/pnpm Managers: Homebrew: 4.2.5 - /usr/local/bin/brew pip3: 22.0.4 - /usr/local/bin/pip3 RubyGems: 3.0.3.1 - /usr/bin/gem Utilities: Make: 3.81 - /usr/bin/make GCC: 15.0.0 - /usr/bin/gcc Git: 2.39.3 - /usr/bin/git Clang: 15.0.0 - /usr/bin/clang FFmpeg: 6.1.1 - /usr/local/bin/ffmpeg Curl: 8.7.1 - /usr/bin/curl OpenSSL: 3.2.0 - /usr/local/bin/openssl Servers: Apache: 2.4.59 - /usr/sbin/apachectl IDEs: Vim: 9.0 - /usr/bin/vim WebStorm: 2024.2.2 Xcode: /undefined - /usr/bin/xcodebuild Languages: Bash: 3.2.57 - /bin/bash Java: 17.0.4.1 - /Users/jan.konopasek/.sdkman/candidates/java/current/bin/javac Perl: 5.34.1 - /usr/bin/perl Python3: 3.9.12 - /usr/local/bin/python3 Ruby: 2.6.10 - /usr/bin/ruby Databases: SQLite: 3.43.2 - /usr/bin/sqlite3 Browsers: Chrome: 129.0.6668.101 Safari: 17.6 ``` Tested in Chrome.

Search keywords: AvatarGroup spacing

mj12albert commented 1 day ago

Looks like it's just a typo in the CSS variable:

diff --git a/packages/mui-material/src/AvatarGroup/AvatarGroup.js b/packages/mui-material/src/AvatarGroup/AvatarGroup.js
index 7d07c1868d..bdb69662b3 100644
--- a/packages/mui-material/src/AvatarGroup/AvatarGroup.js
+++ b/packages/mui-material/src/AvatarGroup/AvatarGroup.js
@@ -131,7 +131,7 @@ const AvatarGroup = React.forwardRef(function AvatarGroup(inProps, ref) {
     additionalProps: {
       variant,
       style: {
-        '--AvatarRoot-spacing': marginValue ? `${marginValue}px` : undefined,
+        '--AvatarGroup-spacing': marginValue ? `${marginValue}px` : undefined,
         ...other.style,
       },
     },

@hon2a Are you interested in creating a PR?

hon2a commented 1 day ago

Sorry, I don't feel comfortable making a PR without full understanding of the context and I don't have the bandwidth for that right now.

github-actions[bot] commented 15 hours ago

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue. Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

[!NOTE] @hon2a How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

aarongarciah commented 13 hours ago

After https://github.com/mui/material-ui/pull/44202, the bug is still present. https://github.com/mui/material-ui/pull/44202 only fixed the spacing for the "surplus" avatar. You can compare the the PR version and v5 by changing adding the spacing prop in the docs demos:

aarongarciah commented 13 hours ago

The root problem: the --AvatarGroup-spacing var is defined in the surplus component when it should be defined at the AvatarGroup level: https://github.com/mui/material-ui/blob/v6.1.5/packages/mui-material/src/AvatarGroup/AvatarGroup.js#L134

The bug was introduced while migrating the AvatarGroup styles to support static extraction: https://github.com/mui/material-ui/pull/41485

aarongarciah commented 13 hours ago

Here's the fix: https://github.com/mui/material-ui/pull/44208

And here's another PR adding a spacing demo for the AvatarGroup component: https://github.com/mui/material-ui/pull/44209

mj12albert commented 5 hours ago

@aarongarciah thanks for the save 🙏