Skip to content

fix: prevent user avatar flickering on scroll in Rill Cloud user list#9168

Open
royendo wants to merge 5 commits intomainfrom
worktree-fix-flickering-rill-cloud-user-icons
Open

fix: prevent user avatar flickering on scroll in Rill Cloud user list#9168
royendo wants to merge 5 commits intomainfrom
worktree-fix-flickering-rill-cloud-user-icons

Conversation

@royendo
Copy link
Copy Markdown
Contributor

@royendo royendo commented Apr 2, 2026

Replace bits-ui Avatar.Image/Avatar.Fallback with a native <img> element in the Avatar component.

bits-ui's Avatar.Image creates a new Image() on every mount and forces a loadingloaded state transition, setting display: none on the <img> until onload fires. Even for cached images, this is async — so there's at least one frame showing the fallback initials. When infinite scroll loads a new page, the table re-renders, Avatar components remount, and every visible avatar briefly flashes its fallback.

A native <img> renders immediately from browser cache with no loading state machine, eliminating the flicker.

loom: https://www.loom.com/share/70190439af2e455797839a33a28e589a?from_recorder=1&focus_title=1

Fixes: https://linear.app/rilldata/issue/APP-526/user-management-table-header-row-moves-and-avatar-flickers-when

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

Developed in collaboration with Claude Code

Replace bits-ui `Avatar.Image`/`Avatar.Fallback` with a native `<img>` element.
bits-ui creates a new `Image()` on every mount and forces a loading→loaded
state transition, briefly showing the fallback even for cached images. A native
`<img>` renders immediately from browser cache, eliminating the flicker when
infinite scroll triggers a table re-render.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@royendo
Copy link
Copy Markdown
Contributor Author

royendo commented Apr 2, 2026

options:
Option 1: Initialize loadingStatus to "loaded" when src is provided

The bits-ui loadImage function has this early return:
if (this.opts.loadingStatus.current === "loaded") return;

So if we pass the initial status as "loaded", it skips the whole loading cycle. But the downside is we'd lose the error fallback — if an image URL is broken, it shows a broken image instead of initials.

Option 2: Check browser cache synchronously before mounting

<script> function isImageCached(src: string) { const img = new Image(); img.src = src; return img.complete; } $: initialStatus = src && isImageCached(src) ? "loaded" : undefined; </script>

<Avatar.Root loadingStatus={initialStatus} ...>

This preserves the loading/fallback behavior for uncached images while skipping the flash for cached ones. But it creates a hidden Image() object on every render just to check the cache.

Option 3 (what I did): Native

Simplest, no extra objects, browser handles caching natively, fallback via on:error. The only thing lost is the bits-ui delay animation on first load, which isn't used here anyway (delayMs defaults to 0).

I'd stick with the native approach — it's the least code, most reliable, and removes an unnecessary abstraction. But if you'd prefer to keep bits-ui in the loop, Option 2 works too.

@royendo royendo requested a review from ericpgreen2 April 2, 2026 15:15
Copy link
Copy Markdown
Contributor

@ericpgreen2 ericpgreen2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still seeing a flicker (Jam). Are you not?

@royendo
Copy link
Copy Markdown
Contributor Author

royendo commented Apr 9, 2026

Ah, indeed my test was only for img included ones;

A Q:

  tanstack-virtual solves a problem these tables don't have.                                                                                                     
                                                                                                                                                                 
  The data is already paginated at 50 rows per page. Even if a user scrolls through 10 pages, that's 500 rows — trivial for the browser. Virtualization shines   
  when you have 10k+ rows loaded in memory and need to avoid DOM bloat. That's not this case.    

                                                                                                                                                               
  Pros of tanstack-virtual here:                            
  - Limits DOM nodes if someone scrolls through many pages (marginal benefit at 50 rows/page)
  - That's about it                                                                                                                                              
  
  Cons of tanstack-virtual here:                                                                                                                                 
  - Causes the exact flickering bug you're fixing — components get destroyed/recreated on scroll
  - Adds complexity: transform: translateY(...) positioning, measureElement layout thrashing, overscan tuning                                                    
  - Makes rows harder to style (absolute positioning via transforms instead of natural table flow)           
  - Extra dependency for something the browser handles natively                                                                                                  
  - The infinite scroll trigger (load more when last item is visible) could be done with a simple IntersectionObserver instead                                   
                                                                                                                                                                 
  Bottom line: For these admin tables with paginated data in the low hundreds, virtualization is unnecessary overhead that's actively causing bugs. A plain      
  <table> with an IntersectionObserver for load-more would be simpler and flicker-free.                                                                          
                                                                                                                                                                 
  That said, InfiniteScrollTable is shared across 3 consumers. If you want to keep it general-purpose, the overscan={Infinity} approach is a safe per-table      
  opt-out. If you want to simplify, you could remove tanstack-virtual from this component entirely and use a sentinel element + IntersectionObserver for the
  load-more trigger.                                                                                                                                             
                                                            
  What direction do you want to go?                                                                  

@ericpgreen2
Copy link
Copy Markdown
Contributor

What direction do you want to go?

I'm good with removing tanstack-virtual from this table and the other instances of InfiniteScrollTable, since the other instances are also <1k rows in practice. Let's just be clear with our naming and documentation of the component that it should not be used at a scale that would warrant virtualization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants