Skip to content

refactor(tabs): give the editor tab strip's pointer input a single AppKit owner - #2571

Merged
datlechin merged 2 commits into
mainfrom
fix/editor-tab-drag-reorder
Aug 29, 2026
Merged

refactor(tabs): give the editor tab strip's pointer input a single AppKit owner#2571
datlechin merged 2 commits into
mainfrom
fix/editor-tab-drag-reorder

Conversation

@datlechin

Copy link
Copy Markdown
Member

Reordering editor tabs by drag worked by winning an argument. A press on a tab was claimed by a SwiftUI Button, by the enclosing ScrollView, by a simultaneousGesture(DragGesture), and by AppKit's own titlebar window drag, because the strip lives in an NSTitlebarAccessoryViewController. Which one got it depended on timing. This gives the press one owner.

Fixes #2438.

What the report turned out to be

The reported symptom, the whole window travelling with the pointer, is already fixed. The video in the issue is 0.68.x, where reordering was an .onDrag drag-and-drop session; #2472 replaced that with a DragGesture in 0.69.0.

Measured at 0.69.0 on a real display, 13 drags across the strip, sidebar shown and hidden, tab centres from 131pt to 1392pt from the window's leading edge, holds of 0.02s and 0.6s: the window never moved once. That includes the 135pt position #2472 named as the unfixed half of this issue.

