Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 38 additions & 5 deletions Sources/Sidebar/AppKitList/SidebarWorkspaceTableController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -762,6 +762,7 @@ final class SidebarWorkspaceTableController: NSObject, NSTableViewDataSource, NS
var previousIds: [SidebarWorkspaceRenderItemID] = []
var nextIds: [SidebarWorkspaceRenderItemID] = []
var isSmallPureReorder = false
var pureEdit: SidebarWorkspaceTableRowEdit?
if hasStructuralChanges {
previousIds = previousRows.map(\.id)
nextIds = nextRows.map(\.id)
Expand All @@ -777,6 +778,9 @@ final class SidebarWorkspaceTableController: NSObject, NSTableViewDataSource, NS
isSmallPureReorder = previousIds.count == nextIds.count
&& mismatches <= Self.maxAnimatedReorderMoves
&& Self.multisetEqual(previousIds, nextIds)
if !previousIds.isEmpty {
pureEdit = SidebarWorkspaceTableRowEdit(from: previousIds, to: nextIds)
}
}
let requiresAtomicReorderReload =
hasStructuralChanges && !heightChanges.isEmpty && isSmallPureReorder
Expand Down Expand Up @@ -836,12 +840,29 @@ final class SidebarWorkspaceTableController: NSObject, NSTableViewDataSource, NS
table.endUpdates()
// Per-index state (first-row flag, drop-indicator geometry)
// shifts with the order even when per-id content didn't.
let visible = table.rows(in: table.visibleRect)
if visible.length > 0 {
reconfigureVisibleRows(
IndexSet(integersIn: visible.lowerBound..<(visible.lowerBound + visible.length))
)
reconfigureLoadedRows(in: table)
}
} else if let pureEdit {
// Closing or creating a workspace (or collapsing/expanding a
// group) only drops or adds rows. reloadData tore down every
// visible cell for that: rename and checklist drafts on
// unrelated rows committed early, their popovers closed, and
// every row repainted from a recycled cell. Touch only the
// affected rows; the rest keep their cells.
let table = containerView.tableView
performTableGeometryUpdateWithoutAnimation(heightChanges, in: table) {
table.beginUpdates()
switch pureEdit {
case .remove(let indexes):
table.removeRows(at: indexes, withAnimation: [])
case .insert(let indexes):
table.insertRows(at: indexes, withAnimation: [])
}
table.endUpdates()
// Per-index state (shortcut digits, first-row flag, group
// counts) shifts with the edit even for rows whose own
// content did not; configure skips cells whose model is equal.
reconfigureLoadedRows(in: table)
}
} else {
let table = containerView.tableView
Expand Down Expand Up @@ -2450,6 +2471,18 @@ final class SidebarWorkspaceTableController: NSObject, NSTableViewDataSource, NS
}
}

/// Row edits that keep cells (moves, inserts, removes) must refresh every
/// loaded row view, not just the visible ones: NSTableView keeps prepared
/// views above and below the viewport, and viewFor is not asked again
/// when they scroll in, so a visible-only pass left them stale.
private func reconfigureLoadedRows(in table: NSTableView) {
var loaded = IndexSet()
table.enumerateAvailableRowViews { _, row in
if row >= 0 { loaded.insert(row) }
}
reconfigureVisibleRows(loaded)
}

