Skip to content

Restore profile menu opening animation - #13295

Merged
austinywang merged 9 commits into
mainfrom
13294-profile-menu-animation
Sep 21, 2026
Merged

austinywang merged 9 commits into
mainfrom
13294-profile-menu-animation

Conversation

@austinywang

@austinywang austinywang commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes #13294

Restore AppKit's native opening transition for the profile/account menu. #13051 disabled animation for grouped popovers to avoid empty parent windows during nested dismissal; that also removed the root menu's opening transition. Its author and merger were @austinywang (2026-09-20 UTC; source commit 9c1858a44e, merged as 57939a8197).

Opening and dismissal now have separate policies: the root opens with the native transition, hover submenus remain immediate, and grouped closes disable animation before tearing down children and parents. Each presentation samples Reduce Motion. Closing popovers are retained by object identity until their delayed callbacks arrive, so rapid close–reopen–close cycles cannot drop an earlier window or let stale callbacks dismiss the current menu. Representable teardown closes without synchronously publishing binding state; overlapping delayed closes are retained independently.

Impact map:

Surface Effect
Profile/account Native opening restored; application-defined group dismissal retained
Team picker Immediate hover submenu opening/closing and existing shared dismissal retained
Help menu Existing semitransient animation retained, now explicitly honoring Reduce Motion; stale callbacks ignored
Other popover presenters Unchanged

Testing

  • Test-only commit adds runtime policy coverage before the fix. Its focused dispatch was canceled after the headless XCTest harness stalled; the required final CI suite independently compiled and passed the wired tests.
  • Final required CI for HEAD f10ca3664e: CI run is green, including macOS compile admission and the tests job. Workflow guards, source lints, web validation, security checks, CLA, Greptile and CodeRabbit are green or intentionally skipped.
  • Final autoreview command: /Users/austinwang/manaflow/cmuxterm-hq/skills/autoreview/scripts/autoreview --mode branch --base origin/main --stream-engine-output — clean; merge-conflict gate clean, no accepted/actionable findings, and cmux policy check clean. The review loop fixed and resolved the production test-seam finding, the implicit MainActor helper finding, the delayed close callback lifecycle finding, and the overlapping-close retention finding.
  • Local checks passed: python3 scripts/swift_file_length_budget.py, ./scripts/lint-pbxproj-test-wiring.sh, git diff --check, ./scripts/localize-changes, and python3 scripts/localization_catalog.py check (six catalogs, nine locales). No user-facing copy or budget TSV changed.

Demo Video

Pending. The user explicitly deferred developer builds until Austin requests tag issue-13294-profile-menu-animation; no developer build was submitted. First/repeated opening smoothness, keyboard activation, Escape, outside click, nested team selection, menu actions/placement and Reduce Motion off/on still need an isolated UI recording. AppKit documents animates as a hint, so automated policy/lifecycle tests and static screenshots do not prove the visible transition. The documented controller recipes do not provide a general GUI-verification lane; legacy maclease provisioning is retired.

Checklist

  • Added focused runtime regression coverage in the already-wired Swift Testing suite
  • Local non-build checks and localization audit passed
  • Required CI green and final structured autoreview clean
  • Review findings addressed or resolved
  • Tagged build and isolated visual verification after Austin's build request
  • User dogfood approval before merge

@github-actions

Copy link
Copy Markdown
Contributor

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

@coderabbitai

coderabbitai Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 8256bd2a-29dc-46d3-9aaa-3b7b5fc36e80

📥 Commits

Reviewing files that changed from the base of the PR and between 0311980 and 456d555.

📒 Files selected for processing (1)
  • cmuxTests/CmuxPopoverGroupTests.swift

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Popover presentation now uses Reduce Motion and submenu state. Grouped popovers close without animation, and stale close notifications are ignored. Tests cover the updated animation and dismissal behavior.

Changes

Popover animation and dismissal

Layer / File(s) Summary
Animation policy and coordinator handling
Packages/macOS/CmuxAppKitSupportUI/Sources/CmuxAppKitSupportUI/Popover/CmuxPopoverAnimationPolicy.swift, Packages/macOS/CmuxAppKitSupportUI/Sources/CmuxAppKitSupportUI/Popover/ArrowlessPopoverAnchor.swift
Presentation animation is disabled when Reduce Motion is enabled or the popover is a submenu. Grouped dismissal disables animation. Close notifications from unrelated popovers are ignored.
Grouped popover parent and close handling
Packages/macOS/CmuxAppKitSupportUI/Sources/CmuxAppKitSupportUI/Popover/CmuxPopoverGroup.swift
Parent lookup uses parentID(for:). Child popovers disable animation before closing.
Animation and dismissal regression coverage
cmuxTests/CmuxPopoverGroupTests.swift
Tests cover root and submenu presentation, Reduce Motion, and grouped dismissal.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant ArrowlessPopoverAnchorCoordinator
  participant CmuxPopoverGroup
  participant NSPopover
  User->>ArrowlessPopoverAnchorCoordinator: open menu
  ArrowlessPopoverAnchorCoordinator->>CmuxPopoverGroup: check anchor parent
  CmuxPopoverGroup-->>ArrowlessPopoverAnchorCoordinator: return parent ID or nil
  ArrowlessPopoverAnchorCoordinator->>NSPopover: configure animation policy
  ArrowlessPopoverAnchorCoordinator->>NSPopover: present menu
  User->>NSPopover: dismiss menu
  ArrowlessPopoverAnchorCoordinator->>NSPopover: disable grouped close animation
  NSPopover-->>ArrowlessPopoverAnchorCoordinator: close notification
  ArrowlessPopoverAnchorCoordinator->>ArrowlessPopoverAnchorCoordinator: update state for current popover only
