Polish settings hierarchy, controls, and profile actions

[mahanti]

Polish settings hierarchy, controls, and profile actions

Settings used inconsistent headings and row spacing, always showed profile actions, and expanded shortcut rows when capture began. This change introduces shared Header/InlineHeader layouts, lighter light-mode fields and hover fills, and a small Input size. Profile actions now appear only for edits, right-aligned as Cancel then Save; the account menu retains the shell avatar's presence badge.

Notifications retains main's shared PreferenceRow, category order and dependent disabled states, with a compact permission banner whose actions wrap on narrow screens. Appearance aligns Text size with Color mode, removes redundant help text, and shows Reset only away from 100%. Plugin and shortcut rows use reduced horizontal padding. New shared components have maintained design-viewer examples and usage documentation. Dark-mode palette values are unchanged.

Takeover and integration

Pushed head 5b0d224 includes main 64be4c2 (latest fetched at delivery). Existing author history is preserved; no force-push. The refresh retains main’s avatar editor and confirmed profile publishing alongside the PR’s conditional actions and keyboard-focus behavior.

• Retained main's community-scoped asynchronous profile publishing, notifications, account availability/status controls, feedback account action and sidebar changes. Removed this branch's redundant SettingRow in favor of main's PreferenceRow; corrected token metadata and browser assertions.
• Profile Save remains focusable while publication is pending. Ref cleanup records focus ownership; a layout effect hands focus to Display name after inputs are enabled. Failed saves retain Save for retry. Implicit submission and unrelated updates do not steal focus.
• Appearance Reset uses the same post-DOM-update handoff so Increase can receive focus when resetting from 200%. Conditional visibility, external-update focus guards, storage-failure behavior and unmount safety are retained.

Three inherited unit failures were independently reproduced on clean main 0908663. Commit 2e4b513 repairs only their tests: PluginImport's current warning copy, ProfileAgentIdentity's subject-owned kind-10100 capabilities read, and MessageRow's incomplete session.messages fixture. Latest main #242 independently repaired these three tests too. The final integration uses its MessageRow fixture and retains the stronger warning/capabilities assertions without duplicates. Both ProfileButton test sets are retained with the new accountActions fixture. No unrelated production changes or hook bypasses.

Latest main refresh and validation

• Retained Arjun’s light-mode palette by explicit owner decision. The Settings organization from Organize app and community settings #173 and newer avatar/profile-save behavior remain intact. No additional production redesign was needed.
• The inherited agent-deletion assertion mismatch was fixed independently by merged test(agents): Match native delete failure guidance #299 while our equivalent owner-authorized correction was being pushed. The final integration takes test(agents): Match native delete failure guidance #299’s test verbatim: ProfileAgentArchive.test.tsx now has no diff against main. No agent production code changed.
• Full Vitest at clean e39c344: 369 files / 4,087 tests passed (37.89s). The only subsequent file change takes test(agents): Match native delete failure guidance #299’s test assertion verbatim. At final 5b0d224, mandatory commit/push hooks passed: TypeScript, 827 related tests across 77 files, design types and every design guard. Full Vitest was not redundantly rerun after adopting main’s assertion.
• Integration browser run before merge commit: 14 cases passed in Chromium/WebKit, covering Settings and avatar flows, including matched channel/notification Settings files. Exact state: old HEAD 584395e plus incoming main 8d05fcf with resolved working files. Subsequent main e52ec14 adds unrelated feedback work; final e39c344 adds only the authorized test correction.
• At clean 29c522b (same production tree as 5b0d224), a representative light-mode visual probe completed in both Chromium and WebKit: composer, sidebar hover/selection, profile input and keyboard-action focus, Notifications, Appearance selection/focus and narrow layout. Screenshots compared PR styling against temporary main-palette-only overrides on the integrated layout, not a separate full-main build. No concrete regression found in inspected states; independent contrast checks also passed. Probe and screenshots remain outside Git.
• No accidental screenshots or generated review artifacts in the PR diff. Native/package acceptance, a full accessibility audit, and explicit human smoke-test confirmation remain outstanding. Required hosted CI must pass at 5b0d224; earlier green CI is not final-head evidence.

