Skip to content

Remove sidebar row-ids preference aggregation feeding the layout livelock (#2586) #5708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lawrencecchen merged 4 commits into from
Jun 9, 2026
Merged

Remove sidebar row-ids preference aggregation feeding the layout livelock (#2586) #5708

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
830838b
Remove sidebar row-ids preference aggregation that fed the layout liv…
lawrencecchen Jun 9, 2026
68adaa7
Scroll to the group header when the selected workspace is in a collap…
lawrencecchen Jun 9, 2026
57eb378
Move collapsed-group scroll target into SidebarSelectedWorkspaceScrol…
lawrencecchen Jun 9, 2026
5bab011
Import Foundation in SidebarState for UUID
lawrencecchen Jun 9, 2026
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
30 changes: 10 additions & 20 deletions Sources/ContentView.swift
View file
Open in desktop
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 @@ -10876,14 +10875,18 @@ struct VerticalTabsSidebar: View {
flushPendingSelectedWorkspaceScroll(proxy)
}

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

// 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 aggregated across every
// sidebar row; that preference fed `@State` writes from inside the
// layout/preference update cycle and was the surviving 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.
proxy.scrollTo(selectedWorkspaceId)
pendingSelectedWorkspaceScrollId = nil

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep pending scrolls for hidden group children

When the selected workspace is a non-anchor child of a collapsed group (for example after the non-focus socket/CLI collapse path, which intentionally does not change selection), renderContext.workspaceIds still contains that UUID even though SidebarWorkspaceRenderItem.renderItems does not render a view with that .id. scrollTo is therefore an unknown-id no-op, but this now clears pendingSelectedWorkspaceScrollId; expanding the group later does not change workspaceIds or selectedTabId, so the selected row is never scrolled into view. Previously the pending request survived until the row preference appeared after expansion.

Useful? React with 👍 / 👎.

@lawrencecchen lawrencecchen Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 68adaa7: hidden members of a collapsed group now resolve their scroll target to the group header (anchor workspace id) from render-context data, so the scroll lands where the workspace lives instead of silently no-oping. Deliberately not restoring the deferred-scroll-on-expand: that required tracking what the lazy layout realized, which is the feedback edge this PR removes.

Expand Down Expand Up @@ -11236,10 +11239,6 @@ struct VerticalTabsSidebar: View {
lastSidebarSelectionIndex = index
}
}
.onPreferenceChange(SidebarWorkspaceRowIdsPreferenceKey.self) { rowIds in
laidOutWorkspaceRowIds = rowIds
flushPendingSelectedWorkspaceScroll(scrollProxy, laidOutWorkspaceRowIds: rowIds)
}
}
}
}
Expand Down Expand Up @@ -12625,7 +12624,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 +12636,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
1 change: 0 additions & 1 deletion Sources/VerticalTabsSidebar+WorkspaceGroups.swift
View file
Open in desktop
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
Loading