[material-ui][Avatar] Improve fallback when children is empty string or boolean

[mirus-ua]

I faced an unexpected behavior when our BE service didn't return a valid image URL, and my Avatar component showed me an empty circle with a background. After a quick investigation, I found that we may pass both a URL and children as a fallback in some cases in the project. If both of them are falsy, from the UX perspective, it leads to unexpected results, at least according to the current documentation.

You can check it in the current version of the library with the following code snippet.

function App() {
  const firstName = undefined;
  const lastName = undefined;
  const children = "";
  const src = 'invalid'

  const userInitials = getUserInitials(firstName, lastName)

  const alt = typeof children === 'string' ? children : userInitials;

  return (
      <Avatar src={src} alt={alt}>{children}</Avatar>
  )
}

I don't see any reason why this slight quality-of-life improvement could be wrong, but I gladly accept any feedback.

• I have followed (at least) the PR section of the contributing guide.
• NX Successfully ran target test for project @mui/material

[mui-bot]

Netlify deploy preview

https://deploy-preview-40766--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against f61a104

[mirus-ua]

@oliviertassinari hello

I found an example of how you handle expected errors here: packages/mui-material/src/Avatar/Avatar.test.js
So right now, I covered an empty string and a boolean value case + added tests for them.
Do you have any new thoughts about the PR?

BTW ci/circleci: test_unit-1 failed with an error in a package where I didn't do any changes -- @mui/envinfo

[ZeeshanTamboli]

Should I remove the test or extend it so that we expect a console error? If the last is correct, could you reference how similar cases were handled in the codebase?

We should remove the test. It's not needed. The PropType warning is likely because true is not a valid React node for the children prop, unlike strings or null. React considers false as a valid React node, rendering nothing.

[mirus-ua]

Should I remove the test or extend it so that we expect a console error? If the last is correct, could you reference how similar cases were handled in the codebase?

We should remove the test. It's not needed. The PropType warning is likely because true is not a valid React node for the children prop, unlike strings or null. React considers false as a valid React node, rendering nothing.

Done

[ZeeshanTamboli]

Looks good!

[oliviertassinari]

I think we can shorten the condition without making any test fail: #40834.

[DiegoAndai]

Hey! We've been discussing this change at #41291. You might want to check that out and join the discussion.