Skip to content

Show channel member avatars with a flexible overflow capsule - #1566

Open
mahanti wants to merge 3 commits into
mainfrom
am-1022-channel-members
Open

mahanti wants to merge 3 commits into
mainfrom
am-1022-channel-members

Conversation

@mahanti

@mahanti mahanti commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Show up to three human member avatars in the channel toolbar, followed by a +X overflow count. Prefer members with profile pictures, retain the existing members dialog and accessible trigger, and use the shared avatar control treatment without a grouped hover background.

Allow the shared small AvatarStack overflow badge to grow into a capsule with 0.375rem horizontal padding, so multi-digit counts fit. Thread reply summaries share this sizing fix.

Validation

  • Agent code and maintainability reviews: no blockers; optional roster-hook simplification deferred.
  • 53 focused tests passed across preview, channel conversion, and members-dialog coverage.
  • TypeScript and design checks passed.
  • Mandatory commit and push hooks passed on e60f805.
  • Native development build launched; configured identity and relay connection preserved. Browser counterpart loaded the community.

Remaining checks

Draft pending final human acceptance and complete rendered checks in both themes, narrow/wide widths, enlarged text, and pointer/keyboard interaction. Browser capture automation did not reliably activate the intended populated channel, so no representative final screenshot is attached yet. No browser tests were added or removed.

mahanti and others added 2 commits October 22, 2026 16:06
Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
@mahanti
mahanti marked this pull request as ready for review October 22, 2026 21:01
@mahanti
mahanti requested a review from a team as a code owner October 22, 2026 21:01

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (systems/performance/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/accessibility) reviews.

Reviewed head e60f8053dd6a8cec4abe68b7470bffa03c452c39 against base d92bfb2c4dd89fc5ae6e7a1445f38ad1e8ab7060.

One P2 finding is attached inline. Source assessment: 8/10; keep optional toolbar profile enrichment behind conversation reads to reach the intended bar. The shared AvatarStack/IconButton composition preserves the members dialog trigger, accessible name, and focus-return target. The overflow sizing change also reaches thread reply summaries.

Validation: immutable source/diff inspection, relevant product/design guidance, callers, profile scheduling, and success/error/retry paths. The public PR description and both commit messages were inspected; no attached images and no evident sensitive disclosure. The added mounted component test covers profile arrival/reordering and membership shrink; zero browser cases were added or removed. No tests, builds, installs, apps, or PR code were executed, and the author’s reported checks were not reproduced. The single hosted-check snapshot at approximately 21:13 UTC showed 35 successful checks and two skipped checks, including Windows native validation. Cold/warm performance, rendered capsule/layout behavior in both themes and at narrow/enlarged-text sizes, keyboard/screen-reader behavior, native behavior, and human acceptance remain unverified. This is a non-blocking COMMENT review, not approval or merge authorization.

useEffect(() => {
if (memberKey)
void session.profiles
.ensure(memberKey.split(":"), "foreground")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 [P2] Keep automatic toolbar avatar enrichment in the background

This runs whenever the channel toolbar mounts or its roster changes, without the person opening the members dialog. Passing "foreground" makes optional full-roster profile reads compete with conversation history: profile-directory.ts:105–110 issues one read per 500 missing identities, and reader.ts:144–149 allows three active reads while limiting only background work to one. During rapid switches between uncached rosters (or a large cold roster), these session-owned reads can occupy the conversation slots and remain running after the toolbar switches away. Existing profile deduplication only helps identities already cached or pending; it does not cover the rest of the roster.

Use "background" here, matching timeline enrichment in store.ts:483–490 and the explicit optional-avatar priority contract in docs/contributing.md:591–595. Keep the intentional dialog fetch separate. Add a regression assertion for the background priority, and include cold/warm channel switching in the remaining performance check; no latency measurement was performed in this review.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 Fixed in d1cef1e: automatic toolbar avatar enrichment now requests background priority. The explicit members-dialog fetch remains foreground. Extended the existing mounted component test to assert background priority on initial mount and membership updates; it failed on the previous code and passes with the fix. All 49 component/dialog tests and required pre-push checks passed. No browser cases added or removed. Cold/warm channel-switch latency was not measured; hosted CI is pending.

Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
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