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] Enable targeting of additional Avatar when max props is passed #30794

Merged
merged 6 commits into from
Feb 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/pages/api-docs/avatar-group.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
"props": {
"children": { "type": { "name": "node" } },
"classes": { "type": { "name": "object" } },
"componentsProps": {
"type": { "name": "shape", "description": "{ additionalAvatar?: object }" },
"default": "{}"
},
"max": { "type": { "name": "custom", "description": "number" }, "default": "5" },
"spacing": {
"type": {
Expand Down
1 change: 1 addition & 0 deletions docs/translations/api-docs/avatar-group/avatar-group.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"propDescriptions": {
"children": "The avatars to stack.",
"classes": "Override or extend the styles applied to the component. See <a href=\"#css\">CSS API</a> below for more details.",
"componentsProps": "The props used for each slot inside the AvatarGroup.",
"max": "Max avatars to show before +x.",
"spacing": "Spacing between avatars.",
"sx": "The system prop that allows defining system overrides as well as additional CSS styles. See the <a href=\"/system/the-sx-prop/\">`sx` page</a> for more details.",
Expand Down
10 changes: 10 additions & 0 deletions packages/mui-material/src/AvatarGroup/AvatarGroup.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ import { InternalStandardProps as StandardProps, Theme } from '@mui/material';
import { OverridableStringUnion } from '@mui/types';
import { SxProps } from '@mui/system';
import { AvatarGroupClasses } from './avatarGroupClasses';
import Avatar from '../Avatar';

export interface AvatarGroupPropsVariantOverrides {}

export interface AvatarGroupComponentsPropsOverrides {}
export interface AvatarGroupProps extends StandardProps<React.HTMLAttributes<HTMLDivElement>> {
/**
* The avatars to stack.
Expand All @@ -15,6 +17,14 @@ export interface AvatarGroupProps extends StandardProps<React.HTMLAttributes<HTM
* Override or extend the styles applied to the component.
*/
classes?: Partial<AvatarGroupClasses>;
/**
* The props used for each slot inside the AvatarGroup.
* @default {}
*/
componentsProps?: {
additionalAvatar?: React.ComponentPropsWithRef<typeof Avatar> &
AvatarGroupComponentsPropsOverrides;
};
/**
* Max avatars to show before +x.
* @default 5
Expand Down
15 changes: 11 additions & 4 deletions packages/mui-material/src/AvatarGroup/AvatarGroup.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ const AvatarGroup = React.forwardRef(function AvatarGroup(inProps, ref) {
const {
children: childrenProp,
className,
componentsProps = {},
max = 5,
spacing = 'medium',
total,
Expand Down Expand Up @@ -122,11 +123,10 @@ const AvatarGroup = React.forwardRef(function AvatarGroup(inProps, ref) {
{extraAvatars ? (
<AvatarGroupAvatar
ownerState={ownerState}
className={classes.avatar}
style={{
marginLeft,
}}
variant={variant}
{...componentsProps.additionalAvatar}
Copy link
Member

Choose a reason for hiding this comment

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

One thing I noticed during testing is that the provided componentsProps override the existing className.

We should merge the class names instead. Same goes for the style prop:

<AvatarGroupAvatar
  ownerState={ownerState}
  variant={variant}
  {...componentsProps.additionalAvatar}
  className={clsx(classes.avatar, componentsProps.additionalAvatar?.className)}
  style={{ marginLeft, ...componentsProps.additionalAvatar?.style }}
>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that makes sense

className={clsx(classes.avatar, componentsProps.additionalAvatar?.className)}
style={{ marginLeft, ...componentsProps.additionalAvatar?.style }}
>
+{extraAvatars}
</AvatarGroupAvatar>
Expand Down Expand Up @@ -165,6 +165,13 @@ AvatarGroup.propTypes /* remove-proptypes */ = {
* @ignore
*/
className: PropTypes.string,
/**
* The props used for each slot inside the AvatarGroup.
* @default {}
*/
componentsProps: PropTypes.shape({
additionalAvatar: PropTypes.object,
}),
/**
* Max avatars to show before +x.
* @default 5
Expand Down
16 changes: 16 additions & 0 deletions packages/mui-material/src/AvatarGroup/AvatarGroup.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,22 @@ describe('<AvatarGroup />', () => {
expect(container.textContent).to.equal('+2');
});

it('should pass props from componentsProps.additionalAvatar to the slot component', () => {
const componentsProps = { additionalAvatar: { className: 'additional-avatar-test' } };

const { container } = render(
<AvatarGroup max={3} componentsProps={componentsProps}>
<Avatar src="/fake.png" />
<Avatar src="/fake.png" />
<Avatar src="/fake.png" />
<Avatar src="/fake.png" />
</AvatarGroup>,
);

const additionalAvatar = container.querySelector('.additional-avatar-test');
expect(additionalAvatar.classList.contains('additional-avatar-test')).to.equal(true);
});

it('should respect total', () => {
const { container } = render(
<AvatarGroup total={10}>
Expand Down