Bug/69230 Handle 404 errors in AvatarWithFallback with client-side fallback

[akabiru]

https://community.openproject.org/wp/69230

Server-side URL validation would add latency and can't handle relative URLs without knowing the host. Hence, we use client-side error handling:

• Always wrap avatars in element with pre-generated SVG stored in data-fallback-src attribute
• Listen for img error events to swap broken images to fallback SVG
• Check if loading completed but no actual image data loaded to catch errors that fired before the listener was attached (race condition)

Warning

There is a brief flicker from client-side based fallback- not sure if there's a way around this issue. One idea is switching to server-size validation, but requires the component to know about the host. As this is a fallback mechanism for broken images, I suppose this flicker is acceptable.

[changeset-bot]

🦋 Changeset detected

Latest commit: 36d8fb8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@openproject/primer-view-components	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Copilot]

Pull request overview

This pull request implements client-side error handling for avatar images that fail to load (404 errors, network failures, etc.). The component now automatically falls back to displaying SVG initials when an image fails to load, avoiding broken image icons.

Key changes:

• All avatars are now wrapped in an <avatar-fallback> custom element with pre-generated fallback SVG
• Client-side error listener detects image load failures and swaps to fallback SVG
• Race condition handling for errors that fire before listener attachment
• Updated tests to reflect new always-wrapped behavior

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
app/components/primer/open_project/avatar_with_fallback.rb	Changed from conditionally wrapping in avatar-fallback to always wrapping; added fallback_src data attribute
app/components/primer/open_project/avatar_fallback.ts	Added error event handling, race condition checking, and image-broken detection logic
test/components/open_project/avatar_with_fallback_test.rb	Updated tests to verify always-wrapped behavior and fallback SVG presence
test/components/open_project/avatar_stack_test.rb	Updated avatar-fallback count expectation from 1 to 2 to reflect new behavior
previews/primer/open_project/avatar_with_fallback_preview.rb	Added two new preview methods for demonstrating 404 error handling
previews/primer/open_project/avatar_with_fallback_preview/multiple_broken_images.html.erb	New template demonstrating multiple broken images
static/previews.json	Auto-generated: Added entries for new preview methods
static/info_arch.json	Auto-generated: Added entries for new preview methods
.changeset/olive-eyes-juggle.md	Added changeset entry for this patch-level change

[akabiru]

UX/UI Meeting Outcome: https://community.openproject.org/meetings/8147#meeting-agenda-item-23298

• The flickering is bad given the Gravatar is enabled per instance and will thus cause heavy flickering on those instances
• We agreed to support static URLs to be loaded beforehand to catch those flickerings. For relative URLs failures, the flickering is ok.

[akabiru]

See:

• Bug/69230 Extend AvatarWithFallback with server-side validated fallback #398

#396 (comment)

Note

Flickering addressed for known hosts (Gravatars)

avatar-fallback-server-side.mp4

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.

[akabiru]

@copilot open a new pull request to apply changes based on the comments in this thread

[Copilot]

@akabiru I've opened a new pull request, #397, to work on those changes. Once the pull request is ready, I'll request review from you.

[akabiru]

@HDinger @myabc I've extracted server side validation to a separate PR- as I think it warrants more discussion and also for the benefit of unblocking this work that is needed for the 17.0 release.