Skip to content

Fix navbar avatar fallback#140

Merged
stylessh merged 4 commits intostylessh:mainfrom
premsathisha:prem/fix-navbar-avatar
Apr 17, 2026
Merged

Fix navbar avatar fallback#140
stylessh merged 4 commits intostylessh:mainfrom
premsathisha:prem/fix-navbar-avatar

Conversation

@premsathisha
Copy link
Copy Markdown
Contributor

@premsathisha premsathisha commented Apr 16, 2026

Summary

This PR fixes the navbar profile avatar getting stuck on the fallback placeholder:

  • Uses the shared avatar primitive: Replaces the raw navbar <img> handling with AvatarImage and AvatarFallback in both desktop and mobile navigation.
  • Removes sticky failure state: Drops the local avatarLoadFailed flag that could force the navbar avatar to stay on initials after a single image load failure.
  • Keeps desktop and mobile aligned: Applies the same avatar rendering path in both navbar variants.

Why

  • Before: The desktop and mobile navbars each managed avatar loading with a raw <img> plus a local failure flag.
  • Before: If the avatar image failed once, the navbar could remain on the fallback placeholder for the rest of that render path.
  • After: Avatar loading and fallback behavior are delegated to the shared avatar component, so the navbar no longer owns a sticky failed state.

Validation

  • pnpm --filter @diffkit/dashboard check
  • pnpm --filter @diffkit/dashboard check-types
  • pnpm --filter @diffkit/dashboard build

The repo still has the existing src/lib/github.functions.ts non-null assertion warning during check; this PR does not change that file.

Summary by CodeRabbit

  • Refactor
    • Unified avatar rendering across dashboard topbar and mobile nav for more consistent behavior.
    • Replaced ad-hoc image error handling with a single avatar component that always provides a fallback, improving reliability.
    • Simplified component logic by removing local image-failure state, reducing UI flicker and ensuring predictable fallback timing.

Copy link
Copy Markdown
Owner

on each refresh the letters fallback always shows for a bit, it kind of bugs me. any fix on that?

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 16, 2026

📝 Walkthrough

Walkthrough

Two layout components had avatar rendering refactored to always use AvatarImage with AvatarFallback, replacing conditional raw <img> + local image-failure state. AvatarImage now receives src={user.image ?? undefined} and AvatarFallback.delayMs is 400 when an image exists, otherwise 0.

Changes

Cohort / File(s) Summary
Avatar rendering updates
apps/dashboard/src/components/layouts/dashboard-mobile-nav.tsx, apps/dashboard/src/components/layouts/dashboard-topbar.tsx
Replaced conditional <img> + useState failure-tracking and onError handlers with AvatarImage + AvatarFallback. AvatarImage uses src={user.image ?? undefined}; AvatarFallback.delayMs is 400 when user.image exists, 0 otherwise. Removed useState import/state from topbar.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change—fixing avatar fallback behavior in the navbar by replacing raw img elements with the shared Avatar component.
Description check ✅ Passed The description covers the summary, changes, and validation steps. However, it lacks the Test Plan and Screenshots sections specified in the repository template.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
apps/dashboard/src/components/layouts/dashboard-topbar.tsx (1)

237-252: Consider extracting a shared UserAvatar snippet to prevent drift.

The same avatar JSX appears twice; a small local component/helper would keep styling/behavior changes synchronized.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/dashboard/src/components/layouts/dashboard-topbar.tsx` around lines 237
- 252, The avatar JSX is duplicated in DashboardTopbar (used inside
DropdownMenuTrigger and DropdownMenuContent); extract a small reusable component
(e.g., UserAvatar) that wraps Avatar, AvatarImage and AvatarFallback and accepts
props for user, displayName, initials and size/className so both instances reuse
it; replace the two inline Avatar blocks with <UserAvatar user={user}
displayName={displayName} initials={initials} size="size-7" /> and size="size-8"
respectively, preserving the existing props (src fallback, alt, delayMs, border
classes) so styling and behavior remain identical and future changes stay
synchronized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/dashboard/src/components/layouts/dashboard-mobile-nav.tsx`:
- Around line 135-138: The AvatarFallback currently uses a fixed delayMs={400}
causing initials to wait even when no image exists; update the JSX around
AvatarImage and AvatarFallback so delayMs is conditional based on user.image
(e.g., pass delayMs={user.image ? 400 : 0} or omit the prop when user.image is
falsy) so that AvatarFallback renders immediately when user.image is missing;
check the AvatarImage, AvatarFallback, user.image, initials and displayName
usage in dashboard-mobile-nav.tsx to apply this change.

---

Nitpick comments:
In `@apps/dashboard/src/components/layouts/dashboard-topbar.tsx`:
- Around line 237-252: The avatar JSX is duplicated in DashboardTopbar (used
inside DropdownMenuTrigger and DropdownMenuContent); extract a small reusable
component (e.g., UserAvatar) that wraps Avatar, AvatarImage and AvatarFallback
and accepts props for user, displayName, initials and size/className so both
instances reuse it; replace the two inline Avatar blocks with <UserAvatar
user={user} displayName={displayName} initials={initials} size="size-7" /> and
size="size-8" respectively, preserving the existing props (src fallback, alt,
delayMs, border classes) so styling and behavior remain identical and future
changes stay synchronized.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3338683a-baf3-4d01-8f29-b7407d5e67df

📥 Commits

Reviewing files that changed from the base of the PR and between bee56dd and ebff78a.

📒 Files selected for processing (2)
  • apps/dashboard/src/components/layouts/dashboard-mobile-nav.tsx
  • apps/dashboard/src/components/layouts/dashboard-topbar.tsx

Comment thread apps/dashboard/src/components/layouts/dashboard-mobile-nav.tsx
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
apps/dashboard/src/components/layouts/dashboard-topbar.tsx (1)

237-257: Optional: extract repeated avatar JSX into a small helper/component.

The two avatar blocks are now identical; extracting a local UserAvatar renderer would reduce duplication and prevent future drift.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/dashboard/src/components/layouts/dashboard-topbar.tsx` around lines 237
- 257, The two identical Avatar JSX blocks should be extracted to a small
reusable component to remove duplication: create a local UserAvatar component
(e.g., function UserAvatar({user, displayName, initials}) returning the
Avatar/AvatarImage/AvatarFallback structure using user.image, displayName and
initials, preserving delayMs logic) and replace both inline Avatar usages with
<UserAvatar user={user} displayName={displayName} initials={initials} />; ensure
props and className values ("size-7"/"size-8" differences if needed) are exposed
so both places can pass the appropriate size/class.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@apps/dashboard/src/components/layouts/dashboard-topbar.tsx`:
- Around line 237-257: The two identical Avatar JSX blocks should be extracted
to a small reusable component to remove duplication: create a local UserAvatar
component (e.g., function UserAvatar({user, displayName, initials}) returning
the Avatar/AvatarImage/AvatarFallback structure using user.image, displayName
and initials, preserving delayMs logic) and replace both inline Avatar usages
with <UserAvatar user={user} displayName={displayName} initials={initials} />;
ensure props and className values ("size-7"/"size-8" differences if needed) are
exposed so both places can pass the appropriate size/class.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 538c6c70-4401-4e4d-88f7-09d3f9367d7a

📥 Commits

Reviewing files that changed from the base of the PR and between ebff78a and 4d4dfa1.

📒 Files selected for processing (2)
  • apps/dashboard/src/components/layouts/dashboard-mobile-nav.tsx
  • apps/dashboard/src/components/layouts/dashboard-topbar.tsx

@premsathisha
Copy link
Copy Markdown
Contributor Author

on each refresh the letters fallback always shows for a bit, it kind of bugs me. any fix on that?

Fixed now by making AvatarFallback delay conditional, so it uses delayMs={user.image ? 400 : 0} and no longer flashes initials when no image exists.

@stylessh stylessh merged commit 4a0c0e2 into stylessh:main Apr 17, 2026
5 checks passed
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