Broken avatar images

[jwbaer]

I am currently working on a bug in Frontline to do with broken customer avatar images. If a customer has an avatar image URL, we pass that to the Avatar component, and we don't send an icon. However, if the image specified by that url is missing or broken, we just get a broken image icon. I can solve this entirely on our side by checking all the images as we retrieve customer records from the CRM that is integrated with Frontline and setting them to undefined if the image is bad or doesn't exist. However, I'm wondering if it might make more sense for the Avatar component to do something in an onError handler. I'm a little curious why the Avatar component prefers an Icon, then an image, and then the name. It seems in many cases that the image would be preferred, then the name, and then the icon, with perhaps a default UserIcon or something in the case that nothing is provided. Given the fact that it is currently in use I wouldn't want to change the existing default semantics, but I wonder about adding an additional parameter - perhaps something like preferImage. Then one could specify an image and a name, or an image, name and icon and display the image if it's good but display the icon or name instead if the image load errors out. I am probably going to experiment with this anyway, but wanted to get some feedback before actually putting together a change for review. Also, if I do submit a change, is the preferred way to do it with a branch on the main repo, or from a forked repo (I'm new)?

[nkrantz]

Hi @jwbaer - so sorry for the delay on this discussion! We're taking a look and will reply ASAP.

[TheSisb]

Hi @jwbaer,

I can solve this entirely on our side by checking all the images as we retrieve customer records from the CRM that is integrated with Frontline and setting them to undefined if the image is bad or doesn't exist. However, I'm wondering if it might make more sense for the Avatar component to do something in an onError handler.

I'm not sure that the Paste Avatar component should be checking for an onLoad error automatically on images. For many users, they will be serving their own stored assets and so they have confidence the image will load. In those instances, additional onError logic would just add some performance overhead. That said, this isn't a great experience for you I'm sure.

What I would recommend would be to wrap our unopinionated Avatar component in your product as a product specific Avatar component which adds the onError logic you desire. You can do something like this: https://codesandbox.io/s/confident-chandrasekhar-10kfvd?file=/src/index.tsx Do note, there's currently a typescript issue which we will address in the next version of Paste.

I'm a little curious why the Avatar component prefers an Icon, then an image, and then the name. It seems in many cases that the image would be preferred, then the name, and then the icon, with perhaps a default UserIcon or something in the case that nothing is provided. Given the fact that it is currently in use I wouldn't want to change the existing default semantics, but I wonder about adding an additional parameter - perhaps something like preferImage. Then one could specify an image and a name, or an image, name and icon and display the image if it's good but display the icon or name instead if the image load errors out.

The current order of icon -> image -> name was selected based on our research that showed developers either used icons with fallback to text, image with fallback to text, or simply text. From what we could tell, we hardly saw anyone passing both an icon and image to the same avatar. That said, the order is just there because we had to pick something, but you can work around it by passing only the props you want to display and managing that logic outside the Avatar component.

Also, if I do submit a change, is the preferred way to do it with a branch on the main repo, or from a forked repo (I'm new)?

Our repo is locked down due to Continuous Integration so that only forks can submit PRs.

Hope this helps!

[TheSisb]

PR with the typings fix #2452