[Avatar] Add avatar component #1210

acomanescu · 2024-12-21T10:43:27Z

Preview: https://deploy-preview-1210--base-ui.netlify.app/react/components/avatar

This PR adds the Avatar component. I wasn't able to run the tests due to some Mocha run errors that happen even when I run the tests on a fresh clone.

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

Closes #1347

mui-bot · 2024-12-21T10:46:23Z

Netlify deploy preview

https://deploy-preview-1210--base-ui.netlify.app/

Generated by 🚫 dangerJS against e2dd254

mnajdova · 2024-12-24T11:49:36Z

Hey @acomanescu thanks for the contribution. We will be working soon on the roadmap for Base UI, where we will share the next components we have in the pipeline. As of now, the Avatar component is not planned, but please keep an eye on the GH Roadmap/issues if you want to contribute with something else.

If you are interested in Base UI introducing an Avatar component, please create an issue for it.

Thank you!

acomanescu · 2024-12-24T11:58:07Z

Hi @mnajdova, thank you for your response. I am closing this PR.

colmtuite · 2025-01-20T11:29:42Z

I've created an issue for Avatar and added it to the current cycle. I'll reopen this PR and take a look soon. Thanks @acomanescu!

mnajdova

Looks great, I left some initial feedback. I will let Colm review the docs demo

packages/react/src/avatar/fallback/AvatarFallback.tsx

docs/src/app/(public)/(content)/react/components/avatar/demos/hero/css-modules/index.tsx

packages/react/src/avatar/root/AvatarRoot.test.tsx

colmtuite · 2025-01-21T09:18:37Z

@acomanescu Good work 🙏

Let's use a different image. I don't have a preference, just something different.
Let's go with 48px width + height.
On Root, inline-flex would be better than flex, since it's usually an inline component.
Root will need user-select: none for when there is text content inside.
Root will need vertical-align: middle to easier accommodate alignment with other inline components and fix x-browser rendering issues with img.
I see you're using alphabetical order for properties (which is admirable), but we don't follow that convention currently. Instead, we group properties by association. So flex properties should go together, dimensions/size properties should go together etc.
Rather than the hard-coded background-color value, use var(--color-gray-100) instead.
Rather than the hard-coded color value, use var(--color-gray-900) instead.
Fallback will need a font-size. 16px should be good (but use rem). It will also need line-height: 1;
All px values should be converted to rem.

netlify · 2025-01-27T07:45:24Z

✅ Deploy Preview for base-ui ready!

Name	Link
🔨 Latest commit	`9bdba31`
🔍 Latest deploy log	https://app.netlify.com/sites/base-ui/deploys/67a0cc48e06f8300083b1485
😎 Deploy Preview	https://deploy-preview-1210--base-ui.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

dartess · 2025-01-27T12:19:54Z

We will be working soon on the roadmap for Base UI, where we will share the next components we have in the pipeline.

@mnajdova Hi! It's great that the roadmap will be shared! Is there any ETA for this? Or at least what format it will be published in? Looking forward to it.

mnajdova · 2025-01-27T13:35:41Z

@mnajdova Hi! It's great that the roadmap will be shared! Is there any ETA for this? Or at least what format it will be published in? Looking forward to it.

It will likely be shared here on GitHub as a project, but we can share more info on Discord when it'll be ready.

packages/react/src/avatar/image/useImageLoadingStatus.ts

michaldudak · 2025-01-27T14:43:36Z

packages/react/src/avatar/image/useImageLoadingStatus.ts

