Repository navigation
Chrome-style live sidebar reorder: rows animate on mousemove, groups drag as blocks - #8467
lawrencecchen wants to merge 14 commits into
Conversation
Drags that begin in the sidebar table now animate rows apart around a moving slot while a floating snapshot of the dragged row tracks the pointer; group headers are draggable and move their whole block. The system drag session stays alive underneath (image hidden while over the source sidebar), so cross-window drops and bonsplit interop keep the existing indicator path. The model commits once on drop from the same resolved plan the preview showed; escape or dropping outside restores the original order.
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds live workspace drag-reorder previews for the AppKit sidebar, including grouped permutation logic, validated table move planning, floating drag visuals, deferred applies, and controller/action wiring for resolving and committing reorder plans. ChangesWorkspace reorder preview
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant NSTableView
participant SidebarWorkspaceTableLocalReorderController
participant SidebarWorkspaceTableController
participant SidebarWorkspaceTableActions
NSTableView->>SidebarWorkspaceTableLocalReorderController: Start workspace drag
SidebarWorkspaceTableLocalReorderController->>SidebarWorkspaceTableController: Resolve preview plan
SidebarWorkspaceTableController->>SidebarWorkspaceTableActions: resolveWorkspaceReorderPlan
SidebarWorkspaceTableActions-->>SidebarWorkspaceTableController: Return reorder plan
SidebarWorkspaceTableController->>NSTableView: Animate preview order and hide moving rows
SidebarWorkspaceTableLocalReorderController->>SidebarWorkspaceTableActions: commitWorkspaceReorderPlan
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning, 1 inconclusive)
✅ Passed checks (21 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 |
|
@codex review |
![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: 86cd5cff61
ℹ️ 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".
| current.remove(at: fromIndex) | ||
| current.insert(ids[targetIndex], at: targetIndex) | ||
| } | ||
| table.endUpdates() |
There was a problem hiding this comment.
After a preview permutation changes rows and calls moveRow, reorderDropView.targets still contains the old workspace/frame mapping because the active geometry refresh path is not called here. In a local drag such as moving A below C and then moving the pointer back over the top row, the next draggingUpdated resolves against stale targets (the top frame still names A), so the preview/lastPlan can stick to or commit the previous slot instead of the row currently under the pointer. Please refresh active drop targets after the preview order changes.
Useful? React with 👍 / 👎.
| guard case .group(let groupId) = scope else { return nil } | ||
| return groupId |
There was a problem hiding this comment.
indicatorScope describes where to render the indicator, not necessarily whether the workspace will join that group: the resolver has root-lane group-boundary plans that render with .group(...) while committing usesTopLevelRows: true with explicitGroupId == nil. Returning the scope's group here makes the floating row indent as if it will become a group member even though the drop commits a top-level move, so root drops around expanded/collapsed groups preview the wrong hierarchy. Please derive this from the reorder action's explicit destination group instead of the render scope.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/Sidebar/AppKitList/SidebarWorkspaceTableController.swift`:
- Around line 1088-1094: The reorder loop in localReorderApplyOrder performs
repeated linear searches and array removal/insertion, causing quadratic work
during live preview. Replace it with an indexed position map or equivalent
one-pass move plan that tracks each identifier’s current index while applying
moves, preserving the resulting order and table.moveRow behavior with
near-linear complexity.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1c8e3ec5-65fa-4d4a-a938-7c3d14f86740
📒 Files selected for processing (9)
Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SidebarDrop/SidebarWorkspaceReorderPreviewPermutation.swiftPackages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SidebarWorkspaceReorderPreviewPermutationTests.swiftSources/ContentView.swiftSources/Sidebar/AppKitList/SidebarWorkspaceTableActions.swiftSources/Sidebar/AppKitList/SidebarWorkspaceTableController.swiftSources/Sidebar/AppKitList/SidebarWorkspaceTableLocalReorderSession.swiftSources/SidebarWorkspaceReorderDropView.swiftcmux.xcodeproj/project.pbxprojcmuxTests/SidebarWorkspaceTableTests.swift
Greptile SummaryThis PR replaces the 2px indicator-line drop preview in the AppKit sidebar with a Chrome-style live reorder: a floating snapshot of the dragged row (or whole group block) tracks the pointer while sibling rows animate apart in real time, with a no-commit cancel path on Escape and clean settle/restore animations.
Confidence Score: 5/5Safe to merge; the new live-reorder path is well-isolated behind an immutable session snapshot, the model commits exactly once from the previewed plan, and the degrade/restore paths cover structural applies, cross-window handoff, and Escape. The only finding is a missing-locale gap in the new xcstrings entry: sidebar.reorder.floating.moreRows covers en and ja but not the other 19 supported locales. Functionally the fallback default value produces the correct string everywhere, so no user-visible regression exists. Resources/Localizable.xcstrings — the new sidebar.reorder.floating.moreRows key needs translations added for the remaining 19 supported locales. Important Files Changed
Sequence DiagramsequenceDiagram
participant TV as SidebarWorkspaceTableViewImpl
participant G as DirectReorderGesture
participant TC as TableController
participant LR as LocalReorderController
participant CV as ContentView
TV->>G: mouseDown, drag(to:)
G-->>TV: .beginReorder
TV->>TC: directReorderWillBegin(row:at:)
TC->>LR: directSessionWillBegin
LR-->>TC: session started, cells hidden, floating view shown
loop every mousemove
TV->>TC: directReorderMoved(to:)
TC->>LR: directSessionMoved(to:)
LR->>TC: localReorderResolvePlan
TC->>CV: resolveWorkspaceReorderPlan
CV-->>LR: plan
LR->>TC: localReorderApplyOrder (MovePlanner steps)
TC->>TV: table.moveRow (animated)
end
alt mouseUp in corridor
TV->>TC: directReorderEnded(at:)
LR->>TC: localReorderCommit(plan:)
TC->>CV: commitWorkspaceReorderPlan
LR->>LR: settleFloatingView, finish
else Escape or outside corridor
TV->>TC: directReorderCancelled
LR->>TC: localReorderApplyOrder(originalOrder)
LR->>LR: cancelAndRestore, finish
else exits corridor, system drag handoff
G-->>TV: .handoffToSystemDrag
TC->>LR: directSessionHandedOffToSystemDrag
TV->>TV: beginSystemDragHandoff
end
Reviews (7): Last reviewed commit: "Stabilize sidebar reorder XCU fixture" | Re-trigger Greptile |
…ed indent - Codex P1: refresh the overlay's drop targets after a preview permutation moves rows, so the next draggingUpdated resolves against current row ids and frames instead of sticking to (and committing) the previous slot. - Codex P2: derive the floating row's destination-group indent from the plan's reorder action (explicitGroupId) instead of the indicator render scope; root-lane group-boundary plans render with a group scope while committing a top-level move. - CodeRabbit/Greptile: replace the greedy per-index firstIndex/remove/insert loop in localReorderApplyOrder (quadratic per drag update, one moveRow per displaced row) with SidebarWorkspaceReorderMovePlanner, a pure CmuxFoundation planner that emits one moveRow per dragged-block row and refuses any order that is not an id-preserving block permutation. Covered by package tests including full slot sweeps for single rows and group blocks. - Greptile/CodeRabbit i18n: route the floating-row "+N" badge through String(localized:defaultValue:).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14d46d9698
ℹ️ 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".
| guard let plan = delegate.localReorderResolvePlan(point: point, targets: targets) else { | ||
| // Dead zone: keep the last valid preview rather than snapping. | ||
| return |
There was a problem hiding this comment.
Fresh evidence after the target-refresh fix: this nil path still preserves the last preview/lastPlan, but localReorderResolvePlan also returns nil for valid no-op slots because SidebarDropPlanner suppresses no-op indicators. For example, drag A below C, then move the pointer back to the original slot above B and release; the resolver returns nil, this guard keeps the A-below-C preview, and handlePerformDrop commits that stale plan instead of restoring/doing nothing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Sources/Sidebar/AppKitList/SidebarWorkspaceTableLocalReorderSession.swift (2)
325-375: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDuplicate target-rect computation in
settleFloatingViewIntoSlotandcancelAndRestore.Both methods independently recompute
blockRowIndexes/targetRectfromcurrent.blockRowIdswith the samerows.indices.filter { ... }+reduce(CGRect.null) { ... }pattern. Extracting a shared helper avoids future drift between the two paths.♻️ Proposed refactor
+ private func targetRect( + for blockRowIds: [SidebarWorkspaceRenderItemID], + in container: SidebarWorkspaceTableContainerView, + rows: [SidebarWorkspaceTableRowConfiguration] + ) -> CGRect? { + let table = container.tableView + let overlay = container.reorderDropView + let blockRowIndexes = rows.indices.filter { blockRowIds.contains(rows[$0].id) } + let rect = blockRowIndexes.reduce(CGRect.null) { partial, row in + partial.union(table.convert(table.rect(ofRow: row), to: overlay)) + } + return rect.isNull ? nil : rect + } + private func settleFloatingViewIntoSlot() { guard let delegate, let container = delegate.localReorderContainerView, let current = session else { finish(flushDeferredApply: true) return } - let rows = delegate.localReorderRows - let table = container.tableView - let overlay = container.reorderDropView - let blockRowIndexes = rows.indices.filter { current.blockRowIds.contains(rows[$0].id) } - let targetRect = blockRowIndexes.reduce(CGRect.null) { partial, row in - partial.union(table.convert(table.rect(ofRow: row), to: overlay)) - } - guard let floating = current.floatingView, !targetRect.isNull else { + guard let floating = current.floatingView, + let rect = targetRect(for: current.blockRowIds, in: container, rows: delegate.localReorderRows) else { finish(flushDeferredApply: true) return } - animateEndOfSession(floating: floating, to: targetRect) + animateEndOfSession(floating: floating, to: rect) }Apply the same simplification to
cancelAndRestore.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/Sidebar/AppKitList/SidebarWorkspaceTableLocalReorderSession.swift` around lines 325 - 375, Extract the repeated block-row target rectangle calculation from settleFloatingViewIntoSlot and cancelAndRestore into a shared helper on the reorder session type. Have the helper use the current blockRowIds, localReorderRows, table view, and reorderDropView to return the computed CGRect, then replace both inline blockRowIndexes/reduce implementations while preserving the existing null-rectangle handling.
129-137: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore the drag image on exit
hideDragImage(_:)setsNSDraggingItem.imageComponentsProviderfor the whole drag session, sodraggingExited()needs to clear or reapply it. Right now the system drag image stays hidden after the pointer leaves this sidebar, which breaks cross-window drags.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/Sidebar/AppKitList/SidebarWorkspaceTableLocalReorderSession.swift` around lines 129 - 137, Update draggingExited(_:) to restore the drag image after leaving the sidebar by clearing or reapplying the NSDraggingItem imageComponentsProvider configured by hideDragImage(_:). Keep the existing session and phase handling intact, and ensure setInsideSidebar(false) still runs for valid active drags.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Sources/Sidebar/AppKitList/SidebarWorkspaceTableLocalReorderSession.swift`:
- Around line 325-375: Extract the repeated block-row target rectangle
calculation from settleFloatingViewIntoSlot and cancelAndRestore into a shared
helper on the reorder session type. Have the helper use the current blockRowIds,
localReorderRows, table view, and reorderDropView to return the computed CGRect,
then replace both inline blockRowIndexes/reduce implementations while preserving
the existing null-rectangle handling.
- Around line 129-137: Update draggingExited(_:) to restore the drag image after
leaving the sidebar by clearing or reapplying the NSDraggingItem
imageComponentsProvider configured by hideDragImage(_:). Keep the existing
session and phase handling intact, and ensure setInsideSidebar(false) still runs
for valid active drags.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 84ade6b3-0fa0-47b0-8d6c-2077d188b3f0
📒 Files selected for processing (6)
Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SidebarDrop/SidebarWorkspaceReorderMovePlan.swiftPackages/macOS/CmuxFoundation/Sources/CmuxFoundation/SidebarDrop/SidebarWorkspaceReorderPreviewPermutation.swiftPackages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SidebarWorkspaceReorderMovePlanTests.swiftPackages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SidebarWorkspaceReorderPreviewPermutationTests.swiftSources/Sidebar/AppKitList/SidebarWorkspaceTableController.swiftSources/Sidebar/AppKitList/SidebarWorkspaceTableLocalReorderSession.swift
…h indent preview - A nil resolver plan (the no-op zone over the dragged block's original slot) now previews the original order and clears the pending plan, so a drag can be returned exactly where it started; dropping there restores instead of committing the last previewed slot. - Plain presses commit the workspace switch at mouseDown (Arc parity): pressing a row to reorder it also activates it. Modifier presses and presses inside a multi-selection keep click-time semantics, and the optimistic press paint now survives the drag instead of rolling back. - The floating snapshot resizes for group indent: content slides right and clips to member width when joining a group, un-indents when a member row previews a top-level slot, and commits keep the indent so the snapshot lands exactly where the member cell renders.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…agged block The planner suppresses no-op plans, and with drop targets refreshed to preview geometry the pointer's resting state is the dragged block's own frame — so every stationary moment resolved nil, restored the original order, moved a neighbor under the pointer, and re-previewed, oscillating rows under the cursor. Nil now keeps the current preview while the pointer is inside the block's preview frame (that gap IS the block), and only restores the original order when the pointer has crossed into the no-op dead zone outside it, which is what makes drag-back-to-origin land exactly.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Sidebar/AppKitList/SidebarWorkspaceTableLocalReorderSession.swift (1)
552-574: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the missing locale entries for
sidebar.reorder.floating.moreRows.Resources/Localizable.xcstringshas the key, but onlyenandjaare filled; the other catalog locales are still missing, so the reorder badge isn’t fully localized.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/Sidebar/AppKitList/SidebarWorkspaceTableLocalReorderSession.swift` around lines 552 - 574, Add translations for the existing sidebar.reorder.floating.moreRows key in Resources/Localizable.xcstrings for every catalog locale currently missing a localized value, preserving the existing en and ja entries and the +count badge format.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Sources/Sidebar/AppKitList/SidebarWorkspaceTableLocalReorderSession.swift`:
- Around line 552-574: Add translations for the existing
sidebar.reorder.floating.moreRows key in Resources/Localizable.xcstrings for
every catalog locale currently missing a localized value, preserving the
existing en and ja entries and the +count badge format.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c19bb86f-0e6b-4a43-8884-a7a9b7199b15
📒 Files selected for processing (3)
Sources/Sidebar/AppKitList/SidebarWorkspaceTableController.swiftSources/Sidebar/AppKitList/SidebarWorkspaceTableLocalReorderSession.swiftSources/Sidebar/AppKitList/SidebarWorkspaceTableViewImpl.swift
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/Sidebar/AppKitList/SidebarWorkspaceTableLocalReorderSession.swift`:
- Around line 339-344: Update the block-rectangle calculation in the
drag/reorder method containing blockRect to avoid nested scans: build a Set from
current.blockRowIds for O(1) membership checks, then iterate rows once while
accumulating matching row rectangles. Preserve the existing table.convert
conversion and return false when no matching rows are found, and exit the scan
early once all block row IDs have been located.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2b0c2205-4dfc-4ba1-9902-d8a3c4b45d86
📒 Files selected for processing (1)
Sources/Sidebar/AppKitList/SidebarWorkspaceTableLocalReorderSession.swift
| let blockRect = rows.indices | ||
| .filter { current.blockRowIds.contains(rows[$0].id) } | ||
| .reduce(CGRect.null) { partial, row in | ||
| partial.union(table.convert(table.rect(ofRow: row), to: overlay)) | ||
| } | ||
| guard !blockRect.isNull else { return false } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Avoid nested array scans on drag update hot paths.
rows.indices.filter { current.blockRowIds.contains(...) } performs a full scan over scalable collections (up to ~1000 rows), and calling contains on an array introduces a nested scan per item.
As per path instructions, avoid repeated full scans over scalable collections in production drag/reorder code paths, and don't call contains/filter/map/sorted across the same backing array for each item. Use a Set for O(1) membership lookups and a single-pass loop with an early exit to bound the algorithmic complexity.
⚡️ Proposed fix using a Set and early exit
- let blockRect = rows.indices
- .filter { current.blockRowIds.contains(rows[$0].id) }
- .reduce(CGRect.null) { partial, row in
- partial.union(table.convert(table.rect(ofRow: row), to: overlay))
- }
- guard !blockRect.isNull else { return false }
+ let blockIds = Set(current.blockRowIds)
+ var blockRect = CGRect.null
+ var foundCount = 0
+ for i in rows.indices {
+ if blockIds.contains(rows[i].id) {
+ blockRect = blockRect.union(table.convert(table.rect(ofRow: i), to: overlay))
+ foundCount += 1
+ if foundCount == blockIds.count { break }
+ }
+ }
+ guard !blockRect.isNull else { return false }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let blockRect = rows.indices | |
| .filter { current.blockRowIds.contains(rows[$0].id) } | |
| .reduce(CGRect.null) { partial, row in | |
| partial.union(table.convert(table.rect(ofRow: row), to: overlay)) | |
| } | |
| guard !blockRect.isNull else { return false } | |
| let blockIds = Set(current.blockRowIds) | |
| var blockRect = CGRect.null | |
| var foundCount = 0 | |
| for i in rows.indices { | |
| if blockIds.contains(rows[i].id) { | |
| blockRect = blockRect.union(table.convert(table.rect(ofRow: i), to: overlay)) | |
| foundCount += 1 | |
| if foundCount == blockIds.count { break } | |
| } | |
| } | |
| guard !blockRect.isNull else { return false } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/Sidebar/AppKitList/SidebarWorkspaceTableLocalReorderSession.swift`
around lines 339 - 344, Update the block-rectangle calculation in the
drag/reorder method containing blockRect to avoid nested scans: build a Set from
current.blockRowIds for O(1) membership checks, then iterate rows once while
accumulating matching row rectangles. Preserve the existing table.convert
conversion and return false when no matching rows are found, and exit the scan
early once all block row IDs have been located.
Source: Path instructions
…ag slop past the sidebar - Slot decisions now key off the floating block's vertical center instead of the raw cursor: a swap fires when the dragged row's middle crosses a neighbor's midpoint, wherever inside the row it was grabbed. - Group/root-ambiguous boundary slots (a group's last-member edge) resolve toward where the drag already previews instead of the pointer-lane heuristic: a drag inside the group stays through the boundary band, a top-level drag stays out, and stickiness releases a full row past the group's last visible row so tail groups can still be exited downward. This makes the last slot of a group reachable without pixel precision. - The reorder drop overlay expands around the sidebar during local drags (drag destination routing is frame-based), so a cursor drifting out of the narrow sidebar keeps the preview alive, hit-tests as if clamped to the row band, and dropping there still commits.
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
…order # Conflicts: # Sources/Sidebar/AppKitList/SidebarWorkspaceTableController.swift # Sources/Sidebar/AppKitList/SidebarWorkspaceTableViewImpl.swift # cmux.xcodeproj/project.pbxproj
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7984b9c45c
ℹ️ 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".
| if Set(nextRowIds) == Set(currentIds), nextRowIds.count == currentIds.count { | ||
| pendingApply = deferred | ||
| return true |
There was a problem hiding this comment.
When a same-ID structural apply arrives while a local drag is active, this Set-only check classifies it as content-only and defers it. The model order has already changed, but the table keeps resolving the preview through the previous actions/pickup order until the drag ends, so a socket/notification/other reorder with the same row IDs can make the final drop commit a stale target index over the new order; compare the incoming order against the active preview/expected order and degrade on mismatches instead of deferring every same-set apply.
AGENTS.md reference: AGENTS.md:L148-L150
Useful? React with 👍 / 👎.
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
lawrencecchen
added
the
stale-revisit
2 participants
Dragging in the AppKit sidebar previously showed only a 2px indicator line; the reorder happened after mouseup. Now drags that begin in the sidebar animate rows apart around a moving slot on every mousemove, Chrome-vertical-tabs style: a floating snapshot of the dragged row tracks the pointer (X-locked, lift shadow), the origin slot closes immediately, and escape or dropping outside animates everything back with no model change.
Group headers are now draggable and move their whole group as one block (the drop resolver already planned anchor drags as top-level block moves; the table just refused to start them). Dragging a workspace across a group's member span previews the join live and the floating row indents; the collapsed-group and pinned-tier semantics come from the existing resolver unchanged.
Mechanism: the system NSDraggingSession stays alive underneath. Our own drop overlay hides the drag image destination-side (it auto-restores on exit, so cross-window drags and bonsplit interop keep today's indicator path), resolves the same SidebarWorkspaceReorderDropResolver plan on every draggingUpdated, and a new pure permutation resolver (CmuxFoundation, unit-tested) maps that plan's indicator onto a visible row order which the table animates via moveRow. The model commits once on drop from the last previewed plan, so preview and commit agree by construction. Authoritative applies landing mid-drag are deferred when content-only and degrade the session to the classic indicator path when structural.
Verified on a fleet Mac (aws-m4pro-4, ardrec GUI session) with real cursor drags over the ARD protocol: mid-drag frames show rows reordered before mouseup for a workspace drag, a group-header block drag, and a group join with indent; escape restored the original order with no commit. 10 new unit tests on the permutation resolver pass.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds Chrome‑style live reorder in the sidebar. Rows animate around a moving slot with a floating snapshot; group headers drag their whole block. Direct pointer tracking runs local drags and hands off cleanly to the system session.
New Features
Refactors
SidebarWorkspaceDirectReorderGestureand direct pointer tracking in the table to own press/drag with a drag threshold and a reliable system‑drag handoff; rebuilt the local session around immutable drag state withSidebarWorkspaceTableLocalReorderSession.CmuxFoundation, addedSidebarWorkspaceReorderMovePlannerandSidebarWorkspaceReorderPreviewPermutation; resolver acceptsstickyDestinationand the preview refreshes targets after each permutation. Localized the “+N” badge string.Written for commit df2bb0a. Summary will update on new commits.
Summary by CodeRabbit