feat(shell): open your profile from the account menu avatar - #390
Conversation
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord automated source review — via Wes's account
Reviewed head 85eff698db565f11199f3d913f2636f5ba409153 against pinned base df4b735c8bfa98018ee4e65de60b5fb8fd0e6f31.
Three actionable comments below: one first-open failure-state focus defect, one stale Settings keyboard journey, and one unit-fixture incompatibility demonstrated by the hosted merge build. The existing profile/session owner and retained companion approach fit the requested scope; no parallel data owner is needed. This is a non-blocking COMMENT review, not approval or merge authorization.
Validation and public-material limits
- Source-only: no PR code, tests, app, browser flow, or native build executed locally. Inspected callers, retained-panel lifecycle, initial/unavailable focus paths and existing profile retry handling.
- Hosted snapshot: run 36517276051 failed JavaScript and Chromium/WebKit journey shard 2. JavaScript recorded 419 passing files / 5,089 passing tests, with the new avatar assertion as the sole failure; both browser failures stop at the old Settings keyboard-focus expectation. This run tested merge
ace8198389d7500cbf0fbd2f14fb4084b34a9ca5(this head merged intocfbb61246425cbfd18c10c33ab96d63fedf17942), not the isolated reviewed head/base. I inspected the relevant merge-source differences rather than attributing its presence behavior to the branch source. - Public repository: inspected the changed code/tests/docs, commit messages, and current PR description. No internal coordination URL, deployment identifier, or secret found in those inspected materials. The description now attaches an MP4 demo; it was downloadable but could not be visually inspected with the available tools, so its visual/privacy surface remains unverified. No image attachments were present in the description.
- The PR description still needs the repository-required browser-case accounting/justification and fail-then-pass evidence (one browser case added, none removed). Browser focus/keyboard routing is the stated browser-only boundary. Hosted repairs, native acceptance, and human confirmation remain outstanding; no
buzz-review-completedattestation is established here.
Clicking the avatar inside the top-right account menu now opens the viewer's own community profile in the shell companion panel, reusing the registered `profile` panel that other profile links resolve to. - usePanelLauncher tracks the opened target and can open any resolved target, not only header launchers; canOpen gates availability. - App hands ProfileButton an onProfile callback while a community is selected and a profile panel is registered. Personal space has no community profile, so its menu avatar stays presentational. - The menu avatar renders as the first menu item, so keyboard openings land on it; activating it closes the menu and hands focus to the panel, and closing the panel returns focus to the header avatar. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Render shell companions on the New message route while keeping channel-specific panels hidden. Preserve the mounted composer so viewing and closing the account profile does not discard recipients or draft text. Cover the account-menu entry point with the real Channels and Profiles plugins, including draft retention and continued editing after closing. Signed-off-by: Matt Toohey <contact@matttoohey.com>
Focus the retained companion card after a repeated opening commits, so the account menu hands focus back to the profile and Escape can close it. Keep the profile mounted to preserve its selected tab. Add component and Chromium/WebKit regression coverage for mouse and keyboard reopening, retained profile state, Escape dismissal, and focus restoration to the account avatar. Signed-off-by: Matt Toohey <contact@matttoohey.com>
Set nativeButton on the account-menu avatar item to match the button rendered by IconButton and remove the Base UI development warning. Preserve the existing avatar styling. Signed-off-by: Matt Toohey <contact@matttoohey.com>
Focus a newly opened companion card when its content has not taken focus, including unavailable profiles, while preserving ready profile focus and retained tabs. Cover keyboard opening, Escape dismissal and avatar focus return. Align the account avatar fixture with observed-presence behavior from main, expecting no badge without session evidence. Update the Settings journey for the new first menu item while retaining submenu, dismissal, Tab, narrow/wide layout and Settings navigation coverage. Validation: TypeScript, 22 component/composition tests and 16 Chromium/WebKit checks passed. Reproduced the focus, presence assertion and Settings failures before fixing them. Add one browser case (two across the feature), remove none; real menu focus handoff and Escape routing require browser coverage. Native acceptance, human testing and hosted CI remain deferred. Signed-off-by: Matt Toohey <contact@matttoohey.com>
85eff69 to
e138639
Compare
The retained-profile reopen test failed intermittently on the keyboard
iteration. After a mouse click opens the account menu, Base UI's
FloatingFocusManager moves focus into the popup on the next animation
frame. findByRole("menu") resolves as soon as the popup mounts, so the
test could press Home while focus was still on the header trigger, which
only handles arrow keys, Enter and Space, and drops Home. A focus and
keydown timeline probe confirmed this ordering; the launcher's card
focus fallback, the menu close handoff and the panel mount focus are
not involved.
Wait for focus to enter the menu before sending keys. This matches the
existing settings journey, which asserts the Availability button is
focused before typing. No production code changes.
Validation: App.profile.test.tsx passed 12 of 12 whole-file runs, and
co-running with ProfileButton.test.tsx passed; tsc and biome clean.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Star Lord <b89298dbe87c6b3fd8a425b535d9d161f23c88d555a47925ae0b04dfe02b201e@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
No remaining code blockers found in this follow-up. Reviewed head 309d5228ac67e877400558410db4292a56324251 against base c6b47a5837fd8912dc84d98bb959714a23d70818. The earlier unavailable-profile focus, Settings keyboard sequence, presence fixture, and retained-reopen test-ordering findings are addressed; all four existing review threads are resolved. The final test change waits for actual menu focus rather than adding a delay or retry, consistent with Base UI’s deferred initial-focus implementation.
Hosted CI run 36538214625 is green: 420 Vitest files / 5,091 tests, Rust/tool integration, all Chromium/WebKit journey shards, measurements, and DCO/security checks. Logs confirm both new profile journeys and the existing Settings journey pass in both engines. That run checked merge 22b6d8f8da8c021de496bd8beec8c0246a40753f of the exact head/base above. I reviewed source and existing CI evidence; I did not rerun tests or exercise the native app locally. The clean head also passes git diff --check.
Ready for human approval on code quality; merge-readiness still needs human smoke-test confirmation. The PR description explicitly leaves human testing pending. Confirm avatar → own profile → repeat opening → Escape/focus return, including retaining a New message recipient/draft, then update the readiness checklist. The repair commit records browser-case accounting and fail-then-pass evidence, but the PR description has not been refreshed. These are readiness/documentation gaps, not additional code findings. Native acceptance remains unverified. This is a COMMENT review, not approval or merge authorization.
Public-material check: inspected changed source/tests/docs, commit text, PR description, and sampled frames from the attached synthetic-data demo; no disclosure issue found in those inspected materials. The demo is from an earlier head, not current-head runtime validation.
* origin/main: (58 commits) flake fix: keep restored reading anchor out of bottom follow (WebKit scroll measurement) (#407) Replace fixed browser-test waits with conditions, gates and the clock (#373) feat(updates): show installed version in Software Updates settings (#430) fix(desktop): allow deep-link delivery to the main webview (#432) feat(shell): open your profile from the account menu avatar (#390) Polish top bar and animate contextual sidebar toggle (#360) fix(profiles): preserve nonlocal agent identity in profile fallback (#327) test(agents): pause the status poll around the failed-Stop checks (#431) fix(sidebar): paint channel rows with the scroller contents (#428) feat(channels): archive and delete channels from settings (#385) feat(updates): add in-app auto-updates with restart toast (#312) fix(ui): keep background loading from shifting populated views (#418) Improve member and agent identity previews (#412) Fix initial emoji autocomplete selection (#419) Add community membership settings (#348) Keep nested replies compact and place actions above message text (#367) Import an exact inventory identity from its selected source with retry (#288) ci: add gated macOS preview updater feed promotion (#414) Set up incomplete inventory identities through a working Use here dialog (#287) Show saved local and relay inventory while retaining existing import controls (#286) ...
* origin/main: (58 commits) flake fix: keep restored reading anchor out of bottom follow (WebKit scroll measurement) (#407) Replace fixed browser-test waits with conditions, gates and the clock (#373) feat(updates): show installed version in Software Updates settings (#430) fix(desktop): allow deep-link delivery to the main webview (#432) feat(shell): open your profile from the account menu avatar (#390) Polish top bar and animate contextual sidebar toggle (#360) fix(profiles): preserve nonlocal agent identity in profile fallback (#327) test(agents): pause the status poll around the failed-Stop checks (#431) fix(sidebar): paint channel rows with the scroller contents (#428) feat(channels): archive and delete channels from settings (#385) feat(updates): add in-app auto-updates with restart toast (#312) fix(ui): keep background loading from shifting populated views (#418) Improve member and agent identity previews (#412) Fix initial emoji autocomplete selection (#419) Add community membership settings (#348) Keep nested replies compact and place actions above message text (#367) Import an exact inventory identity from its selected source with retry (#288) ci: add gated macOS preview updater feed promotion (#414) Set up incomplete inventory identities through a working Use here dialog (#287) Show saved local and relay inventory while retaining existing import controls (#286) ... Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Open your own community profile from the account menu avatar using the existing companion panel. Preserve keyboard navigation and focus return, and keep the avatar presentational in Personal space or when no profile panel is available.
Adds component and app wiring regression coverage and updates the shell design documentation.
Current validation —
abb51df1Integrated main
986f6881(including #327) without changing the feature beyond the automatic merge. Independent source review found the four previous findings addressed; the integrated wiring was rechecked.bin/pnpm test:browser account-profile.spec.mjs settings.spec.mjs profiles.spec.mjs --project chromium --project webkit --no-deps. The path patterns also include channel-settings and notification-settings.Pending: new-head hosted CI/DCO and human acceptance; browser tests are not native-app acceptance. No approval or merge was performed by the integrating agent.
Try: in a selected community, choose View your profile from the account-menu avatar, reopen with the panel already open, and press Escape. Focus should return to the header avatar. Repeat after a failed connection; opening/closing from New message should retain the recipient and unsent draft.
Profile-menu demo —
85eff698, Chromium, synthetic data. Open from a channel or New message; retain the recipient, draft and Channels tab across mouse/keyboard reopenings; close with Escape and continue editing. Capture assertions passed; native acceptance and human testing remain unverified.profile-menu.mp4