Skip to content

Show user profile avatars in viewer and share lists - #2464

Merged
shomix merged 5 commits into
mainfrom
ai_main_638a9da5a8bf4f69ba6f
Jul 29, 2026
Merged

Show user profile avatars in viewer and share lists#2464
shomix merged 5 commits into
mainfrom
ai_main_638a9da5a8bf4f69ba6f

Conversation

@shomix

@shomix shomix commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes missing user avatars across the clips template by wiring up the useAvatarUrl hook 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 useAvatarUrl hook and render it with AvatarImage when available, falling back to the existing initials/icon placeholder when no avatar URL is present. Extracted a shared ViewerAvatar component in the "viewed by" popover to avoid duplicating avatar-rendering logic.

Key Changes

  • recording-views-badge.tsx: ViewerAvatar now calls useAvatarUrl and renders AvatarImage when a URL is returned, otherwise falls back to the existing icon/initials.
  • share-ui.tsx: Avatar component 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 new ViewerAvatar component (using useAvatarUrl) to render viewer avatars, replacing the inline Avatar/AvatarFallback markup and reducing duplication.
  • Added tests covering avatar resolution for ViewerAvatar in the recording badge, the share dialog Avatar, and the viewed-by-popover ViewerAvatar, including the organization fallback case.
  • Added a changelog entry documenting the fix.

Edit in Builder  Preview


To clone this PR locally use the Github CLI with command gh pr checkout 2464

You can tag me at @BuilderIO for anything you want me to fix or change

@netlify

This comment has been minimized.

@builder-io-integration builder-io-integration Bot changed the title Update from the Builder.io agent Show user profile avatars in viewer and share lists Jul 28, 2026
@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

builder-io-integration[bot]

This comment was marked as outdated.

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Here's a visual recap of what changed:

Visual recap

Open the full interactive recap

@builder-io-integration builder-io-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

  • 🟡 MEDIUMuseVisibleAvatarUrl retains isVisible when 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 a HTMLDivElement, 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.

Comment thread templates/clips/app/lib/use-visible-avatar-url.ts
import { useEffect, useRef, useState } from "react";

export function useVisibleAvatarUrl(email: string | null | undefined) {
const avatarRef = useRef<HTMLSpanElement>(null);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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.

Fix in Builder

@builder-io-integration builder-io-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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.

Fix in Builder

@shomix
shomix merged commit cc0e17c into main Jul 29, 2026
99 checks passed
@shomix
shomix deleted the ai_main_638a9da5a8bf4f69ba6f branch July 29, 2026 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants