Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avatar: Fallback to label or icon variant when image loading fails #3647

Closed
KOliver94 opened this issue Nov 15, 2022 · 4 comments · Fixed by #3648
Closed

Avatar: Fallback to label or icon variant when image loading fails #3647

KOliver94 opened this issue Nov 15, 2022 · 4 comments · Fixed by #3648
Assignees
Labels
Type: New Feature Issue contains a new feature or new component request
Milestone

Comments

@KOliver94
Copy link
Contributor

Describe the feature you would like to see added

It would be great if the Avatar component would have some kind of default fallback functionality. Currently, if I specify the image property it will always be rendered as <img src={props.image} alt={props.imageAlt} onError={props.onImageError}></img> even if the image cannot be loaded. The only option to overcome this issue is to define a function for onError which I can use to change the event.target.src but if I don't have a fallback image and I want to use an icon just like the component would do I need to change then event.target.outerHTML which is complicated and will not follow PrimeReact changes.

Is your feature request related to a problem?

No response

Describe the solution you'd like

MUI uses the following solution to try to load images before render:
https://github.com/mui/material-ui/blob/5eb6caf89e763f346e5c57494d6aa45eff17d3b3/packages/mui-material/src/Avatar/Avatar.js#L92-L129

Based on that they decide what to render:
https://github.com/mui/material-ui/blob/5eb6caf89e763f346e5c57494d6aa45eff17d3b3/packages/mui-material/src/Avatar/Avatar.js#L150-L180

In my opinion this can be an opt-out feature for example defining a new prop defaultFallback with true as default value and change this line:


to

if (props.image && (!prop.defaultFallback || (prop.defaultFallback && imageLoaded))) {

However, this solution as opt-out can be a breaking change for those who use onError to solve image loading issues.

Describe alternatives you have considered

No response

Additional context

No response

@KOliver94 KOliver94 added Status: Discussion Issue or pull request needs to be discussed by Core Team Type: New Feature Issue contains a new feature or new component request labels Nov 15, 2022
@github-actions github-actions bot added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Nov 15, 2022
@melloware
Copy link
Member

I like this idea. I have also used a couple of other options in the past like this as a fallback URL: https://ui-avatars.com/

@melloware melloware removed Status: Discussion Issue or pull request needs to be discussed by Core Team Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Nov 15, 2022
@melloware
Copy link
Member

melloware commented Nov 15, 2022

Actually I think I want to do this...

<Avatar image={`invalid.jpg`} imageFallback={`https://ui-avatars.com/api/?name=Fall+Back`} />

Give you an imageFallback URL so that would do this..

image

Thoughts?

This way its NOT a breaking change on users already using onImageError.

melloware added a commit to melloware/primereact that referenced this issue Nov 15, 2022
@melloware melloware self-assigned this Nov 15, 2022
@melloware melloware added this to the 8.7.3 milestone Nov 15, 2022
@melloware
Copy link
Member

Please review my PR.

melloware added a commit to melloware/primereact that referenced this issue Nov 15, 2022
@melloware
Copy link
Member

OK i also allow a default as you stated where it will fallback to label then icon if you want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: New Feature Issue contains a new feature or new component request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants