Skip to content

Sidebar: remove rows measurement that re-livelocks layout at scale #6188

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 6 commits into from
Jun 16, 2026
Merged

Sidebar: remove rows measurement that re-livelocks layout at scale #6188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b8dd9db
Sidebar: remove rows measurement that re-livelocks layout at scale
lawrencecchen Jun 15, 2026
003707d
Trim comments to keep ContentView within file-length budget
lawrencecchen Jun 15, 2026
483b389
Sidebar: keep end-of-list drop target below the rows, not behind them
lawrencecchen Jun 15, 2026
fa3de2b
Sidebar: neutralize all empty-area interactions over the rows block
lawrencecchen Jun 15, 2026
d7e5359
Merge origin/main into fix-sidebar-fill-no-measure
austinywang Jun 16, 2026
f9e29b8
Merge origin/main into fix-sidebar-fill-no-measure
austinywang Jun 16, 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
4 changes: 2 additions & 2 deletions .github/swift-file-length-budget.tsv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# Reduce counts as files shrink. CI fails if tracked files exceed this budget.
34475 CLI/cmux.swift
17894 Sources/AppDelegate.swift
16696 Sources/ContentView.swift
16714 Sources/ContentView.swift
14829 Sources/TerminalController.swift
13358 Sources/Panels/BrowserPanel.swift
13017 Sources/Workspace.swift
Expand Down Expand Up @@ -107,6 +107,7 @@
845 cmuxTests/SSHStartupSignalLifecycleTests.swift
841 Sources/Panels/MarkdownWebRenderer.swift
830 Sources/TaskManagerTypes.swift
825 Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swift
810 Packages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/SwiftViewInterpreterTests.swift
797 Sources/ClosedItemHistory.swift
779 cmuxUITests/BrowserOmnibarSuggestionsUITests.swift
Expand All @@ -120,7 +121,6 @@
752 cmuxUITests/CloseWorkspaceCmdDUITests.swift
749 Packages/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+Input.swift
746 Sources/App/MenuBarExtraController.swift
825 Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swift
738 Packages/CMUXProjectModel/Sources/CMUXProjectModel/XcodeProjectAdapter.swift
736 Packages/CmuxAuthRuntime/Sources/CmuxAuthRuntime/Coordinator/AuthCoordinator.swift
726 Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift
Expand Down
84 changes: 51 additions & 33 deletions Sources/ContentView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12061,34 +12061,53 @@ struct VerticalTabsSidebar: View {
renderContext: WorkspaceListRenderContext,
minHeight: CGFloat
) -> some View {
// Rows take their natural height; the empty drop/tap area stretches to
// fill the remaining viewport. SidebarRowsFillLayout sizes that
// remainder from the explicit viewport height (`minHeight`, the floored
// content height from the scroll geometry) rather than from a layout
// proposal, which a vertical ScrollView leaves unspecified in the
// scroll axis. So we never measure the LazyVStack's whole-content height
// into @State. That measurement (a .background GeometryReader writing a
// PreferenceKey) fed a non-converging relayout loop (#2586 / #5764 /
// #5845). The empty area fills exactly to the viewport when rows fit and
// collapses to 0 when they overflow, so the overlay scroller stays
// hidden (#3241) and the blank area below the last row stays a
// drop/tap target.
SidebarRowsFillLayout(viewportHeight: minHeight) {
workspaceRows(renderContext: renderContext)

SidebarEmptyArea(
rowSpacing: tabRowSpacing,
selection: $selection,
selectedTabIds: $selectedTabIds,
lastSidebarSelectionIndex: $lastSidebarSelectionIndex,
dragAutoScrollController: dragAutoScrollController,
topDropIndicatorVisible: emptyAreaTopDropIndicatorVisible(),
tabDropDelegate: emptyAreaTabDropDelegate(renderContext: renderContext),
bonsplitDropIndicator: dropIndicatorBinding,
expandsVertically: false
)
}
.frame(minHeight: minHeight, alignment: .top)
// Rows stay lazy + pinned top; `.frame(minHeight:)` fills the viewport
// (#3241) or scrolls without measuring the LazyVStack. The prior
// SidebarRowsFillLayout measured it (`sizeThatFits(height: nil)`) every
// pass, realizing all rows and re-livelocking at scale (#2586 / #5764 /
// #5845; regressed by #6033). Drop/tap = background; indicator on rows.
workspaceRows(renderContext: renderContext)
.overlay(alignment: .bottom) {
if emptyAreaTopDropIndicatorVisible() {
Rectangle()
.fill(cmuxAccentColor())
.frame(height: 2)
.padding(.horizontal, 8)
.offset(y: tabRowSpacing / 2)
}
}
Comment thread
austinywang marked this conversation as resolved.
// Neutralize ALL end-of-list empty-area interactions over the rows
// block (2pt gaps, row padding, and the entire list when it
// overflows) so none fall through to SidebarEmptyArea behind:
// workspace-reorder drops, Bonsplit new-workspace drops, and the
// double-tap-to-create gesture. Sized to the rows, so only the
// genuine blank area below the last row stays interactive. This is
// the measurement-free equivalent of physically placing the empty
// area below the rows; doing that requires asking the LazyVStack for
// its height, which realizes every row each layout pass and is the
// livelock this change removes. Per-row delegates render in front
// and still win over their own rows.
.background {
Color.clear
.contentShape(Rectangle())
.onTapGesture(count: 2) {}
.onDrop(of: SidebarTabDragPayload.dropContentTypes, isTargeted: nil) { _ in false }
.onDrop(of: BonsplitTabDragPayload.dropContentTypes, isTargeted: nil) { _ in false }
}
.frame(minHeight: minHeight, alignment: .top)
.background(alignment: .top) {
SidebarEmptyArea(
rowSpacing: tabRowSpacing,
selection: $selection,
selectedTabIds: $selectedTabIds,
lastSidebarSelectionIndex: $lastSidebarSelectionIndex,
dragAutoScrollController: dragAutoScrollController,
topDropIndicatorVisible: false,
tabDropDelegate: emptyAreaTabDropDelegate(renderContext: renderContext),
bonsplitDropIndicator: dropIndicatorBinding,
expandsVertically: true
)
}
}

@ViewBuilder
Expand Down Expand Up @@ -12123,11 +12142,10 @@ struct VerticalTabsSidebar: View {
}
.padding(.vertical, SidebarWorkspaceListMetrics.rowVerticalPadding)
.frame(maxWidth: .infinity, alignment: .leading)
// No whole-content height measurement here. Reading the LazyVStack's
// total height via a .background GeometryReader (to size the empty area)
// fed a non-converging relayout loop (#2586 / #5764 / #5845). The
// empty-area fill is now handled geometrically by SidebarRowsFillLayout
// in workspaceScrollContent without measuring these rows.
// No whole-content height measurement here: reading the LazyVStack's
// total height (GeometryReader, or a custom Layout's sizeThatFits) fed a
// non-converging relayout loop (#2586 / #5764 / #5845). Fill is handled
// by `.frame(minHeight:)` in workspaceScrollContent.

// Gate ONLY the per-row frame-anchor *reader* (the virtualization-defeating
// work) behind the drag-active check, and keep the Bonsplit drop-capture
Expand Down
69 changes: 0 additions & 69 deletions Sources/SidebarRowsFillLayout.swift
View file
Open in desktop

This file was deleted.

48 changes: 0 additions & 48 deletions Sources/WindowChromeMetrics.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,52 +89,4 @@ enum SidebarWorkspaceScrollLayout {
// (https://github.com/manaflow-ai/cmux/issues/3241).
return max(0, (viewportHeight - insets.total).rounded(.down))
}

/// Height of the empty drop/tap area `SidebarRowsFillLayout` places below the
/// last workspace row: the space remaining in the layout's concrete container.
///
/// The container height is the viewport floor (`.frame(minHeight:)`) when the
/// rows fit, or the rows' natural height when they overflow it. Because it is
/// derived from the layout's own bounds rather than a measured whole-content
/// height, the rows are never read into SwiftUI `@State` — which is what fed
/// the relayout loop
/// (https://github.com/manaflow-ai/cmux/issues/2586,
/// https://github.com/manaflow-ai/cmux/issues/5764). When the rows fit, rows +
/// empty area exactly fill the viewport, so there is no overflow and the
/// overlay scroller stays hidden (https://github.com/manaflow-ai/cmux/issues/3241);
/// when the rows overflow, this is `0` and the document view genuinely scrolls.
///
/// - Parameters:
/// - containerHeight: The layout's resolved container height.
/// - rowsHeight: The rows' natural height.
/// - Returns: The non-negative height for the empty area.
nonisolated static func emptyAreaFillHeight(
containerHeight: CGFloat,
rowsHeight: CGFloat
) -> CGFloat {
return max(0, containerHeight - rowsHeight)
}