Earlier takeover validation (historical heads)

• Full Vitest package suite at 584395e: 363 files / 3,984 tests passed, 48.97 seconds.
• Mandatory push hooks at 584395e: TypeScript, 764 related tests across 73 files, design types and all design guards passed. Hosted DCO Check passed at that head.
• After integrating main f761867 (exact working tree committed as 584395e, no formatter edits): all 8 Settings browser cases passed in Chromium and WebKit, including the new feedback/menu order and Profile save lifecycle.
• Before the first integration commit: 32 browser cases across Settings, Notifications, plugin import and shortcuts passed in Chromium and WebKit. Async community Save reproduced failure before repair, then passed pending/rejection/retry/success and subsequent Tab-order assertions.
• After the 200% Reset fix (working delta committed as 5ef66b5): all 12 shortcuts cases passed across Chromium and WebKit. The new maximum-scale focus assertion failed in both engines before the repair. The existing zoom journey now also checks focused and unfocused external resets. No browser cases were added, removed or moved; actual browser focus and Tab order justify this coverage. Unit tests cover lower/max-scale reset, storage failure and unmount.
• Source-only independent reviews covered focus lifecycle, shared controls/current-main integration and inherited test repairs; no remaining material code finding. Full PR file inventory contains no accidental screenshots or generated review artifacts.

Required hosted CI still needs a successful run at the final head; the old head's green run is not final-head evidence. Native/package acceptance, Windows validation and a complete app-wide visual audit remain deferred. Original screenshots remain local, not GitHub attachments.

Remaining merge gates / human smoke test

The owner has marked this PR ready; that state was preserved. No agent approval or merge was performed, and no human smoke test is claimed complete. Required final-head CI and human/code-owner re-review must complete; the existing changes-requested review still stands.

1. In a community, edit Profile, Tab to Save and press Enter. Save should retain focus while pending, then focus Display name on success; Cancel should also return focus to Display name. If publication fails, Save should remain focused and retryable.
2. In Appearance, set Text size to 200%, focus Reset and press Enter. Size should return to 100% and Increase text size should receive focus.
3. Check Notifications and light-mode Settings/composer controls for the intended spacing and fills, including at a narrow window width. Confirm the human smoke test before marking ready for review.

Takeover notes by Carl, an automated reviewer, commenting via Wes’s GitHub account.

[mahanti]

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested: one keyboard-focus regression across the conditional settings actions, detailed inline. Keep the intended conditional visibility; the fix is a guarded focus handoff, not restoring always-visible buttons.

• Source-only review of ec5d979a391fb0feebd43264b640a248cbe82e16 against 119195ea331de33c8480bab180df0091ca8e9421, including settings state/persistence owners and shared-control consumers. No PR code was executed by this review.
• Existing CI run 36065115172 passed on merge e45bc9f of these pins: 3,318 unit tests, 600 browser cases, and 7 measurements. The changed count, presence-selector, and palette assertions preserve the intended boundaries; they do not verify focused-control retirement. Actual next-Tab behavior, native/packaged acceptance, and app-wide visual acceptance remain unverified; Windows validation was skipped.
• Merge criteria: preserve focus for the retiring focused actions and cover those transitions; resolve the current merge conflicts and validate the resulting head. The existing CI result is not current-main readiness.

[wesbillman]

Star Lord’s automated source review via Wes’s GitHub account.

One actionable P2 remains in Appearance’s focus handoff, detailed inline. The earlier Profile Save/Cancel finding is addressed at source level: cleanup transfers focus only when the retiring actions contain it, and the tests cover failed save/retry, implicit submission and unrelated external updates. The Appearance repair handles an enabled destination but misses the 200% boundary. Keep the intended conditional visibility; do not restore always-visible actions.

Reviewed head dc9f0756a0a0fc59c8549f2b08d7a8891ceedb0f against base/merge-base df7b7e7f45739f3e06e12d81623385701acdc51d. Reviewed the actual feature diff, settings owners/callers, shared controls and changed browser assertions; Mantis’s bounded focus/lifecycle review is reconciled. No other actionable source defects found in that scope. Source came from pinned Git objects, not dirty working-tree files.

