Skip to content

refactor(tabs): reorder editor tabs by direct manipulation instead of a drag session - #2472

Merged
datlechin merged 1 commit into
mainfrom
fix/tab-strip-drag-moves-window
Aug 26, 2026
Merged

refactor(tabs): reorder editor tabs by direct manipulation instead of a drag session#2472
datlechin merged 1 commit into
mainfrom
fix/tab-strip-drag-moves-window

Conversation

@datlechin

Copy link
Copy Markdown
Member

Reordering editor tabs was built as a SwiftUI drag-and-drop session: .onDrag handing out an NSItemProvider, a per-tab DropDelegate calling QueryTabManager.moveTab from dropEntered, and a second delegate over the track to reset the state. That shape cannot express a cancel, because .onDrag reports none, and it produced four separate defects. This replaces it with direct manipulation, which is what every system tab bar does.

Found while investigating #2438. It does not close that issue: see "What this does not fix" below.

What was wrong

  • A cancelled drag was not cancelled. dropEntered committed moveTab as the pointer crossed each neighbour, and nothing recorded the pre-drag order, so there was no way back. Every one of those writes bumped tabStructureVersion, which routes to saveAggregated() and a TabDiskActor write, so a drag the user abandoned was already saved to disk and survived a relaunch. No undo.
  • A couple of points of hand drift ended the reorder. The reset delegate sat on the 28pt track while tabs are 24pt inside 2pt of padding, so dropExited, which fires on a pointer exit rather than on a drag ending, cleared the drag state mid-gesture and the rest of that drag silently did nothing.
  • The strip accepted any dragged text and threw it away. EditorTabStripDropReset had no validateDrop, so the protocol default accepted every public.text drop, dropUpdated returned .move, and performDrop returned true after doing nothing. Dragging SQL onto the strip reported success to the source and discarded it.
  • A cancelled drag left the strip visibly wrong. The dragged tab stayed at 45% opacity and every separator stayed hidden until the pointer next hovered a tab, which keyboard-only work never does.

The change

EditorTabReorder holds the order the strip draws and the order it came from, so the manager is written once, on release. A cancel is then just dropping that value, and nothing reaches the persisted tab list until the user has finished asking for it.

EditorTabReorderResolver decides where the dragged tab belongs. It swaps on a neighbour's midpoint rather than on contact, which is what stops a pair flipping against each other while the pointer sits on a boundary, and it answers with the furthest midpoint actually crossed, because macOS coalesces a fast drag and one event can arrive several tabs along.

The gesture is a DragGesture in a coordinate space named on the scroll view's content, so a position is measured against the whole run of tabs rather than the part on screen; named on the viewport it would place the drag one tab further along for every tab the track had scrolled past. It is attached with simultaneousGesture so the tab's own button keeps the click that selects it.

Cancelling is latched until the gesture ends. reorder == nil could not carry it, because that means both "not started" and "cancelled": the next onChanged of an abandoned gesture rebuilt a reorder from the manager and the release committed it, so Escape read as working and reordered the strip anyway. EditorTabReorderCancelMonitor owns the Escape monitor, because a View is a value with no deinit and every other local monitor in this app is owned by a reference type.

The drag also ends when the tab being dragged is closed under it, when a tab is opened beside it, and when the pane is unparented by a connection switch.

EditorTabDropDelegate, EditorTabStripDropReset, .onDrag and the UniformTypeIdentifiers import are gone.

What this does not fix

Pressing a tab inside the leading region of the titlebar still drags the window rather than the tab. That is the other half of #2438 and it is not addressed here.

It is positional, not a property of the tab: measured in the app, a tab whose centre sat 135pt from the window's leading edge dragged the window, and in a standalone harness the same undecorated tab reordered normally once the strip was inset 200pt. Two candidate fixes were built and then refuted by measurement, so neither is in this branch: giving every tab an always-present fill changed nothing, and mouseDownCanMoveWindow = false over the track changed nothing in either placement, because AppKit consults the hit-tested view and its ancestors and a sibling background or overlay is neither.

What is missing is the extent of the reserved region. The standalone harness contradicts itself run to run, so the number has to come from the app, and the UI harness on this machine began failing every case at launch with "The sample database never finished opening", including cases that had passed minutes earlier. That is an environment failure rather than a change failure, so the constant was not guessed at.

The UI tests here therefore assert the reorder and deliberately do not assert the window's origin.

Verified

  • build PASS.
  • test PASS, 63 cases: the new EditorTabReorderTests and EditorTabReorderResolverTests, plus QueryTabReorderTests and all five existing EditorTabStrip* suites.
  • uitest EditorTabReorderUITests passed its reorder cases on the last healthy run of the harness; the harness later degraded as described above, so the final run of this suite is INCONCLUSIVE rather than green.
  • SwiftLint clean over the app target, and over the test targets under an adjusted included: scope.
  • docs/scripts/check-writing-style.sh and docs/scripts/check-docs-against-source.py both clean.
  • Reviewed by Codex. It raised three points: the cancellation latch and the coalesced-drag midpoint are both fixed above, with tests. Its third, that this removes edge autoscrolling for an overflowing strip, is dismissed: the drag-and-drop session it replaces never auto-scrolled either, since a SwiftUI drop does not drive a ScrollView, so that gap is pre-existing rather than a regression.

No screenshots: the change is behavioural and the strip looks identical at rest, so a still cannot show the difference.

@mintlify

mintlify Bot commented Aug 26, 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 26, 2026, 5:04 PM

💡 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.

@datlechin
datlechin merged commit 85dd888 into main Aug 26, 2026
1 check passed
@datlechin
datlechin deleted the fix/tab-strip-drag-moves-window branch August 26, 2026 17:07
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