private func reconfigureVisibleRows(_ indexes: IndexSet) {
guard let table = containerView?.tableView else { return }
for row in indexes where rows.indices.contains(row) {
Expand Down
43 changes: 43 additions & 0 deletions Sources/Sidebar/AppKitList/SidebarWorkspaceTableRowEdit.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import Foundation

/// A structural row change that only drops rows or only adds rows, with the
/// surviving rows in the same relative order. Such edits can be applied with
/// removeRows/insertRows instead of reloading every visible cell.
enum SidebarWorkspaceTableRowEdit: Equatable {
/// Indexes in the previous row list.
case remove(IndexSet)
/// Indexes in the next row list.
case insert(IndexSet)

init?<ID: Hashable>(from previous: [ID], to next: [ID]) {
guard previous != next,
Set(previous).count == previous.count,
Set(next).count == next.count else {
return nil
}
if next.count < previous.count,
let removed = Self.indexesMissing(from: next, in: previous) {
self = .remove(removed)
} else if next.count > previous.count,
let inserted = Self.indexesMissing(from: previous, in: next) {
self = .insert(inserted)
} else {
return nil
}
}

/// Indexes of `superset` not in `subset`, or nil unless `subset` is an
/// order-preserving subsequence of `superset`.
private static func indexesMissing<ID: Equatable>(from subset: [ID], in superset: [ID]) -> IndexSet? {
var missing = IndexSet()
var subsetIndex = subset.startIndex
for (index, id) in superset.enumerated() {
if subsetIndex < subset.endIndex, subset[subsetIndex] == id {
subsetIndex += 1
} else {
missing.insert(index)
}
}
return subsetIndex == subset.endIndex ? missing : nil
}
}
4 changes: 4 additions & 0 deletions cmux.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -3053,6 +3053,7 @@
B804A0160000000000000016 /* SidebarWorkspaceTableMutationScheduler.swift in Sources */ = {isa = PBXBuildFile; fileRef = B804B0160000000000000016 /* SidebarWorkspaceTableMutationScheduler.swift */; };
7D41A0C0DE0100000000A0A1 /* SidebarWorkspaceTableReorderIndicatorPainter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7D41B0C0DE0100000000B0B1 /* SidebarWorkspaceTableReorderIndicatorPainter.swift */; };
B804A0060000000000000006 /* SidebarWorkspaceTableRowConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = B804B0060000000000000006 /* SidebarWorkspaceTableRowConfiguration.swift */; };
B804A00500000000000000E1 /* SidebarWorkspaceTableRowEdit.swift in Sources */ = {isa = PBXBuildFile; fileRef = B804B00500000000000000E1 /* SidebarWorkspaceTableRowEdit.swift */; };
B804A00D000000000000000D /* SidebarWorkspaceTableRowHeightCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = B804B00D000000000000000D /* SidebarWorkspaceTableRowHeightCache.swift */; };
B804A0090000000000000009 /* SidebarWorkspaceTableRowHeightCalculator.swift in Sources */ = {isa = PBXBuildFile; fileRef = B804B0090000000000000009 /* SidebarWorkspaceTableRowHeightCalculator.swift */; };
B8624C030000000000000003 /* SidebarWorkspaceTableSuspensionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B8624D030000000000000003 /* SidebarWorkspaceTableSuspensionTests.swift */; };
Expand Down Expand Up @@ -7012,6 +7013,7 @@
B804B0160000000000000016 /* SidebarWorkspaceTableMutationScheduler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/AppKitList/SidebarWorkspaceTableMutationScheduler.swift; sourceTree = "<group>"; };
7D41B0C0DE0100000000B0B1 /* SidebarWorkspaceTableReorderIndicatorPainter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/AppKitList/SidebarWorkspaceTableReorderIndicatorPainter.swift; sourceTree = "<group>"; };
B804B0060000000000000006 /* SidebarWorkspaceTableRowConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/AppKitList/SidebarWorkspaceTableRowConfiguration.swift; sourceTree = "<group>"; };
B804B00500000000000000E1 /* SidebarWorkspaceTableRowEdit.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/AppKitList/SidebarWorkspaceTableRowEdit.swift; sourceTree = "<group>"; };
B804B00D000000000000000D /* SidebarWorkspaceTableRowHeightCache.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/AppKitList/SidebarWorkspaceTableRowHeightCache.swift; sourceTree = "<group>"; };
B804B0090000000000000009 /* SidebarWorkspaceTableRowHeightCalculator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/AppKitList/SidebarWorkspaceTableRowHeightCalculator.swift; sourceTree = "<group>"; };
B8624D030000000000000003 /* SidebarWorkspaceTableSuspensionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarWorkspaceTableSuspensionTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -9023,6 +9025,7 @@
7D41B0C0DE0100000000B0B1 /* SidebarWorkspaceTableReorderIndicatorPainter.swift */,
B804B00A000000000000000A /* SidebarWorkspaceTableEnvironmentSnapshot.swift */,
B804B0050000000000000005 /* SidebarWorkspaceTableHoverResolver.swift */,
B804B00500000000000000E1 /* SidebarWorkspaceTableRowEdit.swift */,
B804B0060000000000000006 /* SidebarWorkspaceTableRowConfiguration.swift */,
B804B00D000000000000000D /* SidebarWorkspaceTableRowHeightCache.swift */,
B804B0090000000000000009 /* SidebarWorkspaceTableRowHeightCalculator.swift */,
Expand Down Expand Up @@ -14552,6 +14555,7 @@
B804A0160000000000000016 /* SidebarWorkspaceTableMutationScheduler.swift in Sources */,
7D41A0C0DE0100000000A0A1 /* SidebarWorkspaceTableReorderIndicatorPainter.swift in Sources */,
B804A0060000000000000006 /* SidebarWorkspaceTableRowConfiguration.swift in Sources */,
B804A00500000000000000E1 /* SidebarWorkspaceTableRowEdit.swift in Sources */,
B804A00D000000000000000D /* SidebarWorkspaceTableRowHeightCache.swift in Sources */,
B804A0090000000000000009 /* SidebarWorkspaceTableRowHeightCalculator.swift in Sources */,
B804A0070000000000000007 /* SidebarWorkspaceTableView.swift in Sources */,
Expand Down
62 changes: 62 additions & 0 deletions cmuxTests/SidebarWorkspaceRowRetirementTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,68 @@ struct SidebarWorkspaceRowRetirementTests {
#expect(applies > 0, "A retired menu must not suppress hover on replacement rows.")
}

/// Closing one workspace must not rebuild the other rows: reloadData
/// retired every visible cell, committing rename/checklist drafts and
/// closing popovers on unrelated rows.
@Test
func closingOneWorkspaceKeepsSurvivingRowCells() async throws {
let models = (0..<3).map { _ in SidebarWorkspaceRowSuspensionTests.makeModel() }
let rows = models.map {
makeRowConfiguration(model: $0, actions: SidebarWorkspaceRowSuspensionTests.makeActions(model: $0))
}
let controller = SidebarWorkspaceTableController()
let container = controller.makeContainerView()
let tableActions = makeTableActions()
let window = NSWindow(
contentRect: NSRect(x: 0, y: 0, width: 320, height: 480),
styleMask: [.borderless],
backing: .buffered,
defer: false
)
window.contentView = container
window.orderFront(nil)
defer { window.close() }

func apply(_ rows: [SidebarWorkspaceTableRowConfiguration]) async {
controller.apply(
rows: rows,
actions: tableActions,
workspaceIds: rows.compactMap { $0.appKitWorkspaceRowModel?.workspaceId },
selectedWorkspaceId: nil,
selectedScrollTargetWorkspaceId: nil
)
await flushStagedTableMutations()
container.layoutSubtreeIfNeeded()
container.tableView.layoutSubtreeIfNeeded()
}
func cell(at row: Int) -> NSView? {
container.tableView.view(atColumn: 0, row: row, makeIfNecessary: false)
}

await apply(rows)
let first = try #require(cell(at: 0))
let third = try #require(cell(at: 2))

await apply([rows[0], rows[2]])

#expect(container.tableView.numberOfRows == 2)
#expect(cell(at: 0) === first)
#expect(cell(at: 1) === third)
}

@Test
func rowEditClassifiesOnlyOrderPreservingDropsAndAdds() {
#expect(SidebarWorkspaceTableRowEdit(from: [1, 2, 3], to: [1, 3]) == .remove([1]))
#expect(SidebarWorkspaceTableRowEdit(from: [1, 2, 3, 4], to: [2, 4]) == .remove([0, 2]))
#expect(SidebarWorkspaceTableRowEdit(from: [1, 3], to: [1, 2, 3, 4]) == .insert([1, 3]))
#expect(SidebarWorkspaceTableRowEdit(from: [1, 2], to: []) == .remove([0, 1]))
// Reorders, mixed edits, no-ops, and duplicate ids keep the reload path.
#expect(SidebarWorkspaceTableRowEdit(from: [1, 2, 3], to: [3, 1]) == nil)
#expect(SidebarWorkspaceTableRowEdit(from: [1, 2, 3], to: [1, 4]) == nil)
#expect(SidebarWorkspaceTableRowEdit(from: [1, 2], to: [1, 2]) == nil)
#expect(SidebarWorkspaceTableRowEdit(from: [1, 1, 2], to: [1, 2]) == nil)
}

private func mount(
model: SidebarWorkspaceRowModel,
actions: SidebarAppKitRowActions
Expand Down
Loading