+
+export function useImageLoadingStatus(
+  src?: string,
+  referrerPolicy?: React.HTMLAttributeReferrerPolicy,


referrerPolicy seems unused anywhere

Fixed in cc939fc

michaldudak · 2025-01-27T14:46:29Z

packages/react/src/avatar/fallback/AvatarFallback.test.tsx

We could use a test that verifies if the fallback is displayed when the image fails to load and a one testing the delay prop.

Added in 98457eb

mj12albert · 2025-01-31T08:00:30Z

packages/react/src/avatar/image/AvatarImage.tsx

+  const {
+    className,
+    render,
+    onLoadingStatusChange: onLoadingStatusChangeProp,


onLoadingStatusChange wasn't reflected in the API docs before, fixed in df65f19

mj12albert

@colmtuite @michaldudak @mnajdova This PR should be ready to review again, I've gone through the existing comments fixed them all, also made some extra fixes.

In particular: I've removed useAvatarRoot and moved everything to the AvatarRoot component, the prop getter seems unnecessary and without that there's only a simple useState remaining 8c0e720

colmtuite · 2025-02-03T10:07:53Z

Perhaps add width and height attributes on the img? No big deal.

mj12albert · 2025-02-03T10:41:36Z

Perhaps add width and height attributes on the img? No big deal.

Added 👍

mnajdova · 2025-02-04T09:27:56Z

@dartess we updated the project with the near future plans/initiatives, you can check it here: https://github.com/orgs/mui/projects/1/views/13. We will keep updating it for each iteration.

[Avatar] Add avatar component

a54a77f

acomanescu requested review from atomiks, colmtuite and michaldudak as code owners December 21, 2024 10:43

[Avatar] Remove image src value from usage example

e2dd254

acomanescu closed this Dec 24, 2024

zannager added the component: avatar This is the name of the generic UI component, not the React module! label Dec 27, 2024

colmtuite reopened this Jan 20, 2025

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 20, 2025

mnajdova reviewed Jan 21, 2025

View reviewed changes

Merge branch 'master' into feat/avatar

6abe27a

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 27, 2025

mnajdova added 3 commits January 27, 2025 10:15

review comments

edb9578

convert to rem

34dad43

proptypes & docs:api

c8a6a51

michaldudak reviewed Jan 27, 2025

View reviewed changes

mj12albert added 6 commits January 30, 2025 19:51

Merge remote-tracking branch 'upstream/master' into feat/avatar

1a8f87b

Add a test

ad48d1e

Test the delay prop

98457eb

Fix TS

7a06444

Use noop util

416d69b

Fix referrerPolicy

cc939fc

mj12albert added 2 commits January 31, 2025 15:51

Simplify root and context

8c0e720

Fix onLoadingStatusChange missing from docs

df65f19

mj12albert reviewed Jan 31, 2025

View reviewed changes

Set font-size on avatar fallback demo

33a3ab0

mj12albert reviewed Jan 31, 2025

View reviewed changes

mj12albert requested review from michaldudak and mnajdova January 31, 2025 08:09

mj12albert added 2 commits January 31, 2025 16:11

Fix eslint

c6eb5e2

docs

83870fe

mj12albert force-pushed the feat/avatar branch from f129e6f to 83870fe Compare January 31, 2025 08:41

mj12albert added 2 commits January 31, 2025 20:30

Merge branch 'master' into feat/avatar

2d75eb6

Merge branch 'master' into feat/avatar

8e23e1f

colmtuite approved these changes Feb 3, 2025

View reviewed changes

Set width and height attrs in demo

5be8d7f

michaldudak approved these changes Feb 3, 2025

View reviewed changes

Fix style hooks

9bdba31

mj12albert merged commit 37590c5 into mui:master Feb 4, 2025
22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Avatar] Add avatar component #1210

[Avatar] Add avatar component #1210

acomanescu commented Dec 21, 2024 •

edited by mj12albert

Loading

mui-bot commented Dec 21, 2024 •

edited

Loading

mnajdova commented Dec 24, 2024

acomanescu commented Dec 24, 2024

colmtuite commented Jan 20, 2025

mnajdova left a comment

colmtuite commented Jan 21, 2025

netlify bot commented Jan 27, 2025 •

edited

Loading

dartess commented Jan 27, 2025

mnajdova commented Jan 27, 2025

michaldudak Jan 27, 2025

mj12albert Jan 31, 2025

michaldudak Jan 27, 2025

mj12albert Jan 31, 2025

mj12albert Jan 31, 2025

mj12albert left a comment

colmtuite commented Feb 3, 2025

mj12albert commented Feb 3, 2025

mnajdova commented Feb 4, 2025

[Avatar] Add avatar component #1210

[Avatar] Add avatar component #1210

Conversation

acomanescu commented Dec 21, 2024 • edited by mj12albert Loading

mui-bot commented Dec 21, 2024 • edited Loading

Netlify deploy preview

mnajdova commented Dec 24, 2024

acomanescu commented Dec 24, 2024

colmtuite commented Jan 20, 2025

mnajdova left a comment

Choose a reason for hiding this comment

colmtuite commented Jan 21, 2025

netlify bot commented Jan 27, 2025 • edited Loading

✅ Deploy Preview for base-ui ready!

dartess commented Jan 27, 2025

mnajdova commented Jan 27, 2025

michaldudak Jan 27, 2025

Choose a reason for hiding this comment

mj12albert Jan 31, 2025

Choose a reason for hiding this comment

michaldudak Jan 27, 2025

Choose a reason for hiding this comment

mj12albert Jan 31, 2025

Choose a reason for hiding this comment

mj12albert Jan 31, 2025

Choose a reason for hiding this comment

mj12albert left a comment

Choose a reason for hiding this comment

colmtuite commented Feb 3, 2025

mj12albert commented Feb 3, 2025

mnajdova commented Feb 4, 2025

acomanescu commented Dec 21, 2024 •

edited by mj12albert

Loading

mui-bot commented Dec 21, 2024 •

edited

Loading

netlify bot commented Jan 27, 2025 •

edited

Loading