Skip to content

[Joy] Add Avatar component#31303

Merged
hbjORbj merged 4 commits intomui:masterfrom
hbjORbj:joy/avatar
Mar 15, 2022
Merged

[Joy] Add Avatar component#31303
hbjORbj merged 4 commits intomui:masterfrom
hbjORbj:joy/avatar

Conversation

@hbjORbj
Copy link
Contributor

@hbjORbj hbjORbj commented Mar 4, 2022

  • I have followed (at least) the PR section of the contributing guide.

  • variant: 'outlined' | 'light' | 'contained' (contained by default)

  • color: 'danger' | 'info' | 'neutral' | 'primary' | 'success' | 'warning' (neutral by default)

  • size: 'sm' | 'md' | 'lg' (md by default)

Preview: https://deploy-preview-31303--material-ui.netlify.app/experiments/joy/avatar/

@hbjORbj hbjORbj added scope: avatar Changes related to the avatar. PR: needs test The pull request needs tests. package: joy-ui Specific to Joy UI. labels Mar 4, 2022
/**
* Private module reserved for @mui packages.
*/
export default function createSvgIcon(path: React.ReactNode, displayName: string): typeof SvgIcon {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@siriwatknp Copied from your checkbox MR (#31273)

@mui-bot
Copy link

mui-bot commented Mar 4, 2022

Details of bundle changes

@mui/joy: parsed: +2.20% , gzip: +1.95%

Generated by 🚫 dangerJS against e671cc6

* @default 'md'
*/
size: PropTypes /* @typescript-to-proptypes-ignore */.oneOfType([
PropTypes.oneOf(['lg', 'md', 'sm']),
Copy link

@pahrizal pahrizal Mar 4, 2022

Choose a reason for hiding this comment

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

it looks nice shortening the size props. It would be nice too if we keep the consistency of size props like the other component?
image
wdyt @hbjORbj @DanailH @siriwatknp

Copy link
Member

Choose a reason for hiding this comment

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

@pahrizal Glad, you like it! this component is for the new design system (Joy UI) we are working on but the image you posted is Material UI, so right now it is a different package. We will try to make both of them consistent to yield the best DX in the future.

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.

Looks good overall but I am not sure about the shape prop because of #29962 (comment). cc @danilo-leal

@hbjORbj
Copy link
Contributor Author

hbjORbj commented Mar 9, 2022

Looks good overall but I am not sure about the shape prop because of #29962 (comment). cc @danilo-leal

@siriwatknp

Makes sense. We already provide sx prop and border radius can be easily configured. I will remove the shape prop.

@hbjORbj hbjORbj requested a review from siriwatknp March 10, 2022 09:25
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.

👍 Nicely done!

Copy link
Collaborator

@danilo-leal danilo-leal left a comment

Choose a reason for hiding this comment

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

Looking great so far!

@hbjORbj hbjORbj removed the PR: needs test The pull request needs tests. label Mar 15, 2022
@hbjORbj hbjORbj merged commit 6d4cf21 into mui:master Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: joy-ui Specific to Joy UI. scope: avatar Changes related to the avatar.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants