Skip to content

Show a neutral initial when an avatar fails to load - #295

Merged
MaggieAppleton merged 10 commits into
mainfrom
design/avatar-fallback
Oct 7, 2026
Merged

MaggieAppleton merged 10 commits into
mainfrom
design/avatar-fallback

Conversation

@MaggieAppleton

@MaggieAppleton MaggieAppleton commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Integration and validation

Use one shared human avatar with a tinted initial while loading or after failure. Keep the current Chopin mark, make faces decorative beside visible names, and reset load state when the handle changes. Preserve word-boundary comment excerpts and the exact-limit regression.

Rebased onto current main. Reviewed the changed Face colour expressions and merged sidebar boundary before renewing their source pins. Formatting/lint/design checks, types, 30 focused tests and all 21 avatar/mention/navigation browser tests pass locally. Final required CI is running.

Original proposal and screenshots

Why

When an avatar URL failed to load, the sidebar account row showed the browser's broken-image icon. Chat and presence faces fell back to a solid account-colour block. The sidebar used the API's avatarUrl while chat and presence used https://github.com/<login>.png. Separately, the "dismissed a comment" notice cut its quoted excerpt mid-word with no ellipsis.

What changed

  • Face (packages/editor/src/face.tsx) is now the one avatar. Until the photo loads, and if it fails, it shows a tile tinted with the person's cursor colour (18% over --color-page, opaque so overlapping faces stay clean) with the initial in the same hue mixed 80% toward --color-text-primary (text-xs, weight 600), same size and radius as the photo. Presence, chat, the evidence popover, the mention picker and the sidebar account row all use it.
  • Source rule: https://github.com/<login>.png, which chat and presence already used. The login is the identity we hold everywhere, and avatarUrl is only on the session user, so it cannot serve other people. The mention picker's private copy of Face is removed.
  • Shape stays the rounded square that Face uses for people (the agent keeps its circle), so the account row avatar goes from a circle to a rounded square. This follows the existing "a person is a square, the agent a circle" rule in face.tsx; say if you want the account row to stay round.
  • Face gets a decorative prop (empty alt) for places where the name is written next to it.
  • excerpt() in apps/server/src/comments/service.ts now cuts at a word boundary (falling back to a hard cut if no boundary leaves at least half) and appends …. Exported and unit tested.
  • The existing color(handle) design exception for face.tsx is kept as it was, but its expressions changed (see below).

The notice before: @octocat dismissed a comment on "…reach a share". After: the excerpt ends on a whole word followed by ….

Screenshots

Photos blocked on both servers to force the failure (the fake GitHub's example.invalid URLs fail on their own).

Sidebar account row. Before:

before account row

After:

after account row

Chat author. Before:

before chat author

After:

after chat author

Presence stack. Before:

before presence

After:

after presence

The notice is covered by unit tests rather than a screenshot.

Testing

  • bun test apps/server/src/comments/excerpt.test.ts: 4 pass
  • bun test apps/web/src/chat/mentions.test.ts apps/web/src/navigation-chrome.test.ts: 34 pass (mention test updated: the loading-state initial is aria-hidden)
  • bun test apps/web packages/editor: all pass except one design-contract focus test that timed out at 10s under load and passes alone
  • bun run types: pass. bun run fix: no further changes.
  • bun run ci: fails only on the design-contract review below.
  • Local E2E suspended per brief; relying on CI.

Needs a design-contract review

  • apps/web/src/project-sidebar.tsx, scripts/design-contract/exceptions/dynamic-web.json (NavigationIcon className case). New hash: 74828419d9903e2784093b4f9bee1ab52ddf0b939f27870cabed13d139224ddb. The edit swaps the account-row <img> for <Face> and adds no class or style logic to NavigationIcon.
  • packages/editor/src/face.tsx, scripts/design-contract/exceptions/preserved-values.json. The old case background: color(handle) is replaced by two cases: background: color-mix(in srgb, ${tone} 18%, var(--color-page)) and color: color-mix(in srgb, ${tone} 80%, var(--color-text-primary)), where tone is still color(handle). New hash: 830c3d5275279f75c3fa0da9d01f5d4802daff0deb0923f95df1072150b9d11e. Same reviewed purpose (identity colour from the handle, matching the cursor), now as a tint with a darker initial built from tokens.

🤖 Generated with Claude Code

@MaggieAppleton

Copy link
Copy Markdown
Collaborator Author

Updated per design note: the fallback tile now uses a soft tint of the person's cursor colour (18% over page) with the initial in a darker tone (80% over black). Retaken crops below.

after account row

after presence

after account row

after presence

@MaggieAppleton

Copy link
Copy Markdown
Collaborator Author

Review: Looks good

Checked in a worktree on :8924 with github.com photos aborted, delayed 3s, and real. Three handles (octocat, alice, bob) were in one document.

  • Contrast: initial on tint is 4.90-5.10:1 for all 8 palette tones (sRGB mix against white and gray-900, matching color-mix). The app has no dark theme, so there is no dark-on-dark case.
  • Tint: computed colours match each cursor colour. alice is #A45B9F, bob #358264, octocat #7E65BB, and the tint is the same in sidebar, chat, mention picker and presence.
  • Loading to loaded: the box stays 24x24 at the same x/y, no layout shift. The initial is removed when the photo loads, so there is a hard cut but no flash or double paint.
  • Presence stack: three overlapping faces stay opaque, with no bleed-through. Initials remain legible, and the group label and tooltips are unchanged.
  • Excerpt: 29 tests pass, and I also tried long, no-space, CJK and punctuation inputs; all cut cleanly with ….
  • Design-contract exceptions: correct and minimal. The two new expressions are the exact tone mixes against tokens, and the old color(handle) case is rightly stale. The project-sidebar.tsx renewal is justified because it only swaps <img> for <Face>.

Non-blocking:

  1. apps/web/src/chat/transcript.tsx:306: the chat Face (role=img, label and title) sits beside the written name ("Octocat"), so a screen reader hears it twice. The new decorative prop fits here. Pass decorative and titled={false}.
  2. packages/editor/src/face.tsx:72-73: failed and loaded never reset if handle changes on a mounted Face. Key the Face by handle or reset on change.
  3. packages/editor/src/face.tsx:16-18: the new comment lines are over 100 columns. Rewrap.
  4. apps/server/src/comments/excerpt.test.ts:14: the test name "word that ends exactly at the limit" does not match its input. The word ends at 59, so the value[60] === " " branch is untested. Add a case whose word ends at index 60.

@MaggieAppleton

Copy link
Copy Markdown
Collaborator Author

Applied the four nits: chat Face is now decorative and untitled; Face is keyed by handle so load state resets; face.tsx comment rewrapped; excerpt tests renamed and a boundary case (space at index 60) added. face.tsx new hash in the PR body: 830c3d5275279f75c3fa0da9d01f5d4802daff0deb0923f95df1072150b9d11e. The design-contract check is still the only failing job.

lavaman131 added a commit that referenced this pull request Oct 6, 2026
# Conflicts:
#	apps/web/src/project-sidebar.tsx
lavaman131 added a commit that referenced this pull request Oct 6, 2026
Fixes that only exist because several open PRs now share one tree:

- Drop #290's duplicate ImageIcon and its LineIcon titles, which #287 removed.
- Remove the import #302 left unused after #276's inline code.
- Renew design-contract pins for files more than one PR edited, and record
  #290's new icons and #295's face colours as reviewed exceptions.
- Find icons by path rather than the titles #287 removed, and faces by the
  label #295 gives them.
- A tool that returns after its turn was stopped ends as failed, so #314's
  early completion keeps #272's stopped-question outcome.

Assistant-model: Claude Opus 5.5
Assistant-workflow: inline
Assistant-verification: typecheck passed: bun run types
Assistant-verification: dprint/oxlint/design checks passed: bun run ci
Assistant-verification: bun test passed: 3860 pass, 0 fail
Co-authored-by: Alex Lavaee <lavaman131@github.com>
@MaggieAppleton
MaggieAppleton force-pushed the design/avatar-fallback branch 2 times, most recently from 86f2f67 to 4092977 Compare October 7, 2026 12:02
@MaggieAppleton

Copy link
Copy Markdown
Collaborator Author

Commit pushed: 6679759

Generated by PR CI fixer · gpt54 · 223.2 AIC · ⌖ 15 AIC · ⊞ 15.1K

@MaggieAppleton

This comment has been minimized.

@MaggieAppleton
MaggieAppleton force-pushed the design/avatar-fallback branch 3 times, most recently from 50234ca to e19de1c Compare October 7, 2026 13:56
@MaggieAppleton

Copy link
Copy Markdown
Collaborator Author

CI babysitter rechecked the current failed ci run for e19de1c75eac2042d9814969e8049598ee98c408. This is no longer only a stale design-contract-exception failure: the first meaningful errors are now two test regressions introduced after the prior [ci-fix] commit refreshed the exception hashes.

Root cause:

  • apps/web/src/navigation-chrome.test.ts:177 still expects the old exact inline style string style="width:24px;height:24px", but Face now renders additional background and color inline styles for the fallback tint.
  • scripts/design-contract/boundaries.test.ts:19 still tries to mutate the removed source text background: color(handle), so the change callback is now a no-op and the boundary test fails before it reaches the intended assertion.

I did not queue another commit from this runner because bun is not installed here and the environment blocked downloading Bun (curl to the Bun release returned HTTP 403), so I could not run the required local reproduction and verification steps (bun install --frozen-lockfile, the failing tests, bun run fix, and bun run ci). Next action for a human or a runner with Bun 1.4.2 available: update those two tests on this head, rerun the failing test file plus bun run ci, and only then push a follow-up fix if it stays green.

babysit-head:e19de1c75eac2042d9814969e8049598ee98c408

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • releaseassets.githubusercontent.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "releaseassets.githubusercontent.com"

See Network Configuration for more information.

Generated by PR CI fixer · gpt54 · 42.8 AIC · ⌖ 2.8 AIC · ⊞ 16.4K · ◷

MaggieAppleton and others added 10 commits October 8, 2026 00:02
…xcerpts on a word

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… text

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…undary excerpt

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MaggieAppleton
MaggieAppleton force-pushed the design/avatar-fallback branch from 99a11d7 to ebb08b1 Compare October 7, 2026 23:05
@MaggieAppleton
MaggieAppleton merged commit ebf436d into main Oct 7, 2026
3 checks passed
@MaggieAppleton
MaggieAppleton deleted the design/avatar-fallback branch October 7, 2026 23:15
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