Skip to content

Fix sidebar avatar loading fallback - #9375

Merged
lawrencecchen merged 5 commits into
mainfrom
feat-sidebar-avatar-loading-icon
Aug 1, 2026
Merged

lawrencecchen merged 5 commits into
mainfrom
feat-sidebar-avatar-loading-icon

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Aug 1, 2026 •

Copy link
Copy Markdown
Contributor

The bottom-left account avatar now shows the profile icon while a remote image is loading. Missing or failed images use the name initial with neutral gray styling; loaded images remain unchanged.

Verified with tagged macOS build avload and UI-test auth fixtures for a stalled image request and a user without a profile image. No user-facing strings changed.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Improve sidebar account avatar loading fallback: show an optional system icon while the image loads and use neutral gray styling during loading; accent color only when no loading icon. Keep the icon visible during session restore; loaded images remain unchanged.

  • Refactors
    • StackAccountAvatarView accepts loadingSystemName and renders that icon at avatar size with secondaryLabelColor while loading; the sidebar passes signedOutSystemName and keeps it during session restore.
    • Sidebar presentation now considers isRestoringSession (exposed by HostAccountFlow) to avoid showing a profile picture while restoring; fallback tint derives from loadingSystemName.

Written for commit 1e7d2ca. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Improved account avatar loading states with optional system-icon placeholders.
    • Loading placeholders now use the configured icon size and secondary-label color.
    • Fallback avatars adapt their colors for clearer loading and signed-out states.
    • Sidebar avatars now display consistent icons while images load or sessions are being restored.
    • Added consistent visual treatment across loading, signed-in, signed-out, and session-restoration states.

@coderabbitai

coderabbitai Bot commented Aug 1, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

Account avatar loading now supports a configured system icon and adjusted fallback colors. Sidebar account presentation now uses the profile icon during session restoration. Tests cover the restoration presentation.

Account avatar rendering and restoration

Layer / File(s) Summary
Avatar loading and fallback styling
Sources/Auth/StackAccountAvatarView.swift
Renders the configured loading icon and selects fallback colors from loading-icon configuration.
Session-restoration account presentation
Sources/Auth/HostAccountFlow.swift, Sources/VerticalTabsSidebar+EmptyAreasAndFooter.swift
Exposes restoration state and selects the profile icon while restoration is active.
Sidebar avatar configuration and validation
Sources/VerticalTabsSidebar+EmptyAreasAndFooter.swift, cmuxTests/WorkspaceContentViewVisibilityTests.swift
Uses signedOutSystemName for signed-in avatar loading and tests restoration presentation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant HostAccountFlow
  participant SidebarAccountButton
  participant SidebarAccountButtonPresentation
  participant StackAccountAvatarView
  HostAccountFlow->>SidebarAccountButton: expose isRestoringSession
  SidebarAccountButton->>SidebarAccountButtonPresentation: resolve account presentation
  SidebarAccountButtonPresentation-->>SidebarAccountButton: select profile icon during restoration
  SidebarAccountButton->>StackAccountAvatarView: configure signedOutSystemName
  StackAccountAvatarView-->>SidebarAccountButton: render loading system icon
Loading

Possibly related PRs

  • manaflow-ai/cmux#8354: Both changes modify shared account and avatar presentation code in HostAccountFlow and VerticalTabsSidebar+EmptyAreasAndFooter.swift.

Suggested reviewers: azooz2003-bit, austinywang


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (2 warnings, 1 inconclusive)

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.
Description check ⚠️ Warning The description includes the change summary and testing details but omits the required Demo Video, Review Trigger, and Checklist sections. Add the missing template sections, include a demo video or explain why none is available, and complete the checklist.
Cmux Swiftui State Layout ❓ Inconclusive The checked-out HEAD does not show the summarized avatar files in its latest diff, so the relevant PR diff is not yet identified. Provide the PR commit or checkout containing the four summarized Swift files so the state-layout changes can be verified.
✅ Passed checks (22 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed The only new state is HostAccountFlow.isRestoringSession, which reads the @MainActor AuthCoordinator; avatar and sidebar changes are UI types, with no new Sendable or background access.
Cmux Swift Blocking Runtime ✅ Passed The feature diff changes four Swift files and adds no semaphores, waits, sleeps, delayed dispatch, polling, sync queues, or locks; the test change is deterministic presentation coverage.
Cmux Browser Automation Off-Main ✅ Passed The PR diff changes only account-avatar flow and presentation; no browser automation command, worker router, WebKit wait, or policy code changed.
Cmux Expensive Synchronous Load ✅ Passed The PR only propagates restoration state and updates AsyncImage rendering; its production diff adds no agent-history loads, file scans, synchronous parsing, or main-actor I/O.
Cmux Cache Substitution Correctness ✅ Passed The diff changes only transient avatar rendering and forwards coordinator restoration state; it replaces no fresh persistence, history, undo, or snapshot read with a cache.
Cmux No Hacky Sleeps ✅ Passed The PR changes only Swift production files and a Swift test; the rule applies to TypeScript, JavaScript, shell, and non-Swift runtime scripts.
Cmux Algorithmic Complexity ✅ Passed The PR adds only avatar rendering, color selection, and session-state forwarding; changed production hunks add no scalable collection iteration, scans, sorting, joins, or rescans.
Cmux Swift Concurrency ✅ Passed The feature diff adds no DispatchQueue, Combine, completion-handler, or new fire-and-forget Task usage; it only adds synchronous state/presentation logic and an AsyncImage UI boundary.
Cmux Swift @Concurrent ✅ Passed The diff adds no async, nonisolated, or @concurrent work. The new state property remains @MainActor-isolated, and UI code only reads it synchronously.
Cmux Swift Package Boundaries ✅ Passed The diff only changes SwiftUI/AppKit avatar rendering, sidebar presentation, and a HostAccountFlow lifecycle projection; it introduces no reusable or independent domain logic.
Cmux Swiftpm Lockfiles ✅ Passed The PR range changes only four Swift source/test files; it changes no Package.swift, Package.resolved, .gitignore, workflow, or Xcode project package references.
Cmux Swift Logging ✅ Passed The feature branch changes four Swift files and adds no print, debugPrint, dump, NSLog, Logger, or file/stdout logging statements.
Cmux User-Facing Error Privacy ✅ Passed The production diff adds no user-facing error, alert, command output, or recovery text; it only changes avatar rendering and session-state plumbing.
Cmux Full Internationalization ✅ Passed The PR changes avatar presentation and restoration state only; it adds no user-facing text, localization keys, catalogs, or locale-specific web content.
Cmux Architecture Rethink ✅ Passed The diff adds a computed projection of coordinator-owned isRestoringSession and a pure presentation rule; it adds no timing repair, mutable side channel, duplicate action path, or new UI lifecycle...
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR changes only avatar, sidebar account-flow, and test code. It adds no NSWindow, NSPanel, Window, WindowGroup, or close-shortcut routing.
Cmux Source Artifacts ✅ Passed The feature diff contains only four regular tracked Swift source/test files under Sources/ and cmuxTests; no artifact-like paths or binary/log files were added.
Cmux No Test Or Debug Seam In Production Source ✅ Passed The diff adds only product behavior and a production caller for isRestoringSession; it adds no test/debug-named seam, test guard, or visibility widening in Sources.
Cmux No Ambient Global State ✅ Passed The production diff adds an AppDelegate instance method, enum cases, and a stateful constructable BrowserAuthCallbackNavigationPolicy; it adds no top-level API, mutable global, or new singleton.
Title check ✅ Passed The title clearly describes the primary change to the sidebar avatar loading fallback.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-sidebar-avatar-loading-icon

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Sources/Auth/StackAccountAvatarView.swift`:
- Around line 59-71: Update the .systemIcon branch of loadingView to pass the
existing CmuxSystemSymbolImage through framedAvatar(...), preserving its
systemName, pointSize, weight, and foregroundStyle configuration so all loading
states share the avatar framing.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 24cf1224-dda8-41f0-a884-58b57838e5cc

📥 Commits

Reviewing files that changed from the base of the PR and between a102695 and 80b3ca1.

📒 Files selected for processing (2)
  • Sources/Auth/StackAccountAvatarView.swift
  • Sources/VerticalTabsSidebar+EmptyAreasAndFooter.swift

Comment thread Sources/Auth/StackAccountAvatarView.swift Outdated
@lawrencecchen
lawrencecchen force-pushed the feat-sidebar-avatar-loading-icon branch from 2c67313 to 1094425 Compare August 1, 2026 10:29
@lawrencecchen
lawrencecchen merged commit 367f2ef into main Aug 1, 2026
6 checks passed
@lawrencecchen
lawrencecchen deleted the feat-sidebar-avatar-loading-icon branch August 1, 2026 11:26
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.

1 participant