Skip to content

Commit be01160

Browse files
akabiruclaude
andcommitted
Handle 404 errors in AvatarWithFallback with client-side fallback
Server-side URL validation would add latency and can't handle relative URLs without knowing the host. Instead, we use client-side error handling: - Always wrap avatars in <avatar-fallback> element with pre-generated SVG stored in data-fallback-src attribute - Listen for img error events to swap broken images to fallback SVG - Check img.complete && naturalWidth === 0 on connect to catch errors that fired before the listener was attached (race condition fix) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent e7b7cff commit be01160

File tree

7 files changed

+100
-13
lines changed

7 files changed

+100
-13
lines changed

.changeset/olive-eyes-juggle.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@openproject/primer-view-components': patch
3+
---
4+
5+
Handle 404 errors in AvatarWithFallback with client-side fallback

app/components/primer/open_project/avatar_fallback.ts

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,44 @@ import {attr, controller} from '@github/catalyst'
44
export class AvatarFallbackElement extends HTMLElement {
55
@attr uniqueId = ''
66
@attr altText = ''
7+
@attr fallbackSrc = ''
78

89
connectedCallback() {
10+
const img = this.querySelector<HTMLImageElement>('img')
11+
if (!img) return
12+
13+
// Handle image load errors (404, network failure, etc.)
14+
img.addEventListener('error', () => this.handleImageError(img))
15+
16+
// Check if image already failed (error event fired before listener attached)
17+
if (this.isImageBroken(img)) {
18+
this.handleImageError(img)
19+
} else if (this.isFallbackImage(img)) {
20+
this.applyColor(img)
21+
}
22+
}
23+
24+
private isImageBroken(img: HTMLImageElement): boolean {
25+
// Image is broken if loading completed but no actual image data loaded
26+
// Skip check for data URIs (fallback SVGs) as they're always valid
27+
return img.complete && img.naturalWidth === 0 && !img.src.startsWith('data:')
28+
}
29+
30+
private handleImageError(img: HTMLImageElement) {
31+
// Prevent infinite loop if fallback also fails
32+
if (this.isFallbackImage(img)) return
33+
34+
if (this.fallbackSrc) {
35+
img.src = this.fallbackSrc
36+
this.applyColor(img)
37+
}
38+
}
39+
40+
private applyColor(img: HTMLImageElement) {
941
// If either uniqueId or altText is missing, skip color customization so the SVG
1042
// keeps its default gray fill defined in the source and no color override is applied.
1143
if (!this.uniqueId || !this.altText) return
1244

13-
const img = this.querySelector<HTMLImageElement>('img[src^="data:image/svg+xml"]')
14-
if (!img) return
15-
16-
// Generate consistent color based on uniqueId and altText (hash must match OP Core)
1745
const text = `${this.uniqueId}${this.altText}`
1846
const hue = this.valueHash(text)
1947
const color = `hsl(${hue}, 50%, 30%)`
@@ -46,4 +74,8 @@ export class AvatarFallbackElement extends HTMLElement {
4674
// to avoid breaking the component.
4775
}
4876
}
77+
78+
private isFallbackImage(img: HTMLImageElement): boolean {
79+
return img.src === this.fallbackSrc
80+
}
4981
}

app/components/primer/open_project/avatar_with_fallback.rb

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ class AvatarWithFallback < Primer::Beta::Avatar
2121
# - https://github.com/primer/css/blob/main/src/support/variables/typography.scss
2222
FONT_STACK = "-apple-system, BlinkMacSystemFont, 'Segoe UI', 'Noto Sans', Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji'"
2323

24-
# @param src [String] The source url of the avatar image. When nil, renders a fallback with initials.
24+
# @param src [String] The source url of the avatar image. When nil, or broken URL it renders a fallback with initials.
2525
# @param alt [String] Alt text for the avatar. Used for accessibility and to generate initials when src is nil.
2626
# @param size [Integer] <%= one_of(Primer::Beta::Avatar::SIZE_OPTIONS) %>
2727
# @param shape [Symbol] Shape of the avatar. <%= one_of(Primer::Beta::Avatar::SHAPE_OPTIONS) %>
@@ -32,20 +32,20 @@ def initialize(src: nil, alt: nil, size: DEFAULT_SIZE, shape: DEFAULT_SHAPE, hre
3232
require_src_or_alt_arguments(src, alt)
3333

3434
@unique_id = unique_id
35-
@use_fallback = src.blank?
36-
final_src = @use_fallback ? generate_fallback_svg(alt, size) : src
35+
@fallback_svg = generate_fallback_svg(alt, size)
36+
final_src = src.blank? ? @fallback_svg : src
3737

3838
super(src: final_src, alt: alt, size: size, shape: shape, href: href, **system_arguments)
3939
end
4040

4141
def call
4242
render(
43-
Primer::ConditionalWrapper.new(
44-
condition: @use_fallback,
43+
Primer::BaseComponent.new(
4544
tag: :"avatar-fallback",
4645
data: {
4746
unique_id: @unique_id,
48-
alt_text: @system_arguments[:alt]
47+
alt_text: @system_arguments[:alt],
48+
fallback_src: @fallback_svg
4949
}
5050
)
5151
) { super }

previews/primer/open_project/avatar_with_fallback_preview.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,26 @@ def fallback_as_link
6666
end
6767
#
6868
# @!endgroup
69+
70+
# @!group Error Handling (404 Fallback)
71+
#
72+
# @label Broken image (404)
73+
# @snapshot
74+
def broken_image_404
75+
# Uses a non-existent URL - will trigger error handler and show fallback SVG
76+
render(Primer::OpenProject::AvatarWithFallback.new(
77+
src: "/non-existent-avatar.png",
78+
alt: "User With Missing Avatar",
79+
unique_id: 42
80+
))
81+
end
82+
83+
# @label Multiple broken images
84+
def multiple_broken_images
85+
render_with_template(locals: {})
86+
end
87+
#
88+
# @!endgroup
6989
end
7090
end
7191
end
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<div class="d-flex gap-3 flex-items-center">
2+
<p class="color-fg-muted">These avatars have broken image URLs and will fallback to SVG initials:</p>
3+
</div>
4+
<div class="d-flex gap-3 flex-items-center mt-2">
5+
<%= render(Primer::OpenProject::AvatarWithFallback.new(src: "/missing/alice.png", alt: "Alice Johnson", unique_id: 10)) %>
6+
<%= render(Primer::OpenProject::AvatarWithFallback.new(src: "/missing/bob.png", alt: "Bob Smith", unique_id: 20)) %>
7+
<%= render(Primer::OpenProject::AvatarWithFallback.new(src: "/missing/charlie.png", alt: "Charlie Brown", unique_id: 30)) %>
8+
<%= render(Primer::OpenProject::AvatarWithFallback.new(src: "https://example.com/404.png", alt: "Diana Prince", unique_id: 40)) %>
9+
<%= render(Primer::OpenProject::AvatarWithFallback.new(src: "https://invalid-domain-12345.com/avatar.png", alt: "Eve Anderson", unique_id: 50)) %>
10+
</div>

test/components/open_project/avatar_stack_test.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ def test_renders_mixed_avatars
4141
assert_selector(".AvatarStack-body") do
4242
# 2 img tags total: 1 with remote src, 1 with data URI fallback
4343
assert_selector("img.avatar", count: 2)
44-
assert_selector("avatar-fallback", count: 1)
44+
# All avatars wrapped in avatar-fallback for 404 error handling
45+
assert_selector("avatar-fallback", count: 2)
4546
assert_selector("img.avatar[src^='data:image/svg+xml;base64,']", count: 1)
4647
end
4748
end

test/components/open_project/avatar_with_fallback_test.rb

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,27 @@ class PrimerOpenProjectAvatarWithFallbackTest < Minitest::Test
88
def test_renders_image_avatar_with_src
99
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: "https://github.com/github.png", alt: "github"))
1010

11-
assert_selector("img.avatar")
12-
refute_selector("avatar-fallback")
11+
# Always wrapped in avatar-fallback for 404 error handling
12+
assert_selector("avatar-fallback[data-fallback-src^='data:image/svg+xml;base64,']") do
13+
assert_selector("img.avatar[src='https://github.com/github.png']")
14+
end
15+
end
16+
17+
def test_image_avatar_error_handling_setup
18+
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: "https://example.com/avatar.png", alt: "Alice Johnson", unique_id: 123))
19+
20+
# Original src is preserved (client-side JS handles error -> fallback swap)
21+
assert_selector("avatar-fallback[data-unique-id='123'][data-alt-text='Alice Johnson']") do
22+
assert_selector("img.avatar[src='https://example.com/avatar.png']")
23+
end
24+
25+
# Verify fallback SVG is available and contains correct initials
26+
fallback_wrapper = page.find("avatar-fallback")
27+
fallback_src = fallback_wrapper["data-fallback-src"]
28+
29+
assert fallback_src.start_with?("data:image/svg+xml;base64,")
30+
svg_content = Base64.decode64(fallback_src.sub("data:image/svg+xml;base64,", ""))
31+
assert_includes svg_content, ">AJ<", "Fallback SVG should contain initials 'AJ'"
1332
end
1433

1534
def test_renders_fallback_when_src_is_nil

0 commit comments

Comments
 (0)