Repository navigation
Polish channel actions, inbox, search, and composer attachments - #1078
Conversation
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
kalvinnchau
left a comment
There was a problem hiding this comment.
🤖 Found two lifecycle regressions and one failure-state usability defect. Fix the pending-operation ownership and completion behavior before merging; package validation also remains unresolved.
| )} | ||
| </Dialog> | ||
| <Dialog | ||
| open={!busy && !error} |
There was a problem hiding this comment.
🤖 [P2] Preserve submitted operations when the sidebar unmounts
After confirming Archive/Delete, navigate to Settings or Me while the operation is pending. Those routes unmount ChannelSidebar and this dialog, aborting the run, while the provider retains lifecycleDialog because beginLifecycle cleared its origin. Returning to channels then presents a fresh confirmation for the already-submitted action, without a result toast. A sidebar unmount/remount reproduction confirmed both the aborted signal and the reappearing Archive/Delete confirmation (2/2 cases). An abort after publication can also suppress the unconfirmed outcome. Keep submitted work and result reporting in an owner that survives sidebar swaps, while retaining session teardown cancellation.
| if (!controller.signal.aborted) completed(); | ||
| if (!controller.signal.aborted) { | ||
| notify(`${copy[action].done}: ${channelName}`, "success"); | ||
| completed(); |
There was a problem hiding this comment.
🤖 [P2] Use current navigation state when completing a pending action
Confirm Delete, delay signing, navigate to another conversation or Inbox, then complete signing. This calls the completed callback captured at confirmation, whose ChannelSidebar closure still holds the old target/sections; it redirects away from the user's new destination. Delete, Leave and Hide reproduced this in Chromium and WebKit; Archive controls retained route, focus and draft. A mounted React reproduction also invoked the original callback after the parent target changed. Invoke the latest callback or read live navigation state, and redirect only if the user still views the affected channel.
| title={busy ? copy[action].pending : `${copy[action].title} failed`} | ||
| description={busy ? channelName : error} | ||
| tone={busy ? "info" : "error"} | ||
| {...(!busy && { onDismiss: close })} |
There was a problem hiding this comment.
🤖 [P3] Release the lifecycle gate when displaying a failure
The persistent failure toast retains lifecycleDialog until dismissal. Sidebar/header lifecycle actions remain disabled, while channel Settings action buttons remain enabled but silently do nothing because openLifecycle returns when that state is set. The previous modal made this gate visible; the nonmodal toast leaves apparently usable buttons unresponsive. Release the operation gate on failure and own the toast separately, or disable the Settings buttons with an explanation. This finding is based on the state and caller paths, without a runtime reproduction.
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
kalvinnchau
left a comment
There was a problem hiding this comment.
🤖 The three previous sidebar findings are fixed, and the full package run now passes (9,030 passed, one real-relay fixture skipped). One reproduced lifecycle defect remains in actions started from Me/Sessions; fix that caller before merge.
| )} | ||
| </Dialog> | ||
| <Dialog | ||
| open={!busy && !error} |
There was a problem hiding this comment.
🤖 [P2] Preserve submitted Me/Sessions actions across visit changes
Making the shared dialog nonmodal while pending also affects LiveSessions, which still ties it to the originating visit (SessionsPage.tsx:163,256). Confirm Delete through Session actions, delay signing, select another Me conversation, then release: the visit abort unmounts the dialog and silently cancels the confirmed action. If navigation happens after upstream acceptance while the receipt is delayed, the write succeeds but all result feedback disappears. Both schedules reproduced in Chromium and WebKit, with matching no-navigation controls passing. Archive also has a mounted reproduction. The provider fix covers sidebar-origin actions but not this caller. Keep the Me dialog modal while pending, or move submitted execution and feedback to the session owner while preserving cancellation on actual session replacement.
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
kalvinnchau
left a comment
There was a problem hiding this comment.
🤖 Approved. Two nonblocking P3 follow-ups remain: disable Me/Settings lifecycle controls while an action is pending (they currently silently ignore activation), and remove the unused dialog-local failure fallback or move its test assertions onto the production provider path.
Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>

Polish channel actions, search, inbox, and composer attachments
Channel actions now close their confirmation into progress and result toasts, while navigation remains usable. Search and Inbox use consistent row styling, and composer attachments share spacing, corner geometry, controls, and scrolling behavior.
Changes
Validation
Latest repair: Me archive/delete now use the existing session-owned ChannelNavigationProvider and ChannelLifecycleDialog. Two deferred-operation regressions fail before the repair and pass afterward. Me tests pass 17/17, including success/failure feedback after leaving the page, pre-confirmation visit cancellation, and session replacement. Three affected unit files passed 49 tests before the final three Me boundary cases were added.
Complete Me workspace, archive/delete-pane, leave-pane, and lifecycle browser files pass 38/38 across Chromium and WebKit (1.7 minutes). Independent review found no blockers. No browser cases were added or removed for this repair; ordering and lifetime permutations use mounted React tests with deferred operations.
Integrated main
76420e81d, resolving the additive sidebar-test conflict by retaining lifecycle and upstream ordering coverage. No signing or publication semantics changed.Previous head
1599a471epassed all hosted required CI, DCO, security, 18 browser-journey shards, 11 flake checks, and macOS recording. These results do not establish CI success for the new head.Earlier focused checks exercised search/member dialogs, Inbox, tabs, attachments, voice notes, notifications, and unread behavior in both browser engines. Browser assertions were updated for the approved UI; pending states remain gated and persistence, recovery, focus, and overflow checks are retained. The unread test now establishes its startup baseline with the existing reading-focus gate before releasing the controlled clock. No retries, timeout increases, disabled engines, or relaxed thresholds were introduced.
Previous repair
515bcd8a7: all mandatory push hooks passed (types/related tests 366.74 seconds, design 4.38 seconds, Clippy 0.24 seconds). The first attempt caught an unsupported Promise API in new tests; replaced it with standard deferred promises, then reran the complete Me file (17/17) and all hooks. DCO passed at the new remote head; fresh hosted CI is running.Current head
b99a96b38integrates main9f21b2452and preserves both Select's preference variant and this PR's member-filter sizing. Design tests pass 164/164, design build passes, and member-focus/dialog-actions pass 30/30 across Chromium and WebKit. Initial push checks had intermittent local failures; the three remaining affected files subsequently passed unchanged (125/125). All mandatory push hooks then passed with the supported single-worker setting (types/related tests 968.12 seconds, design 4.93 seconds, Clippy 0.26 seconds). No assertions, timeouts, or coverage were weakened. DCO passed at the new head; fresh hosted CI is pending.Visual evidence and remaining acceptance
Shared owners include Dialog, Select, NavigationItem, IconButton, EmptyState, and Toast. The composer fixture was visually exercised for mixed attachment geometry, scrolling, and keyboard focus. Browser inspection verified member/search scrollports reach the bottom and Inbox keyboard focus swaps the date for Archive. The later 64ch preview cap has not been visually verified.
Full native, both-theme, narrow/enlarged-text, touch, and complete-batch human acceptance remain outstanding. Live destructive writes were not performed. The daily build's stale development-server/native session was restarted; browser verification loaded the configured community, channels, history, and draft, but the rebuilt native window was not directly inspected. The temporary composer playground was removed.
Required reviewer approval and verification of the open review threads remain pending. Arjun explicitly requested ready-for-review status throughout repairs; this is not human acceptance. Current-head hosted checks remain a separate gate.