/// Empty-area height from the explicit viewport, the way `SidebarRowsFillLayout`
/// computes it. The container is the viewport when the rows fit, or the rows'
/// height when they overflow it, so the empty area fills the remaining
/// viewport below the rows (keeping the blank area a drop/tap target) and
/// collapses to `0` once the rows overflow. Driven by the explicit viewport,
/// not a layout proposal — a vertical `ScrollView` leaves the scroll-axis
/// height unspecified, so deriving it from the proposal would collapse the
/// area to a placeholder height when the rows fit.
///
/// - Parameters:
/// - viewportHeight: The floored viewport height available to the content.
/// - rowsHeight: The rows' natural height.
/// - Returns: The non-negative height for the empty area.
nonisolated static func emptyAreaFillHeight(
viewportHeight: CGFloat,
rowsHeight: CGFloat
) -> CGFloat {
return emptyAreaFillHeight(
containerHeight: max(viewportHeight, rowsHeight),
rowsHeight: rowsHeight
)
}
}
4 changes: 0 additions & 4 deletions cmux.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,6 @@
C0DE48000000000000000001 /* SidebarProviderMenuRegressionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C0DE48000000000000000002 /* SidebarProviderMenuRegressionTests.swift */; };
B9000130A1B2C3D4E5F60719 /* SidebarPullRequestInteractivityUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B9000131A1B2C3D4E5F60719 /* SidebarPullRequestInteractivityUITests.swift */; };
B8F266236A1A3D9A45BD840F /* SidebarResizeUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 818DBCD4AB69EB72573E8138 /* SidebarResizeUITests.swift */; };
C9A57501C9A57501C9A57501 /* SidebarRowsFillLayout.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9A57502C9A57502C9A57502 /* SidebarRowsFillLayout.swift */; };
C0DE35010000000000000001 /* SidebarScrim.swift in Sources */ = {isa = PBXBuildFile; fileRef = C0DE35010000000000000002 /* SidebarScrim.swift */; };
C9A57513C9A57513C9A57513 /* SidebarScrollViewConfiguratorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9A57514C9A57514C9A57514 /* SidebarScrollViewConfiguratorTests.swift */; };
E62155868BB29FEB5DAAAF25 /* SidebarSelectionState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9AD52285508B1D6A9875E7B3 /* SidebarSelectionState.swift */; };
Expand Down Expand Up @@ -1662,7 +1661,6 @@
C0DE48000000000000000002 /* SidebarProviderMenuRegressionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarProviderMenuRegressionTests.swift; sourceTree = "<group>"; };
B9000131A1B2C3D4E5F60719 /* SidebarPullRequestInteractivityUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarPullRequestInteractivityUITests.swift; sourceTree = "<group>"; };
818DBCD4AB69EB72573E8138 /* SidebarResizeUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarResizeUITests.swift; sourceTree = "<group>"; };
C9A57502C9A57502C9A57502 /* SidebarRowsFillLayout.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarRowsFillLayout.swift; sourceTree = "<group>"; };
C0DE35010000000000000002 /* SidebarScrim.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarScrim.swift; sourceTree = "<group>"; };
C9A57514C9A57514C9A57514 /* SidebarScrollViewConfiguratorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarScrollViewConfiguratorTests.swift; sourceTree = "<group>"; };
9AD52285508B1D6A9875E7B3 /* SidebarSelectionState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarSelectionState.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2228,7 +2226,6 @@
D5037010000000000000002 /* RenderableSystemSymbol.swift */,
C9A57202C9A57202C9A57202 /* SidebarWorkspaceRenderItem.swift */,
C0DE5F210000000000000002 /* SidebarMetadataMarkdownRenderer.swift */,
C9A57502C9A57502C9A57502 /* SidebarRowsFillLayout.swift */,
C9A5720CC9A5720CC9A5720C /* TabItemView+WorkspaceGroups.swift */,
C9A5720AC9A5720AC9A5720A /* VerticalTabsSidebar+WorkspaceGroups.swift */,
C9A57204C9A57204C9A57204 /* WorkspaceGroupMenuSnapshot.swift */,
Expand Down Expand Up @@ -3803,7 +3800,6 @@
C0DE5F210000000000000001 /* SidebarMetadataMarkdownRenderer.swift in Sources */,
EA1F00000000000000000001 /* SidebarPathFormatter.swift in Sources */,
C0DEF0A30000000000000001 /* SidebarPortDisplayText.swift in Sources */,
C9A57501C9A57501C9A57501 /* SidebarRowsFillLayout.swift in Sources */,
C0DE35010000000000000001 /* SidebarScrim.swift in Sources */,
E62155868BB29FEB5DAAAF25 /* SidebarSelectionState.swift in Sources */,
F57072635F25EBCA741E125D /* SidebarState.swift in Sources */,
Expand Down
Loading
Loading