Material-ui: [Avatar] Improve image loading and fallback logic

[mschipperheyn]

The Avatar shows either an image or a child (text or icon). I would like it show the child temporarily while the image is loading (of provided).

• [x ] This is not a v0.x issue.
• [ x] I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 🤔

Letter Avatar should be used as a fallback for when the image is loading or when image loading fails. This is buildable outside of material-ui, but it seems like a sensible default to me.

Current Behavior 😯

nothing is shown while the image is loading

[oliviertassinari]

@mschipperheyn Thank you for opening this request. I don't think that the letter should be displayed when the image is loading.

There is a talk on this topic (React conf 2019): https://www.youtube.com/watch?v=KT3XKDBZW7M. Basically, you want to wait for all the images to be loaded to display them at once. The browser implements a close behavior with the initial server-side render. In the event, the batching feature is not available, I think that it's better to display a skeleton-like loading background-color rather than a rich element.

However, I think that we should fallback when the image fails to load.
What do you think of this diff?

diff --git a/packages/material-ui/src/Avatar/Avatar.js b/packages/material-ui/src/Avatar/Avatar.js
index 95347e73d..a0f840844 100644
--- a/packages/material-ui/src/Avatar/Avatar.js
+++ b/packages/material-ui/src/Avatar/Avatar.js
@@ -2,6 +2,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import clsx from 'clsx';
 import withStyles from '../styles/withStyles';
+import Person from '../internal/svg-icons/Person';

 export const styles = theme => ({
   /* Styles applied to the root element. */
@@ -20,7 +21,7 @@ export const styles = theme => ({
     overflow: 'hidden',
     userSelect: 'none',
   },
-  /* Styles applied to the root element if there are children and not `src` or `srcSet`. */
+  /* Styles applied to the root element if not `src` or `srcSet`. */
   colorDefault: {
     color: theme.palette.background.default,
     backgroundColor:
@@ -43,9 +44,55 @@ export const styles = theme => ({
     textAlign: 'center',
     // Handle non-square image. The property isn't supported by IE 11.
     objectFit: 'cover',
+    // Hide alt text.
+    color: 'transparent',
+    // Same color as the Skeleton.
+    backgroundColor: theme.palette.action.hover,
+    // Hide the image broken icon, only works on Chrome.
+    textIndent: 10000,
+  },
+  /* Styles applied to the fallback icon */
+  fallback: {
+    width: '80%',
+    height: '80%',
   },
 });

+const useLoaded = ({ src, srcSet }) => {
+  const [loaded, setLoaded] = React.useState(false);
+
+  React.useEffect(() => {
+    if (!src && !srcSet) {
+      return undefined;
+    }
+
+    setLoaded(false);
+
+    let active = true;
+    const image = new Image();
+    image.src = src;
+    image.srcSet = srcSet;
+    image.onload = () => {
+      if (!active) {
+        return;
+      }
+      setLoaded('loaded');
+    };
+    image.onerror = () => {
+      if (!active) {
+        return;
+      }
+      setLoaded('error');
+    };
+
+    return () => {
+      active = false;
+    };
+  }, [src, srcSet]);
+
+  return loaded;
+};
+
 const Avatar = React.forwardRef(function Avatar(props, ref) {
   const {
     alt,
@@ -62,9 +109,12 @@ const Avatar = React.forwardRef(function Avatar(props, ref) {
   } = props;

   let children = null;
-  const img = src || srcSet;
+  // Use a hook instead of onError on the img element to support server-side rendering.
+  const loaded = useLoaded({ src, srcSet });
+  const hasImg = src || srcSet;
+  const hasImgNotFailing = hasImg && loaded !== 'error';

-  if (img) {
+  if (hasImgNotFailing) {
     children = (
       <img
         alt={alt}
@@ -75,8 +125,12 @@ const Avatar = React.forwardRef(function Avatar(props, ref) {
         {...imgProps}
       />
     );
-  } else {
+  } else if (childrenProp) {
     children = childrenProp;
+  } else if (hasImg && alt) {
+    children = alt[0];
+  } else {
+    children = <Person className={classes.fallback} />;
   }

   return (
@@ -86,7 +140,7 @@ const Avatar = React.forwardRef(function Avatar(props, ref) {
         classes.system,
         classes[variant],
         {
-          [classes.colorDefault]: !img,
+          [classes.colorDefault]: !hasImgNotFailing,
         },
         className,
       )}
@@ -124,8 +178,8 @@ Avatar.propTypes = {
    */
   component: PropTypes.elementType,
   /**
-   * Attributes applied to the `img` element if the component
-   * is used to display an image.
+   * Attributes applied to the `img` element if the component is used to display an image.
+   * It can be used to listen for the loading error event.
    */
   imgProps: PropTypes.object,
   /**

packages/material-ui/src/internal/svg-icons/Person.js

import React from 'react';
import createSvgIcon from './createSvgIcon';

/**
 * @ignore - internal component.
 */
export default createSvgIcon(
  <path d="M12 12c2.21 0 4-1.79 4-4s-1.79-4-4-4-4 1.79-4 4 1.79 4 4 4zm0 2c-2.67 0-8 1.34-8 4v2h16v-2c0-2.66-5.33-4-8-4z" />
, 'Person');

---

Taking the following test case, I have recorded the output before and after, hopefully, it's better :)

import React from 'react';
import { makeStyles } from '@material-ui/core/styles';
import Avatar from '@material-ui/core/Avatar';

const useStyles = makeStyles(theme => ({
  root: {
    display: 'flex',
    '& > *': {
      margin: theme.spacing(1),
    },
  },
  bigAvatar: {
    width: 60,
    height: 60,
  },
}));

export default function ImageAvatars() {
  const classes = useStyles();

  return (
    <div className={classes.root}>
      <Avatar />
      <Avatar variant="square" />
      <Avatar alt="Remy Sharp" src="/static/images/avatar/1.jpg" />
      <Avatar alt="Remy Sharp" src="/static/images/avatar/112.jpg">H</Avatar>
      <Avatar alt="Remy Sharp" src="/static/images/avatar/112.jpg" />
      <Avatar src="/static/images/avatar/112.jpg" />
      <Avatar alt="Dan Abrahmov" src="https://bit.ly/dan-abramov" />
      <Avatar alt="Kola Tioluwani" src="https://bit.ly/tioluwani-kolawole" />
      <Avatar alt="Kent Dodds" src="https://bit.ly/kent-c-dodds" />
      <Avatar alt="Ryan Florence" src="https://bit.ly/ryan-florence" />
      <Avatar alt="Prosper Otemuyiwa" src="https://bit.ly/prosper-baba" />
      <Avatar alt="Christian Nwamba" src="https://bit.ly/code-beast" />
      <Avatar alt="Segun Adebayo" src="https://bit.ly/sage-adebayo" />
      <Avatar imgProps={{ onError: console.log }} alt="Remy Sharp" src="/static/images/avatar/112.jpg" />
      <Avatar alt="Remy Sharp" src="/static/images/avatar/1.jpg" className={classes.bigAvatar} />
    </div>
  );
}

Before

After

Do you want to submit a pull request? :) Also, I think that it would be great to add a new section in the documentation about this fallback mechanism, it would also be an opportunity to mention how to use the onError method.

[netochaves]

Can i take this?

[oliviertassinari]

@netochaves Sure :)