Skip to content

Keep the checklist popover when a row reparent's close finishes after reattach - #14830

Merged
teamleaderleo merged 1 commit into
mainfrom
fix/checklist-popover-animated-reparent
Sep 26, 2026
Merged

teamleaderleo merged 1 commit into
mainfrom
fix/checklist-popover-animated-reparent

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

A sidebar row reparent could close an open checklist popover and write presented = false for the workspace, so the popover vanished instead of coming back. SidebarWorkspaceRowSuspensionTests/transientWindowReparentingPreservesChecklistPopover() catches this and has failed on main on every owned Mac mini (macOS 26.5.1, real display) since app-host shards started running there; it still passes on Blacksmith macos-26 (26.3).

The popover animates. When the reparent detaches its anchor, the close it triggers is still animating when the row is back in the window, so isShown is still true. viewDidMoveToWindow took that to mean the popover survived and cleared the detach flag (#13988). When the close finished, onExternalDismiss saw no flag and treated it as a user click-away.

Now:

  • SidebarRowSwiftUIPopoverPresenter.isClosing is true from popoverWillClose to popoverDidClose.
  • The checklist section clears the detach flag only after the reattach settles (next main-queue turn) with the popover still shown and not closing. A detach-induced close keeps the flag, takes the existing deferred path, and re-presents without writing back.
  • A popover that really survives the reparent still clears the flag, so a later click-away remains a real dismissal (the test: repair the app-host suites that fail only on macOS 26 #13988 follow-up case).

Validation, focused cmuxTests/SidebarWorkspaceRowSuspensionTests on the owned-mini lane (glaeda-std-xcode-26.6):

Not run locally (this Mac is under the fork-CI-only rule). The PR's routed CI doesn't select this suite, since the diff doesn't touch cmuxTests/.

🤖 Generated with Claude Code

…reattach

On the owned Mac minis (macOS 26.5.1, real display) the checklist
popover that a sidebar row reparent closes is still animating closed when
the row is back in its window. viewDidMoveToWindow saw isShown == true,
decided the popover had survived, and cleared the detach flag; the close
then completed as an external dismissal and wrote presented=false.

The presenter now reports a close in progress (popoverWillClose until
popoverDidClose), and the section clears the detach flag only after the
reattach settles with the popover still shown and not closing. A popover
that truly survives the reparent still treats a later click-away as a
real dismissal.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 18 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: a86c2c1a-01a1-4740-9c8a-14ce5a0fe3fd

📥 Commits

Reviewing files that changed from the base of the PR and between 9bae42b and 67ac63c.

📒 Files selected for processing (2)
  • Sources/Sidebar/AppKitList/Cells/SidebarRowSwiftUIPopoverPresenter.swift
  • Sources/Sidebar/AppKitList/Cells/SidebarWorkspaceRowChecklistSection.swift

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@teamleaderleo
teamleaderleo marked this pull request as ready for review September 26, 2026 14:18
@teamleaderleo
teamleaderleo merged commit d90b0c8 into main Sep 26, 2026
70 of 71 checks passed
@teamleaderleo
teamleaderleo deleted the fix/checklist-popover-animated-reparent branch September 26, 2026 16:23
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for 67ac63c556: every check was green at merge (16 verified; 14 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 26, 2026
e7f1c40 Keep the remote daemon's Claude restore preload out of TMPDIR (manaflow-ai#14851)
37187d5 perf(codex-wrapper): verify the cmux-cua client path with one stat process (manaflow-ai#14835)
680fea3 Pace unfocused terminal surfaces to about 30 FPS (manaflow-ai#14843)
d90b0c8 fix: keep the checklist popover when its detach close finishes after reattach (manaflow-ai#14830)
db5103d perf: skip no-op UserDefaults writes on every session autosave (manaflow-ai#14822)
788fe48 Route palette copy mode visibility and focus restore through the focused Dock (manaflow-ai#14848)
edf54b1 Changelog: Unreleased entries for today's contributor merges; keep Unreleased current (manaflow-ai#14849)

# Conflicts:
#	.github/workflows/build-ghosttykit.yml
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