Implement flicker-free "fallback first" avatar loading pattern

Render fallback SVG as initial <img> src (visible immediately), test load the real URL
and replace if valid.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Author: akabiru

.changeset/big-seals-clap.md

@@ -0,0 +1,5 @@
+---
+'@openproject/primer-view-components': patch
+---
+
+Fix flickering in AvatarWithFallback client-side fallback handling for broken avatar urls

app/components/primer/open_project/avatar_fallback.ts

@@ -1,61 +1,68 @@
-import {attr, controller} from '@github/catalyst'
-
+import {controller} from '@github/catalyst'
+
+/**
+ * AvatarFallbackElement implements "fallback first" loading pattern:
+ * 1. Fallback SVG is rendered immediately as <img> src
+ * 2. Real avatar URL is test-loaded in background using new Image()
+ * 3. On success, swaps to real image; on failure, fallback stays visible
+ *
+ * This approach prevents flicker by never showing a broken image state.
+ * Inspired by OpenProject's Angular PrincipalRendererService.
+ *
+ * Note: We read attributes directly via getAttribute() instead of using @attr
+ * due to a Catalyst bug where @attr accessors aren't properly initialized
+ * when elements have pre-existing attribute values.
+ */
 @controller
 export class AvatarFallbackElement extends HTMLElement {
-  @attr uniqueId = ''
-  @attr altText = ''
-  @attr fallbackSrc = ''
-
   private img: HTMLImageElement | null = null
-  private boundErrorHandler?: () => void
 
   connectedCallback() {
     this.img = this.querySelector<HTMLImageElement>('img') ?? null
     if (!this.img) return
 
-    this.boundErrorHandler = () => this.handleImageError(this.img!)
+    const uniqueId = this.getAttribute('data-unique-id') || ''
+    const altText = this.getAttribute('data-alt-text') || ''
+    const avatarSrc = this.getAttribute('data-avatar-src') || ''
 
-    // Handle image load errors (404, network failure, etc.)
-    this.img.addEventListener('error', this.boundErrorHandler)
+    // Apply hashed color to fallback SVG immediately
+    this.applyColor(this.img, uniqueId, altText)
 
-    // Check if image already failed (error event fired before listener attached)
-    if (this.isImageBroken(this.img)) {
-      this.handleImageError(this.img)
-    } else if (this.isFallbackImage(this.img)) {
-      this.applyColor(this.img)
+    // Test-load real avatar URL in background
+    if (avatarSrc) {
+      this.testLoadImage(avatarSrc)
     }
   }
 
   disconnectedCallback() {
-    if (this.boundErrorHandler && this.img) {
-      this.img.removeEventListener('error', this.boundErrorHandler)
-    }
-    this.boundErrorHandler = undefined
     this.img = null
   }
 
-  private isImageBroken(img: HTMLImageElement): boolean {
-    // Image is broken if loading completed but no actual image data loaded
-    // Skip check for data URIs (fallback SVGs) as they're always valid
-    return img.complete && img.naturalWidth === 0 && !img.src.startsWith('data:')
-  }
-
-  private handleImageError(img: HTMLImageElement) {
-    // Prevent infinite loop if fallback also fails
-    if (this.isFallbackImage(img)) return
-
-    if (this.fallbackSrc) {
-      img.src = this.fallbackSrc
-      this.applyColor(img)
+  /**
+   * Test-loads the real avatar URL in background.
+   * On success, swaps the visible img to the real URL.
+   * On failure, does nothing - fallback stays visible.
+   */
+  private testLoadImage(url: string) {
+    const testImage = new Image()
+
+    testImage.onload = () => {
+      // Success - swap to real image
+      if (this.img) {
+        this.img.src = url
+      }
     }
+
+    // On error: do nothing, fallback stays visible (no flicker)
+    testImage.src = url
   }
 
-  private applyColor(img: HTMLImageElement) {
+  private applyColor(img: HTMLImageElement, uniqueId: string, altText: string) {
     // If either uniqueId or altText is missing, skip color customization so the SVG
     // keeps its default gray fill defined in the source and no color override is applied.
-    if (!this.uniqueId || !this.altText) return
+    if (!uniqueId || !altText) return
 
-    const text = `${this.uniqueId}${this.altText}`
+    const text = `${uniqueId}${altText}`
     const hue = this.valueHash(text)
     const color = `hsl(${hue}, 50%, 30%)`
 
@@ -76,6 +83,8 @@ export class AvatarFallbackElement extends HTMLElement {
 
   private updateSvgColor(img: HTMLImageElement, color: string) {
     const dataUri = img.src
+    if (!dataUri.startsWith('data:image/svg+xml;base64,')) return
+
     const base64 = dataUri.replace('data:image/svg+xml;base64,', '')
 
     try {
@@ -87,8 +96,4 @@ export class AvatarFallbackElement extends HTMLElement {
       // to avoid breaking the component.
     }
   }
-
-  private isFallbackImage(img: HTMLImageElement): boolean {
-    return img.src === this.fallbackSrc
-  }
 }

app/components/primer/open_project/avatar_with_fallback.rb

@@ -5,14 +5,12 @@ module OpenProject
     # OpenProject-specific Avatar component that extends Primer::Beta::Avatar
     # to support fallback rendering with initials when no image source is provided.
     #
-    # When `src` is nil, this component renders an SVG with initials extracted from
-    # the alt text. The AvatarFallbackElement web component then enhances it client-side
-    # by applying a consistent background color based on the user's unique_id (using the
-    # same hash function as OP Core for consistency).
+    # Uses a "fallback first" pattern for flicker-free loading:
+    # 1. Always renders fallback SVG as initial <img> src (visible immediately)
+    # 2. Client-side JS test-loads the real URL in background
+    # 3. On success, swaps to real image; on failure, fallback stays visible
     #
-    # This component follows the "extension over mutation" pattern - it extends
-    # Primer::Beta::Avatar without modifying its interface, ensuring compatibility
-    # with upstream changes.
+    # This approach is inspired by OpenProject's Angular PrincipalRendererService.
     class AvatarWithFallback < Primer::Beta::Avatar
       status :open_project
 
@@ -21,8 +19,8 @@ class AvatarWithFallback < Primer::Beta::Avatar
       #   - https://github.com/primer/css/blob/main/src/support/variables/typography.scss
       FONT_STACK = "-apple-system, BlinkMacSystemFont, 'Segoe UI', 'Noto Sans', Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji'"
 
-      # @param src [String] The source url of the avatar image. When nil or a broken URL, it renders a fallback with initials.
-      # @param alt [String] Alt text for the avatar. Used for accessibility and to generate initials when src is nil.
+      # @param src [String] The source url of the avatar image. JS will test-load and swap on success.
+      # @param alt [String] Alt text for the avatar. Used for accessibility and to generate initials.
       # @param size [Integer] <%= one_of(Primer::Beta::Avatar::SIZE_OPTIONS) %>
       # @param shape [Symbol] Shape of the avatar. <%= one_of(Primer::Beta::Avatar::SHAPE_OPTIONS) %>
       # @param href [String] The URL to link to. If used, component will be wrapped by an `<a>` tag.
@@ -32,10 +30,11 @@ def initialize(src: nil, alt: nil, size: DEFAULT_SIZE, shape: DEFAULT_SHAPE, hre
         require_src_or_alt_arguments(src, alt)
 
         @unique_id = unique_id
+        @avatar_src = src.presence
         @fallback_svg = generate_fallback_svg(alt, size)
-        final_src = src.blank? ? @fallback_svg : src
 
-        super(src: final_src, alt: alt, size: size, shape: shape, href: href, **system_arguments)
+        # Always render fallback first - JS will swap to real image on successful load
+        super(src: @fallback_svg, alt: alt, size: size, shape: shape, href: href, **system_arguments)
       end
 
       def call
@@ -45,7 +44,7 @@ def call
             data: {
               unique_id: @unique_id,
               alt_text: @system_arguments[:alt],
-              fallback_src: @fallback_svg
+              avatar_src: @avatar_src
             }
           )
         ) { super }

static/classes.json

@@ -597,7 +597,8 @@
     "Primer::OpenProject::SidePanel::Section"
   ],
   "SkeletonBox": [
-    "Primer::Alpha::SkeletonBox"
+    "Primer::Alpha::SkeletonBox",
+    "Primer::OpenProject::SkeletonBox"
   ],
   "Stack": [
     "Primer::Alpha::Stack"
@@ -738,16 +739,20 @@
     "Primer::Alpha::ToggleSwitch"
   ],
   "TreeItemSkeletonTextStyles": [
-    "Primer::Alpha::TreeView"
+    "Primer::Alpha::TreeView",
+    "Primer::OpenProject::TreeView"
   ],
   "TreeViewFailureMessage": [
-    "Primer::Alpha::TreeView"
+    "Primer::Alpha::TreeView",
+    "Primer::OpenProject::TreeView"
   ],
   "TreeViewRootUlStyles": [
-    "Primer::Alpha::TreeView"
+    "Primer::Alpha::TreeView",
+    "Primer::OpenProject::TreeView"
   ],
   "TreeViewSkeletonItemContainerStyle": [
-    "Primer::Alpha::TreeView"
+    "Primer::Alpha::TreeView",
+    "Primer::OpenProject::TreeView"
   ],
   "Truncate": [
     "Primer::Beta::Truncate"

test/components/open_project/avatar_stack_test.rb

@@ -12,7 +12,10 @@ def test_renders_with_image_avatars
 
     assert_selector("div.AvatarStack") do
       assert_selector(".AvatarStack-body") do
-        assert_selector("img.avatar", count: 1)
+        # Fallback-first pattern: img shows fallback SVG, real URL in data-avatar-src for JS
+        assert_selector("avatar-fallback[data-avatar-src='https://github.com/github.png']") do
+          assert_selector("img.avatar[src^='data:image/svg+xml;base64,']", count: 1)
+        end
       end
     end
   end
@@ -39,11 +42,14 @@ def test_renders_mixed_avatars
 
     assert_selector("div.AvatarStack") do
       assert_selector(".AvatarStack-body") do
-        # 2 img tags total: 1 with remote src, 1 with data URI fallback
+        # 2 img tags total, both with data URI fallback initially (fallback-first pattern)
         assert_selector("img.avatar", count: 2)
         # All avatars wrapped in avatar-fallback for 404 error handling
         assert_selector("avatar-fallback", count: 2)
-        assert_selector("img.avatar[src^='data:image/svg+xml;base64,']", count: 1)
+        # Both start with fallback SVG; JS swaps to real URL on successful load
+        assert_selector("img.avatar[src^='data:image/svg+xml;base64,']", count: 2)
+        # One has a real URL for JS to test-load
+        assert_selector("avatar-fallback[data-avatar-src='https://github.com/github.png']", count: 1)
       end
     end
   end

test/components/open_project/avatar_with_fallback_test.rb

@@ -5,45 +5,48 @@
 class PrimerOpenProjectAvatarWithFallbackTest < Minitest::Test
   include Primer::ComponentTestHelpers
 
-  def test_renders_image_avatar_with_src
+  # "Fallback First" pattern tests:
+  # - img.src is ALWAYS the fallback SVG initially
+  # - data-avatar-src contains the real URL for JS to test-load
+  # - JS swaps to real image only on successful load
+
+  def test_renders_fallback_first_with_real_url_in_data_attribute
     render_inline(Primer::OpenProject::AvatarWithFallback.new(src: "https://github.com/github.png", alt: "github"))
 
-    # Always wrapped in avatar-fallback for 404 error handling
-    assert_selector("avatar-fallback[data-fallback-src^='data:image/svg+xml;base64,']") do
-      assert_selector("img.avatar[src='https://github.com/github.png']")
+    # Image always shows fallback SVG first
+    assert_selector("avatar-fallback[data-avatar-src='https://github.com/github.png']") do
+      assert_selector("img.avatar[src^='data:image/svg+xml;base64,']")
     end
   end
 
-  def test_image_avatar_error_handling_setup
+  def test_preserves_unique_id_and_alt_text_for_client_side_processing
     render_inline(Primer::OpenProject::AvatarWithFallback.new(src: "https://example.com/avatar.png", alt: "Alice Johnson", unique_id: 123))
 
-    # Original src is preserved (client-side JS handles error -> fallback swap)
-    assert_selector("avatar-fallback[data-unique-id='123'][data-alt-text='Alice Johnson']") do
-      assert_selector("img.avatar[src='https://example.com/avatar.png']")
+    assert_selector("avatar-fallback[data-unique-id='123'][data-alt-text='Alice Johnson'][data-avatar-src='https://example.com/avatar.png']") do
+      assert_selector("img.avatar[src^='data:image/svg+xml;base64,']")
     end
 
-    # Verify fallback SVG is available and contains correct initials
+    # Verify fallback SVG contains correct initials
     fallback_wrapper = page.find("avatar-fallback")
-    fallback_src = fallback_wrapper["data-fallback-src"]
-
-    assert fallback_src.start_with?("data:image/svg+xml;base64,")
-    svg_content = Base64.decode64(fallback_src.sub("data:image/svg+xml;base64,", ""))
+    img = fallback_wrapper.find("img")
+    svg_content = Base64.decode64(img["src"].sub("data:image/svg+xml;base64,", ""))
     assert_includes svg_content, ">AJ<", "Fallback SVG should contain initials 'AJ'"
   end
 
   def test_renders_fallback_when_src_is_nil
     render_inline(Primer::OpenProject::AvatarWithFallback.new(src: nil, alt: "OpenProject Admin"))
 
-    # Should render avatar-fallback element wrapping an img with base64 SVG data URI
-    assert_selector("avatar-fallback[data-alt-text='OpenProject Admin']") do
+    # No data-avatar-src when src is nil
+    assert_selector("avatar-fallback[data-alt-text='OpenProject Admin']:not([data-avatar-src])") do
       assert_selector("img.avatar[src^='data:image/svg+xml;base64,']")
     end
   end
 
   def test_renders_fallback_when_src_is_blank
     render_inline(Primer::OpenProject::AvatarWithFallback.new(src: "", alt: "OpenProject Admin"))
 
-    assert_selector("avatar-fallback[data-alt-text='OpenProject Admin']") do
+    # No data-avatar-src when src is blank
+    assert_selector("avatar-fallback[data-alt-text='OpenProject Admin']:not([data-avatar-src])") do
       assert_selector("img.avatar[src^='data:image/svg+xml;base64,']")
     end
   end
@@ -101,7 +104,6 @@ def test_sets_size_height_and_width
   def test_fallback_sets_correct_size_class
     render_inline(Primer::OpenProject::AvatarWithFallback.new(src: nil, alt: "Test User", size: 40))
 
-    # Size is set via attributes, not a dedicated class
     assert_selector("img.avatar[size='40'][height='40'][width='40'][src^='data:image/svg+xml;base64,']")
   end
 
@@ -132,7 +134,6 @@ def test_renders_link_wrapper
   def test_fallback_renders_link_wrapper
     render_inline(Primer::OpenProject::AvatarWithFallback.new(src: nil, alt: "Test User", href: "#test"))
 
-    # When href is provided, the avatar class is on the <a> tag, not the <img>
     assert_selector("avatar-fallback") do
       assert_selector("a.avatar[href='#test']") do
         assert_selector("img[src^='data:image/svg+xml;base64,']")
@@ -143,7 +144,6 @@ def test_fallback_renders_link_wrapper
   def test_fallback_with_unique_id_in_data_attribute
     render_inline(Primer::OpenProject::AvatarWithFallback.new(src: nil, alt: "Test User", unique_id: 123))
 
-    # Should have data attributes for client-side processing
     assert_selector("avatar-fallback[data-unique-id='123'][data-alt-text='Test User']") do
       assert_selector("img.avatar[src^='data:image/svg+xml;base64,']")
     end
@@ -189,9 +189,10 @@ def test_raises_when_both_src_and_alt_are_blank
   def test_fallback_with_single_word_name
     render_inline(Primer::OpenProject::AvatarWithFallback.new(src: nil, alt: "Alice"))
 
-    assert_selector("avatar-fallback") do
-      assert_selector("img.avatar[src^='data:image/svg+xml;base64,']")
-    end
+    img = page.find("img.avatar")
+    svg_content = Base64.decode64(img["src"].sub("data:image/svg+xml;base64,", ""))
+
+    assert_includes svg_content, ">A<", "Single word name should produce single initial 'A'"
   end
 
   def test_status