Show user profile avatars in viewer and share lists - #2464
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Here's a visual recap of what changed: Open the full interactive recap |
There was a problem hiding this comment.
Builder reviewed your changes and found 2 potential issues 🟡
Review Details
Incremental Code Review Summary
The latest commit addresses both prior performance findings by introducing useVisibleAvatarUrl, which gates avatar resolution behind IntersectionObserver and is now used by the share list and viewed-by popover. The existing recording views badge remains unchanged and its bounded viewer query is unaffected. I verified that the two earlier comments are fixed and resolved both review threads.
Risk assessment: Standard — this is UI behavior plus a new shared hook with lifecycle and DOM-ref implications.
Key Findings
- 🟡 MEDIUM —
useVisibleAvatarUrlretainsisVisiblewhen its email changes, so a reused component instance can immediately fetch a newly assigned offscreen email and bypass the visibility gate. - 🟡 MEDIUM — The visibility ref is typed as
HTMLSpanElement, but the shadcn/Radix Avatar root is aHTMLDivElement, creating a ref type mismatch under strict TypeScript checking.
The deferred-loading strategy otherwise fixes the previously reported unbounded request path, and the added tests cover the initial hidden-to-visible transition and organization fallback.
🧪 Browser testing: Ran after this incremental review, but all planned flows were couldnt_verify because browser automation tooling was unavailable; the dev server was healthy.
| import { useEffect, useRef, useState } from "react"; | ||
|
|
||
| export function useVisibleAvatarUrl(email: string | null | undefined) { | ||
| const avatarRef = useRef<HTMLSpanElement>(null); |
There was a problem hiding this comment.
🟡 Use the Avatar root element type for the visibility ref
avatarRef is typed as HTMLSpanElement, but it is passed to the shadcn/Radix Avatar root, whose forwarded ref targets an HTMLDivElement. This produces a strict TypeScript ref incompatibility; type the ref to the actual Avatar root element (or a compatible Element type).
Additional Info
Found by 1 of 2 parallel incremental review agents; confirmed from the shared Avatar component's Radix Root ref typing.
There was a problem hiding this comment.
Builder reviewed your changes and found 1 potential issue 🟡
Review Details
Incremental Code Review Summary
The latest commit fixes the previous visibility-state issue by replacing the boolean with visibleEmail, so a reused row no longer immediately authorizes the new email for fetching. The existing ref-type issue at use-visible-avatar-url.ts:5 remains open and is not reposted.
However, the new gating still leaves a stale-image bug at the boundary: the shared useAvatarUrl hook retains its previous URL when called with null, so changing a mounted avatar from one email to another can continue displaying the previous user's image while the new email is waiting for intersection. The new regression test mocks this behavior away by returning null for null input.
Risk assessment: Standard — this is user-facing avatar state and shared hook lifecycle behavior.
Key Finding
- 🟡 MEDIUM — Clear or isolate the previous avatar URL when the visible email changes before rendering the new row's image.
🧪 Browser testing: Attempted after this review, but live verification was blocked by unrelated Clips environment failures (list-recordings stack overflow and unavailable recording upload); no recording route was reachable.
|
|
||
| return { | ||
| avatarRef, | ||
| avatarUrl: useAvatarUrl(visibleEmail === email ? email : null), |
There was a problem hiding this comment.
🟡 Clear the previous avatar when deferring a new email
When a mounted avatar changes from one email to another, this passes null to useAvatarUrl until the new element intersects. The shared useAvatarUrl hook retains its prior URL when its email becomes null, so the previous user's image can remain rendered during the deferred period (and while the new fetch is pending). Clear the URL at this boundary or update the hook to reset state when the email changes; the current mock-based reuse test masks this stateful behavior.
Additional Info
Found by 1 of 2 parallel incremental review agents; confirmed against the stateful useAvatarUrl implementation. Browser verification was blocked by unrelated Clips data/upload failures.

Summary
Fixes missing user avatars across the clips template by wiring up the
useAvatarUrlhook in the recording viewer badge, share dialog, and "viewed by" popover components.Problem
Several places in the clips template were meant to display the actual user's avatar (viewer badges, share access lists, and "viewed by" popovers), but they only ever rendered generic fallback icons/initials instead of the user's real profile image.
Solution
Fetch each user's avatar via the existing
useAvatarUrlhook and render it withAvatarImagewhen available, falling back to the existing initials/icon placeholder when no avatar URL is present. Extracted a sharedViewerAvatarcomponent in the "viewed by" popover to avoid duplicating avatar-rendering logic.Key Changes
recording-views-badge.tsx:ViewerAvatarnow callsuseAvatarUrland rendersAvatarImagewhen a URL is returned, otherwise falls back to the existing icon/initials.share-ui.tsx:Avatarcomponent now resolves and displays a user's avatar image for individual shares, while organization shares keep the group icon fallback.viewed-by-popover.tsx: extracted a newViewerAvatarcomponent (usinguseAvatarUrl) to render viewer avatars, replacing the inlineAvatar/AvatarFallbackmarkup and reducing duplication.ViewerAvatarin the recording badge, the share dialogAvatar, and theviewed-by-popoverViewerAvatar, including the organization fallback case.To clone this PR locally use the Github CLI with command
gh pr checkout 2464You can tag me at @BuilderIO for anything you want me to fix or change