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

[AvatarGroup] spacing prop does nothing #44172

Closed
hon2a opened this issue Oct 21, 2024 · 8 comments · Fixed by #44202 or #44208
Closed

[AvatarGroup] spacing prop does nothing #44172

hon2a opened this issue Oct 21, 2024 · 8 comments · Fixed by #44202 or #44208
Assignees
Labels
component: avatar This is the name of the generic UI component, not the React module! regression A bug, but worse

Comments

@hon2a
Copy link

hon2a commented Oct 21, 2024

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

@hon2a hon2a added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Oct 21, 2024
@zannager zannager added the component: avatar This is the name of the generic UI component, not the React module! label Oct 21, 2024
@mj12albert
Copy link
Member

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?

@mj12albert mj12albert added bug 🐛 Something doesn't work and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Oct 24, 2024
@mj12albert mj12albert changed the title AvatarGroup spacing does nothing [AvatarGroup] spacing prop does nothing Oct 24, 2024
@hon2a
Copy link
Author

hon2a commented Oct 24, 2024

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.

@mj12albert mj12albert added the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label Oct 24, 2024
@mj12albert mj12albert removed the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label Oct 24, 2024
@siriwatknp siriwatknp added regression A bug, but worse and removed bug 🐛 Something doesn't work labels Oct 25, 2024
Copy link

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
Copy link
Member

After #44202, the bug is still present. #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 aarongarciah reopened this Oct 25, 2024
@aarongarciah
Copy link
Member

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: #41485

@aarongarciah
Copy link
Member

Here's the fix: #44208

And here's another PR adding a spacing demo for the AvatarGroup component: #44209

@mj12albert
Copy link
Member

@aarongarciah thanks for the save 🙏

Copy link

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.

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! regression A bug, but worse
Projects
None yet
5 participants