Loading
🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Linked Issues check ✅ Passed For #13294, the PR restores native root-popover opening animation through CmuxPopoverAnimationPolicy. The policy disables animation for submenus and for macOS Reduce Motion. Grouped dismissal disabl…
Out of Scope Changes check ✅ Passed The changes stay within #13294. The implementation changes the shared popover animation and dismissal lifecycle needed to restore the profile menu transition. The tests cover the required regression c…
Cmux Cloud Persistent Session And Early Input ✅ Passed PASS: The pull request changes only AppKit popover presentation, grouping, and related tests. The authoritative diff contains no Cloud terminal creation, cmux-tui transport, manual renderer, PTY readi…
Cmux Swift Actor Isolation ✅ Passed PASS. The production changes keep UI state and AppKit lifecycle code on explicit @MainActor owners: ArrowlessPopoverAnchor.Coordinator is @MainActor, and CmuxPopoverGroup remains @MainActor.…
Cmux Swift Blocking Runtime ✅ Passed The authoritative diff adds no semaphores, blocking waits, sleeps, delayed dispatch, polling, main-queue sync, timers, or manual locks. Production changes only set NSPopover.animates, compute `CmuxP…
Cmux Browser Automation Off-Main ✅ Passed PASS — The PR changes only popover presentation and test files. The rule’s browser automation targets, Sources/TerminalController.swift and ControlCommandExecutionPolicy.swift, are unchanged. The …
Cmux Expensive Synchronous Load ✅ Passed PASS. The authoritative diff changes only popover animation, grouping, lifecycle notification guards, and tests. It adds no agent-history/session-store loader, transcript or trajectory read, JSON/JSON…
Cmux Cache Substitution Correctness ✅ Passed PASS: The PR changes only transient AppKit popover presentation and dismissal behavior, plus focused tests. The diff does not replace a fresh authoritative read in a persistence, history, undo, or sna…
Cmux No Hacky Sleeps ✅ Passed PASS. The authoritative pull-request diff changes only four Swift files: three production Swift files and one Swift test file. The rule explicitly scopes TypeScript, JavaScript, shell, and non-Swift b…
Cmux Algorithmic Complexity ✅ Passed PASS. The production changes do not introduce a prohibited scalable-data algorithm. CmuxPopoverGroup.parentID(for:) performs one linear members.last lookup with an O(1)-average dictionary access, …
Cmux Swift Concurrency ✅ Passed PASS: The pull request does not introduce or materially expand any prohibited legacy concurrency pattern. The authoritative diff adds no Task, DispatchQueue, DispatchGroup, Combine state, comple…
Cmux Swift @Concurrent ✅ Passed PASS: The PR adds no async, await, nonisolated, Task, or @concurrent declarations or call sites. The new CmuxPopoverAnimationPolicy is a synchronous pure helper. The changed popover and gr…
Cmux Swift Package Boundaries ✅ Passed The production diff stays in the existing Packages/macOS/CmuxAppKitSupportUI SwiftPM target. ArrowlessPopoverAnchor and CmuxPopoverGroup are AppKit/SwiftUI popover glue, which the rule allows. T…
Cmux Swiftpm Lockfiles ✅ Passed PASS. The authoritative PR diff contains only three Swift source changes and one test file. It changes no Package.swift, Package.resolved, .gitignore, workflow, Xcode project, or package-referen…
Cmux Swift Logging ✅ Passed PASS. The authoritative Swift diff adds popover animation and lifecycle logic plus tests, but it adds no print, debugPrint, dump, NSLog, file/stdout logging, Logger, or sensitive-data loggin…
Cmux User-Facing Error Privacy ✅ Passed PASS: The PR changes popover presentation, dismissal, lifecycle guards, and tests only. The added production code contains no user-facing error, alert, command output, API error body, recovery copy, o…
Cmux Full Internationalization ✅ Passed PASS. The authoritative PR diff changes only popover lifecycle/animation logic and tests. It adds no user-facing Swift text, web copy, metadata, changelog, localization key, or string catalog entry. T…
Cmux Swiftui State Layout ✅ Passed PASS. The diff does not introduce any prohibited SwiftUI state or layout pattern. It changes AppKit popover lifecycle code in an existing NSViewRepresentable bridge, adds a plain `CmuxPopoverAnimati…
Cmux Architecture Rethink ✅ Passed The diff is a small lifecycle correctness fix with clear owners. CmuxPopoverAnimationPolicy owns presentation animation decisions. CmuxPopoverGroup owns grouped child-before-parent dismissal. `Arr…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR changes only NSPopover presentation and grouped popover lifecycle code, plus a popover animation policy and tests. The custom rule explicitly allows popovers, menus, and views that are not st…
Cmux Source Artifacts ✅ Passed PASS. The authoritative diff changes only four ordinary Swift paths: three hand-written files under the product source tree and one focused test file under cmuxTests/. The patch adds animation polic…
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS. The production diff adds no #if DEBUG or test-build guard, no debug/test-named member, and no wrapper accessor. popover changes from private to private(set) only; the PR does not add a t…
Title check ✅ Passed The title clearly and concisely describes the primary change: restoring the profile menu opening animation.
Description check ✅ Passed The description explains the problem, implementation, affected surfaces, testing, pending visual verification, and checklist status. It clearly documents that the demo video and user dogfood approval …
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

@greptile-apps

greptile-apps Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge with no outstanding actionable findings.

Summary

Restores the native opening transition for the root profile/account popover while preserving immediate, child-first grouped dismissal.

  • Explicitly enables the root account popover’s presentation animation.
  • Disables animation before closing each grouped popover.
  • Continues honoring Reduce Motion at presentation time.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Open profile menu] --> B[Sample Reduce Motion]
    B -->|Reduce Motion off| C[Show root with native animation]
    B -->|Reduce Motion on| D[Show root without animation]
    C --> E[Grouped dismissal]
    D --> E
    E --> F[Disable child animation and close children]
    F --> G[Disable parent animation and close parent]
Loading

Reviews (14) · Last reviewed commit: "Merge branch 'main' into 13294-profile-m..."

@austinywang
austinywang force-pushed the 13294-profile-menu-animation branch 4 times, most recently from 0311980 to 456d555 Compare September 21, 2026 02:13
@austinywang
austinywang force-pushed the 13294-profile-menu-animation branch from 456d555 to 9aa1273 Compare September 21, 2026 02:27
@austinywang
austinywang force-pushed the 13294-profile-menu-animation branch from 9aa1273 to a098fef Compare September 21, 2026 02:57
@austinywang austinywang mentioned this pull request Sep 21, 2026
2 of 6 tasks
@cursor

cursor Bot commented Sep 21, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

austinywang and others added 5 commits September 20, 2026 21:03
Resolve conflicts with #13311 (team picker opening animation). Adopt main's
explicit CmuxPopoverPresentationAnimation policy instead of the coordinator's
submenu detection, keep main's popover lifecycle handling, and opt the root
account menu into `.enabled` so the profile menu keeps its native opening
transition (#13294).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@austinywang
austinywang merged commit 792ec1f into main Sep 21, 2026
38 checks passed
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.

Restore the profile menu opening animation

1 participant