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
51 changes: 25 additions & 26 deletions Sources/ContentView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10585,7 +10585,6 @@ struct VerticalTabsSidebar: View {
// row sitting behind the open menu. See `SidebarShortcutHintFreezePolicy`.
@State private var frozenShortcutHintsTabId: UUID?
@State private var frozenShortcutHintsValue: Bool = false
@State private var laidOutWorkspaceRowIds: Set<UUID> = []
@State private var pendingSelectedWorkspaceScrollId: UUID?
@State private var collapsedExtensionSidebarSectionIds: Set<String> = []
@State private var extensionSidebarWorktreeCreationInFlightSectionIds: Set<String> = []
Expand Down Expand Up @@ -10865,27 +10864,40 @@ struct VerticalTabsSidebar: View {
return KeyboardShortcutSettings.shortcut(for: .selectWorkspaceByNumber)
}

private func requestSelectedWorkspaceScroll(_ proxy: ScrollViewProxy, workspaceIds: [UUID]) {
private func requestSelectedWorkspaceScroll(
_ proxy: ScrollViewProxy,
renderContext: WorkspaceListRenderContext
) {
guard let selectedWorkspaceId = tabManager.selectedTabId,
workspaceIds.contains(selectedWorkspaceId) else {
renderContext.workspaceIds.contains(selectedWorkspaceId) else {
pendingSelectedWorkspaceScrollId = nil
return
}

pendingSelectedWorkspaceScrollId = selectedWorkspaceId
flushPendingSelectedWorkspaceScroll(proxy)
flushPendingSelectedWorkspaceScroll(proxy, renderContext: renderContext)
}

private func flushPendingSelectedWorkspaceScroll(
_ proxy: ScrollViewProxy,
laidOutWorkspaceRowIds: Set<UUID>? = nil
renderContext: WorkspaceListRenderContext
) {
guard let selectedWorkspaceId = pendingSelectedWorkspaceScrollId else { return }
let rowIds = laidOutWorkspaceRowIds ?? self.laidOutWorkspaceRowIds
guard rowIds.contains(selectedWorkspaceId) else { return }

// No anchor means SwiftUI scrolls the minimum needed to reveal the row.
proxy.scrollTo(selectedWorkspaceId)
// Scroll unconditionally: ScrollViewProxy resolves `.id(_:)` values in
// lazy containers without requiring the row to be realized, and an
// unknown id is a harmless no-op. The previous design gated this on a
// per-row "laid-out row ids" PreferenceKey whose sidebar-wide reduce
// fed `@State` writes from inside the layout/preference update cycle,
// the cmux-owned edge in the sidebar layout livelock
// (https://github.com/manaflow-ai/cmux/issues/2586). No anchor means
// SwiftUI scrolls the minimum needed to reveal the row.
let group = renderContext.workspaceById[selectedWorkspaceId]?.groupId
.flatMap { renderContext.workspaceGroupById[$0] }
proxy.scrollTo(SidebarSelectedWorkspaceScrollPolicy.scrollTargetWorkspaceId(
selectedWorkspaceId: selectedWorkspaceId,
group: group
))
pendingSelectedWorkspaceScrollId = nil
}

Expand Down Expand Up @@ -11173,20 +11185,20 @@ struct VerticalTabsSidebar: View {
.background(Color.clear)
.modifier(ClearScrollBackground())
.onAppear {
requestSelectedWorkspaceScroll(scrollProxy, workspaceIds: renderContext.workspaceIds)
requestSelectedWorkspaceScroll(scrollProxy, renderContext: renderContext)
}
.onChange(of: tabManager.selectedTabId) { _, _ in
requestSelectedWorkspaceScroll(scrollProxy, workspaceIds: renderContext.workspaceIds)
requestSelectedWorkspaceScroll(scrollProxy, renderContext: renderContext)
}
.onChange(of: renderContext.workspaceIds) { oldWorkspaceIds, newWorkspaceIds in
guard shouldRequestSelectedWorkspaceScrollAfterWorkspaceIdsChange(
from: oldWorkspaceIds,
to: newWorkspaceIds
) else {
flushPendingSelectedWorkspaceScroll(scrollProxy)
flushPendingSelectedWorkspaceScroll(scrollProxy, renderContext: renderContext)
return
}
requestSelectedWorkspaceScroll(scrollProxy, workspaceIds: newWorkspaceIds)
requestSelectedWorkspaceScroll(scrollProxy, renderContext: renderContext)
}
.onReceive(NotificationCenter.default.publisher(for: .workspaceOrderDidChange)) { notification in
requestSelectedWorkspaceScrollAfterWorkspaceOrderChange(notification)
Expand Down Expand Up @@ -11236,10 +11248,6 @@ struct VerticalTabsSidebar: View {
lastSidebarSelectionIndex = index
}
}
.onPreferenceChange(SidebarWorkspaceRowIdsPreferenceKey.self) { rowIds in
laidOutWorkspaceRowIds = rowIds
flushPendingSelectedWorkspaceScroll(scrollProxy, laidOutWorkspaceRowIds: rowIds)
}
}
}
}
Expand Down Expand Up @@ -12625,7 +12633,6 @@ struct VerticalTabsSidebar: View {
.equatable()
.id(tab.id)
.accessibilityIdentifier("sidebarWorkspace.\(tab.id.uuidString)")
.preference(key: SidebarWorkspaceRowIdsPreferenceKey.self, value: Set([tab.id]))

row
.sidebarWorkspaceFrameAnchor(id: tab.id, isEnabled: shouldCollectWorkspaceDropTargets)
Expand All @@ -12638,14 +12645,6 @@ struct VerticalTabsSidebar: View {
}
}

struct SidebarWorkspaceRowIdsPreferenceKey: PreferenceKey {
static let defaultValue: Set<UUID> = []

static func reduce(value: inout Set<UUID>, nextValue: () -> Set<UUID>) {
value.formUnion(nextValue())
}
}

struct SidebarWorkspaceFrameAnchorModifier: ViewModifier {
let id: UUID
let isEnabled: Bool
Expand Down
14 changes: 14 additions & 0 deletions Sources/Sidebar/SidebarState.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Combine
import CoreGraphics
import Foundation

final class SidebarState: ObservableObject {
@Published var isVisible: Bool
Expand Down Expand Up @@ -74,4 +75,17 @@ enum SidebarSelectedWorkspaceScrollPolicy {

return true
}

/// A member of a collapsed group has no sidebar row of its own, so its
/// UUID is not a scrollable `.id` and `scrollTo` would no-op. Target the
/// group header (which carries the anchor workspace id) so the scroll
/// still lands where the workspace lives. Decided purely from model data,
/// never from what the lazy layout happens to have realized.
static func scrollTargetWorkspaceId(
selectedWorkspaceId: UUID,
group: WorkspaceGroup?
) -> UUID {
guard let group, group.isCollapsed else { return selectedWorkspaceId }
return group.anchorWorkspaceId
}
}
1 change: 0 additions & 1 deletion Sources/VerticalTabsSidebar+WorkspaceGroups.swift
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ extension VerticalTabsSidebar {
.equatable()
.id(group.anchorWorkspaceId)
.accessibilityIdentifier("sidebarWorkspaceGroup.\(group.id.uuidString)")
.preference(key: SidebarWorkspaceRowIdsPreferenceKey.self, value: Set([group.anchorWorkspaceId]))

header
.sidebarWorkspaceFrameAnchor(
Expand Down
45 changes: 45 additions & 0 deletions cmuxTests/SidebarWorkspaceSnapshotRefreshPolicyTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,51 @@ final class SidebarSelectedWorkspaceScrollPolicyTests: XCTestCase {
)
)
}

func testScrollTargetIsSelfWithoutGroup() {
let workspaceId = UUID()
XCTAssertEqual(
SidebarSelectedWorkspaceScrollPolicy.scrollTargetWorkspaceId(
selectedWorkspaceId: workspaceId,
group: nil
),
workspaceId
)
}

func testScrollTargetIsSelfInExpandedGroup() {
let workspaceId = UUID()
XCTAssertEqual(
SidebarSelectedWorkspaceScrollPolicy.scrollTargetWorkspaceId(
selectedWorkspaceId: workspaceId,
group: makeGroup(isCollapsed: false, anchorWorkspaceId: UUID())
),
workspaceId
)
}

func testScrollTargetIsGroupAnchorWhenGroupIsCollapsed() {
let anchorId = UUID()
XCTAssertEqual(
SidebarSelectedWorkspaceScrollPolicy.scrollTargetWorkspaceId(
selectedWorkspaceId: UUID(),
group: makeGroup(isCollapsed: true, anchorWorkspaceId: anchorId)
),
anchorId
)
}

private func makeGroup(isCollapsed: Bool, anchorWorkspaceId: UUID) -> WorkspaceGroup {
WorkspaceGroup(
id: UUID(),
name: "group",
isCollapsed: isCollapsed,
isPinned: false,
anchorWorkspaceId: anchorWorkspaceId,
customColor: nil,
iconSymbol: nil
)
}
}

final class SidebarWorkspaceRowInteractionStateTests: XCTestCase {
Expand Down
Loading