Evidence and limits: no PR code, tests, builds or app workflows were executed by this review. React 19.2.8 and the pinned Base UI 1.8.0 / utils 0.4.0 ref and disabled-control implementations were inspected as source. A read-only current-head check snapshot verifies success for JavaScript, Rust/tool integration, all four browser shards, measurements, CI required, DCO, Semgrep and zizmor in/alongside run 36124838337; Windows native validation was skipped. This is check-status evidence, not an independent rerun or verification of the author’s detailed test counts. The 200% keyboard transition, native/packaged behavior and app-wide visual acceptance remain unverified at runtime.

This is a non-blocking COMMENT review, not approval or merge authorization; it does not dismiss the earlier review.

[wesbillman]

Star Lord’s automated source review via Wes’s GitHub account.

No remaining actionable findings in this follow-up source review. The previously reported 200% Reset defect is addressed: AppearanceSettings.tsx:27–41 records focus ownership during ref cleanup, then focuses Increase in a layout effect after the disabled state is updated. The conditional Reset visibility remains intact. The added source assertions cover maximum-scale reset, storage failure, focused/unfocused external resets and unmount.

The Profile integration uses the same post-commit handoff (ProfileSettings.tsx:52–69); Save uses the shared focusable loading state while publication is pending, and rejected publication retains the actions for retry. I also checked the affected callers/shared controls and the latest merge’s test-conflict resolutions. No new material defect was found in those changes. This is a follow-up on the prior findings and integration, not a fresh audit of unrelated code inherited from main.

Reviewed head 584395edd1eb6de7a1a6a5dc6cc963d5f5f87949 against base/merge-base f761867ed81f25604933620f9b4747a871a69c04, using byte-verified pinned Git objects; no dirty working-tree source inputs.

Validation limits: source-only; no PR code, tests, builds or app workflows were executed. One read-only current-head check snapshot reports JavaScript, Rust/tool integration, all four browser shards, measurements, CI required, DCO, Semgrep and zizmor successful in/alongside run 36176006481; Windows native validation was skipped. This is hosted status evidence, not an independent rerun or verification of the author’s test counts. Native/packaged acceptance, human smoke testing and app-wide visual acceptance remain unverified by this review.

Non-blocking COMMENT only: not approval, dismissal of the earlier review, or merge authorization.

[wesbillman]

Star Lord’s automated source review via Wes’s GitHub account.

No new actionable findings in this follow-up source review. The new avatar/settings integration preserves the previously repaired conditional-action behavior: Profile Save remains focusable while pending, failure keeps edits for retry, and the retiring focused actions hand off to Display name after the DOM update. Avatar editing gates Save/Cancel while the editor is open, and the viewer/community key in Settings retires the previous editor on selection changes. The 200% Appearance Reset repair is unchanged from the previously covered revision.

Reviewed head 5b0d22479124851103c8dfd16a0b38bb1cc1bab2 against base/merge-base 64be4c2adf21e9920d4a7666354ceab1d784dd44. Scope: changes since covered head 584395edd1eb6de7a1a6a5dc6cc963d5f5f87949 that intersect this feature, including ProfileSettings/Settings integration, relevant shared-control and avatar callers, and the changed unit/browser assertions. The inherited agent-delete assertion now has no diff against base. This is not a fresh audit of unrelated code inherited from main. Source was read from hash-verified pinned Git objects, with no dirty working-tree source inputs.

Validation limits: source-only; no PR code, tests, builds or app workflows were executed. The updated browser source retains the pending/failure/retry/success and keyboard-focus assertions and adds/removes no browser cases; this is source inspection, not runtime evidence. One current-head CI snapshot of run 36185267144 showed Browser measurements successful while JavaScript, Rust/tool integration and all six browser shards were still running; Windows native validation was skipped. DCO, Semgrep and zizmor were successful. CI completion, native/packaged behavior, human smoke testing and app-wide visual/accessibility acceptance remain unverified by this review. Author-reported local results were not independently rerun.

Non-blocking COMMENT only: not approval, dismissal of the earlier review, or merge authorization.