Keep pinned workspaces above the sidebar pin boundary (#1503)

* test: cover pinned workspace reorder boundary

* fix: keep pinned workspaces above drag boundary

---------

Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
This commit is contained in:
Lawrence Chen 2026-03-15 21:31:30 -07:00 committed by GitHub
parent 225c5b83bc
commit 7a9a6a550c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 128 additions and 8 deletions

View file

@ -11578,6 +11578,7 @@ enum SidebarDropPlanner {
draggedTabId: UUID?,
targetTabId: UUID?,
tabIds: [UUID],
pinnedTabIds: Set<UUID> = [],
pointerY: CGFloat? = nil,
targetHeight: CGFloat? = nil
) -> SidebarDropIndicator? {
@ -11598,16 +11599,27 @@ enum SidebarDropPlanner {
insertionPosition = tabIds.count
}
let targetIndex = resolvedTargetIndex(from: fromIndex, insertionPosition: insertionPosition, totalCount: tabIds.count)
guard targetIndex != fromIndex else { return nil }
return indicatorForInsertionPosition(insertionPosition, tabIds: tabIds)
let legalInsertionPosition = legalInsertionPosition(
draggedTabId: draggedTabId,
proposedInsertionPosition: insertionPosition,
tabIds: tabIds,
pinnedTabIds: pinnedTabIds
)
let legalTargetIndex = resolvedTargetIndex(
from: fromIndex,
insertionPosition: legalInsertionPosition,
totalCount: tabIds.count
)
guard legalTargetIndex != fromIndex else { return nil }
return indicatorForInsertionPosition(legalInsertionPosition, tabIds: tabIds)
}
static func targetIndex(
draggedTabId: UUID,
targetTabId: UUID?,
indicator: SidebarDropIndicator?,
tabIds: [UUID]
tabIds: [UUID],
pinnedTabIds: Set<UUID> = []
) -> Int? {
guard let fromIndex = tabIds.firstIndex(of: draggedTabId) else { return nil }
@ -11624,7 +11636,13 @@ enum SidebarDropPlanner {
insertionPosition = tabIds.count
}
return resolvedTargetIndex(from: fromIndex, insertionPosition: insertionPosition, totalCount: tabIds.count)
let legalInsertionPosition = legalInsertionPosition(
draggedTabId: draggedTabId,
proposedInsertionPosition: insertionPosition,
tabIds: tabIds,
pinnedTabIds: pinnedTabIds
)
return resolvedTargetIndex(from: fromIndex, insertionPosition: legalInsertionPosition, totalCount: tabIds.count)
}
private static func indicatorForInsertionPosition(_ insertionPosition: Int, tabIds: [UUID]) -> SidebarDropIndicator {
@ -11648,6 +11666,28 @@ enum SidebarDropPlanner {
return fromIndex < targetIndex ? .bottom : .top
}
private static func legalInsertionPosition(
draggedTabId: UUID,
proposedInsertionPosition: Int,
tabIds: [UUID],
pinnedTabIds: Set<UUID>
) -> Int {
let clampedInsertion = max(0, min(proposedInsertionPosition, tabIds.count))
guard !pinnedTabIds.isEmpty else { return clampedInsertion }
let pinnedCount = tabIds.reduce(into: 0) { count, tabId in
if pinnedTabIds.contains(tabId) {
count += 1
}
}
guard pinnedCount > 0 else { return clampedInsertion }
if pinnedTabIds.contains(draggedTabId) {
return min(clampedInsertion, pinnedCount)
}
return max(clampedInsertion, pinnedCount)
}
static func edgeForPointer(locationY: CGFloat, targetHeight: CGFloat) -> SidebarDropEdge {
guard targetHeight > 0 else { return .top }
let clampedY = min(max(locationY, 0), targetHeight)
@ -12030,7 +12070,8 @@ private struct SidebarTabDropDelegate: DropDelegate {
draggedTabId: draggedTabId,
targetTabId: targetTabId,
indicator: dropIndicator,
tabIds: tabIds
tabIds: tabIds,
pinnedTabIds: Set(tabManager.tabs.filter(\.isPinned).map(\.id))
) else {
#if DEBUG
dlog(
@ -12065,10 +12106,12 @@ private struct SidebarTabDropDelegate: DropDelegate {
private func updateDropIndicator(for info: DropInfo) {
let tabIds = tabManager.tabs.map(\.id)
let pinnedTabIds = Set(tabManager.tabs.filter(\.isPinned).map(\.id))
dropIndicator = SidebarDropPlanner.indicator(
draggedTabId: draggedTabId,
targetTabId: targetTabId,
tabIds: tabIds,
pinnedTabIds: pinnedTabIds,
pointerY: targetTabId == nil ? nil : info.location.y,
targetHeight: targetRowHeight
)

View file

@ -1389,10 +1389,11 @@ class TabManager: ObservableObject {
guard let currentIndex = tabs.firstIndex(where: { $0.id == tabId }) else { return false }
if tabs.count <= 1 { return true }
let clamped = max(0, min(targetIndex, tabs.count - 1))
let workspace = tabs[currentIndex]
let clamped = clampedReorderIndex(for: workspace, targetIndex: targetIndex)
if currentIndex == clamped { return true }
let workspace = tabs.remove(at: currentIndex)
tabs.remove(at: currentIndex)
tabs.insert(workspace, at: clamped)
return true
}
@ -1448,6 +1449,15 @@ class TabManager: ObservableObject {
tabs.insert(tab, at: insertIndex)
}
private func clampedReorderIndex(for workspace: Workspace, targetIndex: Int) -> Int {
let clamped = max(0, min(targetIndex, tabs.count - 1))
let pinnedCount = tabs.filter { $0.isPinned }.count
if workspace.isPinned {
return min(clamped, max(0, pinnedCount - 1))
}
return max(clamped, pinnedCount)
}
// MARK: - Surface Directory Updates (Backwards Compatibility)
func updateSurfaceDirectory(tabId: UUID, surfaceId: UUID, directory: String) {

View file

@ -5173,6 +5173,32 @@ final class WorkspaceReorderTests: XCTestCase {
let manager = TabManager()
XCTAssertFalse(manager.reorderWorkspace(tabId: UUID(), toIndex: 0))
}
@MainActor
func testReorderWorkspaceKeepsUnpinnedWorkspaceBelowPinnedSegment() {
let manager = TabManager()
let firstPinned = manager.tabs[0]
manager.setPinned(firstPinned, pinned: true)
let secondPinned = manager.addWorkspace()
manager.setPinned(secondPinned, pinned: true)
let unpinned = manager.addWorkspace()
XCTAssertTrue(manager.reorderWorkspace(tabId: unpinned.id, toIndex: 0))
XCTAssertEqual(manager.tabs.map(\.id), [firstPinned.id, secondPinned.id, unpinned.id])
}
@MainActor
func testReorderWorkspaceKeepsPinnedWorkspaceInsidePinnedSegment() {
let manager = TabManager()
let firstPinned = manager.tabs[0]
manager.setPinned(firstPinned, pinned: true)
let secondPinned = manager.addWorkspace()
manager.setPinned(secondPinned, pinned: true)
let unpinned = manager.addWorkspace()
XCTAssertTrue(manager.reorderWorkspace(tabId: firstPinned.id, toIndex: 999))
XCTAssertEqual(manager.tabs.map(\.id), [secondPinned.id, firstPinned.id, unpinned.id])
}
}
@MainActor
@ -7301,6 +7327,47 @@ final class SidebarDropPlannerTests: XCTestCase {
)
)
}
func testIndicatorSnapsUnpinnedDropToFirstUnpinnedBoundaryWhenHoveringPinnedWorkspace() {
let pinnedA = UUID()
let pinnedB = UUID()
let unpinnedA = UUID()
let unpinnedB = UUID()
let tabIds = [pinnedA, pinnedB, unpinnedA, unpinnedB]
let pinnedIds: Set<UUID> = [pinnedA, pinnedB]
let indicator = SidebarDropPlanner.indicator(
draggedTabId: unpinnedB,
targetTabId: pinnedA,
tabIds: tabIds,
pinnedTabIds: pinnedIds,
pointerY: 2,
targetHeight: 40
)
XCTAssertEqual(indicator?.tabId, unpinnedA)
XCTAssertEqual(indicator?.edge, .top)
}
func testTargetIndexSnapsUnpinnedDropToFirstUnpinnedBoundaryWhenHoveringPinnedWorkspace() {
let pinnedA = UUID()
let pinnedB = UUID()
let unpinnedA = UUID()
let unpinnedB = UUID()
let tabIds = [pinnedA, pinnedB, unpinnedA, unpinnedB]
let pinnedIds: Set<UUID> = [pinnedA, pinnedB]
let targetIndex = SidebarDropPlanner.targetIndex(
draggedTabId: unpinnedB,
targetTabId: pinnedA,
indicator: SidebarDropIndicator(tabId: pinnedA, edge: .top),
tabIds: tabIds,
pinnedTabIds: pinnedIds
)
XCTAssertEqual(targetIndex, 2)
}
}
final class SidebarDragAutoScrollPlannerTests: XCTestCase {