The second report, losing the ability to "detach them to create separate windows", is native window tabs. Those went with the single-window model in 0.65.0 (#2097), not with the double-click change that comment suspected.

What is still wrong is narrower and worse:

  • Two of those thirteen drags did nothing. A plain one-place drag, no window movement, no reorder.
  • EditorTabReorderUITests has never passed CI. All four cases are quarantined, and the quarantine note already names the strip's titlebar home as the suspect.
  • A drag released on a neighbour's exact centre was a coin flip. hasCrossed compared a float for equality against a midpoint that arrives from a geometry conversion, so it is 609.99998 as often as it is 610.
  • No autoscroll. Past the track's edge the tab kept moving in the model with nothing on screen to show it, so on an overflowing strip, the case this issue was filed about ("tabs opened earlier stay on the far left"), the user was dragging blind.

Two candidate fixes, measured and refuted

Both are recorded so nobody spends the afternoon on them again.

mouseDownCanMoveWindow = false on an ancestor changes nothing on its own. A standalone harness replicating the strip's exact mounting reports canMove=true on every view from the SwiftUI hit view up through NSTitlebarAccessoryContainerView, NSTitlebarView and NSThemeFrame, and the window still moved 0.0pt across a 200pt drag at every hold from 0ms to 600ms. NSHostingView consumes the mouse-down, so the theme frame never gets the chance the flag would deny it.

An NSPanGestureRecognizer on the hosting view is worse than what shipped. Attached to the strip's NSHostingView with delaysPrimaryMouseButtonEvents = false, it is deterministically dead wherever SwiftUI has a real control: 0 callbacks over a tab's title text and over its close button, 16 elsewhere, identical across repeated runs. The SwiftUI DragGesture it would replace fires everywhere.

The change

EditorTabInteractionView is the strip's superview and the only thing that receives a press over the track. It is a parent rather than a view laid over the tabs on purpose: a view mounted over the track covers every tab in the accessibility tree however little it draws, which is why the Escape monitor it replaces had to be a background. A parent adds no sibling and shadows nothing.

  • mouseDownCanMoveWindow is false, so AppKit's window drag can never take a gesture that started on a tab. The band around the track keeps the default, so the empty chrome either side still moves the window the way Finder's tab bar does.
  • hitTest claims the track and nothing else. The new-tab button stays an ordinary SwiftUI button.
  • Click, double-click, close, reorder and tear-off are told apart inside one nextEvent(matching:) tracking loop, with NSEvent periodic events mixed in so autoscroll keeps running while the pointer is held still at the edge. That is the shape AppKit controls have always used.
  • Escape cancels; the rest of the gesture is swallowed so the release cannot start a fresh one. Other key events are forwarded with NSApp.sendEvent, so Cmd+W and Cmd+T still work mid-drag, which the reorder state already handled.

EditorTabRunLayout is now the single source of the strip's geometry. The tab a user sees, the tab the pointer hits and the tab a drag targets are one rectangle, so they cannot disagree. SwiftUI keeps the drawing, the accessibility tree, and the keyboard and VoiceOver routes to every command.

Multiple rows are the issue's own alternative, under Settings > General > Tabs > When tabs stop fitting. Scrolling stays the default because that is what every macOS tab bar does. A wrapped run never overflows, so no tab is off screen, and the titlebar accessory grows with it through preferredContentSize so the content below is laid out around the taller band. A wrapped track stops being a capsule: at that height the radius would curve away most of the first row's close target while the pointer still hit-tests the full rectangle.

The reorder resolves a wrapped run by projecting the point onto the run it would have been unwrapped, so the midpoint rule stays one rule rather than growing a second one for rows.

Not in this PR

Detaching a tab into its own window. Since 0.65.0 a connection's session lives in exactly one window (ConnectionWorkspace, activeSessions, keyed by connection), so tearing one tab out means one session visible in two windows. That is a change to the model whose invariants CLAUDE.md records as having shipped the same bug four times, and stacking it on an unshipped rewrite of the strip would make both unreviewable. The command set and the tear-off gesture are in place behind canTearOff, which answers false; the follow-up branches off this one.

Verified

  • build PASS.
  • test PASS, 87 cases: the new EditorTabRunLayoutTests and EditorTabStripInteractionTests, the new crossing-tolerance cases, and all six existing EditorTabStrip* suites including the rasterisation ones, which still match pixel for pixel.
  • SwiftLint clean over TablePro, TableProTests and TableProUITests. The violations the run reports are all in TableProTests/Core/Vim/VimEngineVisualReselectionTests.swift, untouched here and unlinted at HEAD because .swiftlint.yml scopes to TablePro.
  • docs/scripts/check-writing-style.sh and docs/scripts/check-docs-against-source.py both clean.
  • Reviewed by Codex, which raised seven findings, three of them P1. All seven are fixed: the drop now resolves from the mouse-up location rather than the last drag sample; the overflow preference is observed so open strips follow it immediately; the row count is published from the interaction so a tab opened or closed across a wrap boundary resizes the band; non-Escape keys are forwarded instead of swallowed; hover, the close target and the tooltip are recomputed after a scroll; the tooltip keeps the label resolver's container context; and a wrapped track uses a fixed corner radius.

What could not be verified, and why

TableProUITests did not run. Every case in EditorTabReorderUITests and EditorTabKeepOpenUITests fails at launch with "The sample database never finished opening". That is not this change: a worktree at clean main fails identically, and the app launched by hand from the same build copies the sample, then sits idle at 0% CPU with no window. Five consecutive launches in fresh sandboxes reproduced it.

So the new interaction has unit coverage of its model and no end-to-end coverage of the gesture. Two guards are in the branch and should be run by CI or on a healthy machine before this merges:

The four previously quarantined cases are deliberately left in .github/macos-ui-test-quarantine.txt for now: taking them off that list is a claim about CI that this branch has no way to check.

No screenshots. The strip is pixel-identical at rest in its default mode, and the wrapped mode could not be captured because the app would not open a window on this machine.

https://claude.ai/code/session_01L7uaHbJBPV1LaWL5QXzxyp

@mintlify

mintlify Bot commented Aug 29, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
TablePro 🟢 Ready View Preview Aug 29, 2026, 7:32 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
@datlechin
datlechin merged commit f48d202 into main Aug 29, 2026
9 checks passed
@datlechin
datlechin deleted the fix/editor-tab-drag-reorder branch August 29, 2026 07:50
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.

Support dragging tabs to reorder them, or display tabs in multiple rows

1 participant