Repository navigation
Fix floating tab drag ownership and fallback - #192
lawrencecchen wants to merge 15 commits into
Conversation
📝 WalkthroughWalkthroughManual reorder tracking now installs only when tab reordering is allowed and the tab bar is in minimal mode. Unit tests cover enabled and disabled policy combinations. ChangesManual reorder policy
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR narrows the AppKit manual tab-reorder tracker so it only fires in minimal mode, leaving standard tab bars with a single drag owner (the SwiftUI item-provider). The change is small, well-motivated, and covered by a targeted unit test.
Confidence Score: 5/5Safe to merge — the change is a targeted, well-tested narrowing of the AppKit drag tracker to minimal mode only. The fix is a single boolean predicate change with a clear, accurate doc comment explaining why minimal mode needs the AppKit path while standard tab bars do not. The unit test directly verifies the three meaningful input combinations, and 202 passing tests provide broader regression confidence. No data flow changes, no new async paths, and no API surface alterations. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Tab Bar Mouse Event] --> B{isMinimalMode?}
B -- Yes --> C{allowTabReordering?}
B -- No --> D[SwiftUI Item-Provider Drag\nSole owner]
C -- Yes --> E[AppKit Manual Reorder Tracker\n+ SwiftUI Item-Provider Drag]
C -- No --> F[No drag handling]
Reviews (1): Last reviewed commit: "Give standard tab drags one event owner" | Re-trigger Greptile |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3bc2c9dac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| TabBarManualReorderPolicy.shouldApplyFallback( | ||
| sourceTabId: session.sourceTab.id, | ||
| activeDragTabId: splitViewController?.activeDragTab?.id, | ||
| draggingTabId: splitViewController?.draggingTab?.id | ||
| ), |
There was a problem hiding this comment.
If an item-provider drag for this tab is canceled without reaching the normal drop clear path, the controller can retain activeDragTab/draggingTab for that tab; this guard then treats that stale state as ownership of every later manual fallback gesture for the same tab. Because the removed defer no longer clears matching controller drag state, the fallback keeps returning before reordering and never recovers until some unrelated drop path clears the stale state. Consider tying the yield to drag state created during this mouse sequence, or clearing stale matching state before yielding.
Useful? React with 👍 / 👎.
lawrencecchen
changed the title
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 similar comment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 participant
Standard tab bars use the SwiftUI item-provider drag as their primary owner. Hosts that need AppKit tracking in custom window chrome can opt into
manualTabReorderFallbackEnabled.The fallback observes the same mouse sequence, yields when the matching item-provider drag starts, and defers recovery after cancellation. Recovery mutates only when the source pane and its exact tab order are unchanged, preventing a successful SwiftUI drop from being applied twice. cmux Floating Docks enable this host-scoped fallback through manaflow-ai/cmux#8351.
Empty tab chrome uses one screen-space window-drag session in standard and minimal modes. The actual AppKit drag-zone view accepts the first mouse event, so an inactive child window starts moving on its first gesture while tab hit regions retain tab-drag ownership.
Regression history is preserved as test-only commits followed by fixes for event ownership, host-scoped fallback behavior, cancelled item-provider recovery, and inactive-window first-mouse routing. The focused inactive-window regression passes; the prior full suite passed 204 tests.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by CodeRabbit
Bug Fixes
Tests
Summary by cubic
Standard tab bars now use SwiftUI item‑provider drags as the single owner with a safe AppKit fallback. Empty tab chrome uses a screen‑space window‑drag session across modes and accepts the first click in inactive windows so the initial drag always works; the leading inset now draws a separator at the window‑chrome boundary.
New Features
manualTabReorderFallbackEnabledtoBonsplitConfigurationto opt into the fallback outside minimal mode.BonsplitWindowDragSessionand unified empty tab‑bar window dragging across standard and minimal modes.Bug Fixes
TabBarManualReorderPolicy, including yield‑to‑item‑provider and a deferred fallback that applies only when the source pane is unchanged.performDrag(with:)withBonsplitWindowDragSessionfor empty tab chrome in all modes, fixing child‑window dragging and standard‑mode empty‑chrome dragging, and preserving the first drag on inactive windows by accepting the first mouse.Written for commit f334bf0. Summary will update on new commits.