Skip to content

Fix sidebar row-height layout feedback #6111

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
austinywang merged 1 commit into from
Jun 14, 2026
Merged

Fix sidebar row-height layout feedback #6111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
107 changes: 38 additions & 69 deletions Sources/ContentView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12328,7 +12328,7 @@ struct VerticalTabsSidebar: View {
dragState.beginDragging(tabId: tabId)
return SidebarTabDragPayload.provider(for: tabId)
}
let tabDropDelegateFactory: (CGFloat) -> SidebarTabDropDelegate = { [
let tabDropDelegateFactory: (CGFloat?) -> SidebarTabDropDelegate = { [
tabId = tab.id,
selectedTabIds = $selectedTabIds,
lastSidebarSelectionIndex = $lastSidebarSelectionIndex
Expand Down Expand Up @@ -13356,10 +13356,10 @@ struct TabItemView: View, Equatable {
let isBeingDragged: Bool
let topDropIndicatorVisible: Bool
let onDragStart: () -> NSItemProvider
/// Factory invoked from `body` with the row's measured `rowHeight`. Closure
/// Factory invoked from `body` with a stable drop-hit height. Closure
/// captures the parent's `dragState`, so TabItemView itself never holds an
/// `@Observable` store reference (snapshot-boundary rule).
let tabDropDelegateFactory: (CGFloat) -> SidebarTabDropDelegate
/// `@Observable` store reference or layout-driven state (snapshot-boundary rule).
let tabDropDelegateFactory: (CGFloat?) -> SidebarTabDropDelegate
let contextMenuWorkspaceIds: [UUID]
let remoteContextMenuWorkspaceIds: [UUID]
let allRemoteContextMenuTargetsConnecting: Bool
Expand All @@ -13376,8 +13376,9 @@ struct TabItemView: View, Equatable {
@State private var workspaceSnapshotStorage: SidebarWorkspaceSnapshotBuilder.Snapshot?
@StateObject private var contextMenuState = SidebarTabItemContextMenuState()
@State private var rowInteractionState = SidebarWorkspaceRowInteractionState()
@State private var rowHeight: CGFloat = 1
@State private var workspaceFinderDirectoryOpenRequest: WorkspaceFinderDirectoryOpenRequest?
@State var workspaceFinderDirectoryOpenRequest: WorkspaceFinderDirectoryOpenRequest?
@State var metadataRowsExpanded = false
@State var metadataBlocksExpanded = false

var isMultiSelected: Bool {
selectedTabIds.contains(tab.id)
Expand Down Expand Up @@ -13610,18 +13611,6 @@ struct TabItemView: View, Equatable {
}
}

private var rowHeightProbe: some View {
GeometryReader { proxy in
Color.clear
.onAppear {
rowHeight = max(proxy.size.height, 1)
}
.onChange(of: proxy.size.height) { newHeight in
rowHeight = max(newHeight, 1)
}
}
}

@ViewBuilder
private var remoteWorkspaceSection: some View {
let workspaceSnapshot = self.workspaceSnapshot
Expand Down Expand Up @@ -13713,6 +13702,13 @@ struct TabItemView: View, Equatable {
: nil
let effectiveSubtitle = latestNotificationSubtitle ?? conversationMessageSubtitle
let detailVisibility = visibleAuxiliaryDetails
let titleLineLimit = settings.wrapsWorkspaceTitles
? SidebarWorkspaceRowDropMetrics.maxWrappedTitleLines
: 1
let dropTargetHeight = workspaceDropTargetHeight(
snapshot: workspaceSnapshot,
effectiveSubtitle: effectiveSubtitle
)
let scaledUnreadBadgeSize = 16 * fontScale
let scaledCloseButtonHitSize = max(16, 16 * fontScale)
let scaledCloseButtonWidth = max(
Expand Down Expand Up @@ -13743,7 +13739,7 @@ struct TabItemView: View, Equatable {
Text(workspaceSnapshot.title)
.font(.system(size: scaledFontSize(12.5), weight: titleFontWeight))
.foregroundColor(activePrimaryTextColor)
.lineLimit(settings.wrapsWorkspaceTitles ? nil : 1)
.lineLimit(titleLineLimit)
.truncationMode(.tail)
.fixedSize(horizontal: false, vertical: true)
.frame(maxWidth: .infinity, alignment: .leading)
Expand All @@ -13756,10 +13752,7 @@ struct TabItemView: View, Equatable {
// row. (Matches the group-header plus-button pattern.)
if canCloseWorkspace {
Button(action: {
#if DEBUG
cmuxDebugLog("sidebar.close workspace=\(tab.id.uuidString.prefix(5)) method=button")
#endif
tabManager.closeWorkspaceWithConfirmation(tab)
closeWorkspace(method: "button")
}) {
Image(systemName: "xmark")
.font(.system(size: scaledFontSize(9), weight: .medium))
Expand Down Expand Up @@ -13801,6 +13794,7 @@ struct TabItemView: View, Equatable {
if !metadataEntries.isEmpty {
SidebarMetadataRows(
entries: metadataEntries,
isExpanded: $metadataRowsExpanded,
isActive: usesInvertedActiveForeground,
activeForegroundColor: activeSecondaryColor(0.95),
activeSecondaryForegroundColor: activeSecondaryColor(0.65),
Expand All @@ -13812,6 +13806,7 @@ struct TabItemView: View, Equatable {
if !metadataBlocks.isEmpty {
SidebarMetadataMarkdownBlocks(
blocks: metadataBlocks,
isExpanded: $metadataBlocksExpanded,
isActive: usesInvertedActiveForeground,
activeForegroundColor: activeSecondaryColor(0.8),
activeSecondaryForegroundColor: activeSecondaryColor(0.65),
Expand Down Expand Up @@ -14045,18 +14040,14 @@ struct TabItemView: View, Equatable {
)
.shortcutHintVisibilityAnimation(value: showsWorkspaceShortcutHint)
.padding(.horizontal, 6)
.background { rowHeightProbe }
.contentShape(Rectangle())
.opacity(isBeingDragged ? 0.6 : 1)
.overlay {
SidebarWorkspaceRowHoverTracker(rowInteractionState: $rowInteractionState)
}
.overlay {
MiddleClickCapture {
#if DEBUG
cmuxDebugLog("sidebar.close workspace=\(tab.id.uuidString.prefix(5)) method=middleClick")
#endif
tabManager.closeWorkspaceWithConfirmation(tab)
closeWorkspace(method: "middleClick")
}
}
.overlay(alignment: .top) {
Expand All @@ -14070,26 +14061,13 @@ struct TabItemView: View, Equatable {
refreshWorkspaceSnapshot(force: true)
}
.task(id: workspaceFinderDirectoryOpenRequest) {
guard let request = workspaceFinderDirectoryOpenRequest else { return }
await WorkspaceFinderDirectoryOpener.openInFinder(request.directoryURL)
guard !Task.isCancelled, workspaceFinderDirectoryOpenRequest == request else { return }
workspaceFinderDirectoryOpenRequest = nil
await openPendingFinderDirectoryRequest()
}
.onReceive(
tab.sidebarImmediateObservationPublisher
.receive(on: RunLoop.main)
) { _ in
#if DEBUG
let description = tab.customDescription ?? ""
cmuxDebugLog(
"sidebar.row.invalidate workspace=\(tab.id.uuidString.prefix(8)) " +
"source=immediate " +
"title=\"\(debugCommandPaletteTextPreview(tab.title))\" " +
"descLen=\((description as NSString).length) " +
"desc=\"\(debugCommandPaletteTextPreview(description))\""
)
#endif
refreshWorkspaceSnapshot()
refreshWorkspaceSnapshotAfterObservation(source: "immediate")
}
.onReceive(
tab.sidebarObservationPublisher
Expand All @@ -14099,24 +14077,14 @@ struct TabItemView: View, Equatable {
// row redraws once with the settled state instead of blinking.
.debounce(for: Self.workspaceObservationCoalesceInterval, scheduler: RunLoop.main)
) { _ in
#if DEBUG
let description = tab.customDescription ?? ""
cmuxDebugLog(
"sidebar.row.invalidate workspace=\(tab.id.uuidString.prefix(8)) " +
"source=debounced " +
"title=\"\(debugCommandPaletteTextPreview(tab.title))\" " +
"descLen=\((description as NSString).length) " +
"desc=\"\(debugCommandPaletteTextPreview(description))\""
)
#endif
refreshWorkspaceSnapshot()
refreshWorkspaceSnapshotAfterObservation(source: "debounced")
}
.onChange(of: settings) { _ in
refreshWorkspaceSnapshot(force: true)
}
.onDrag(onDragStart)
.internalOnlyTabDrag()
.onDrop(of: SidebarTabDragPayload.dropContentTypes, delegate: tabDropDelegateFactory(rowHeight))
.onDrop(of: SidebarTabDragPayload.dropContentTypes, delegate: tabDropDelegateFactory(dropTargetHeight))
.onDrop(of: BonsplitTabDragPayload.dropContentTypes, delegate: SidebarBonsplitTabDropDelegate(
targetWorkspaceId: tab.id,
tabManager: tabManager,
Expand Down Expand Up @@ -14152,7 +14120,7 @@ struct TabItemView: View, Equatable {
}
}

private func refreshWorkspaceSnapshot(force: Bool = false) {
func refreshWorkspaceSnapshot(force: Bool = false) {
let nextSnapshot = makeWorkspaceSnapshot()
let decision = SidebarWorkspaceSnapshotRefreshPolicy.decision(
current: workspaceSnapshotStorage,
Expand Down Expand Up @@ -15251,6 +15219,7 @@ private struct SidebarWorkspaceDescriptionText: View {
.font(.system(size: 10.5 * fontScale))
.foregroundColor(foregroundColor)
.multilineTextAlignment(.leading)
.lineLimit(SidebarWorkspaceRowDropMetrics.maxDescriptionLines)
.fixedSize(horizontal: false, vertical: true)
.frame(maxWidth: .infinity, alignment: .leading)
.accessibilityIdentifier("SidebarWorkspaceDescriptionText")
Expand Down Expand Up @@ -15297,15 +15266,13 @@ private struct SidebarWorkspaceDescriptionText: View {

private struct SidebarMetadataRows: View {
let entries: [SidebarStatusEntry]
@Binding var isExpanded: Bool
let isActive: Bool
let activeForegroundColor: Color
let activeSecondaryForegroundColor: Color
let fontScale: CGFloat
let onFocus: () -> Void

@State private var isExpanded: Bool = false
private let collapsedEntryLimit = 3

var body: some View {
VStack(alignment: .leading, spacing: 2) {
ForEach(visibleEntries, id: \.key) { entry in
Expand Down Expand Up @@ -15335,8 +15302,10 @@ private struct SidebarMetadataRows: View {
}

private var visibleEntries: [SidebarStatusEntry] {
guard !isExpanded, entries.count > collapsedEntryLimit else { return entries }
return Array(entries.prefix(collapsedEntryLimit))
guard !isExpanded, entries.count > SidebarWorkspaceRowDropMetrics.collapsedMetadataEntryLimit else {
return entries
}
return Array(entries.prefix(SidebarWorkspaceRowDropMetrics.collapsedMetadataEntryLimit))
}

private var helpText: String {
Expand All @@ -15348,7 +15317,7 @@ private struct SidebarMetadataRows: View {
}

private var shouldShowToggle: Bool {
entries.count > collapsedEntryLimit
entries.count > SidebarWorkspaceRowDropMetrics.collapsedMetadataEntryLimit
}
}

Expand Down Expand Up @@ -15453,15 +15422,13 @@ private struct SidebarMetadataEntryRow: View {

private struct SidebarMetadataMarkdownBlocks: View {
let blocks: [SidebarMetadataBlock]
@Binding var isExpanded: Bool
let isActive: Bool
let activeForegroundColor: Color
let activeSecondaryForegroundColor: Color
let fontScale: CGFloat
let onFocus: () -> Void

@State private var isExpanded: Bool = false
private let collapsedBlockLimit = 1

var body: some View {
VStack(alignment: .leading, spacing: 3) {
ForEach(visibleBlocks, id: \.key) { block in
Expand Down Expand Up @@ -15490,12 +15457,14 @@ private struct SidebarMetadataMarkdownBlocks: View {
}

private var visibleBlocks: [SidebarMetadataBlock] {
guard !isExpanded, blocks.count > collapsedBlockLimit else { return blocks }
return Array(blocks.prefix(collapsedBlockLimit))
guard !isExpanded, blocks.count > SidebarWorkspaceRowDropMetrics.collapsedMetadataBlockLimit else {
return blocks
}
return Array(blocks.prefix(SidebarWorkspaceRowDropMetrics.collapsedMetadataBlockLimit))
}

private var shouldShowToggle: Bool {
blocks.count > collapsedBlockLimit
blocks.count > SidebarWorkspaceRowDropMetrics.collapsedMetadataBlockLimit
}
}

Expand Down Expand Up @@ -15523,6 +15492,7 @@ private struct SidebarMetadataMarkdownBlockRow: View {
}
.font(.system(size: 10 * fontScale))
.multilineTextAlignment(.leading)
.lineLimit(SidebarWorkspaceRowDropMetrics.maxMetadataBlockLines)
.fixedSize(horizontal: false, vertical: true)
.contentShape(Rectangle())
.onTapGesture { onFocus() }
Expand Down Expand Up @@ -16823,4 +16793,3 @@ enum SidebarPresetOption: String, CaseIterable, Identifiable {
}
}
}

5 changes: 5 additions & 0 deletions Sources/SidebarWorkspaceGroupHeaderMetrics.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,9 @@ struct SidebarWorkspaceGroupHeaderMetrics: Equatable {
var plusFontSize: CGFloat { Self.basePlusFontSize * fontScale }
/// Scaled plus-button frame edge.
var plusFrame: CGFloat { Self.basePlusFrame * fontScale }
/// Stable drop-hit height for the group header, without reading SwiftUI layout.
var dropTargetHeight: CGFloat {
let contentHeight = max(chevronFrame, iconFrame, plusFrame, nameFontSize + 4)
return max(24 * fontScale, contentHeight + 10)
}
Comment on lines +76 to +80

@coderabbitai coderabbitai Bot Jun 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Align header scale floor with row drop metrics.

SidebarWorkspaceRowDropMetrics floors fontScale to 0.5, but this header path uses raw fontScale. If settings/config ever provide values below that floor, row/header drop-hit heights become inconsistent.

Suggested fix 
     /// Stable drop-hit height for the group header, without reading SwiftUI layout.
     var dropTargetHeight: CGFloat {
-        let contentHeight = max(chevronFrame, iconFrame, plusFrame, nameFontSize + 4)
-        return max(24 * fontScale, contentHeight + 10)
+        let scale = max(fontScale, 0.5)
+        let contentHeight = max(
+            Self.baseChevronFrame * scale,
+            Self.baseIconFrame * scale,
+            Self.basePlusFrame * scale,
+            Self.baseNameFontSize * scale + 4
+        )
+        return max(24 * scale, contentHeight + 10)
     }
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Sources/SidebarWorkspaceGroupHeaderMetrics.swift` around lines 76 - 80, The
dropTargetHeight property in SidebarWorkspaceGroupHeaderMetrics uses raw
fontScale without enforcing a minimum floor value, while
SidebarWorkspaceRowDropMetrics floors fontScale to 0.5. To ensure consistent
drop-hit heights between headers and rows, apply the same 0.5 floor to fontScale
in the dropTargetHeight calculation by using max(0.5, fontScale) before
multiplying by 24 in the return statement.

}
19 changes: 4 additions & 15 deletions Sources/SidebarWorkspaceGroupHeaderView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ struct SidebarWorkspaceGroupHeaderView: View, Equatable {
let isBeingDragged: Bool
let topDropIndicatorVisible: Bool
let onDragStart: () -> NSItemProvider
/// Factory invoked from `body` with a stable drop-hit height. Closure
/// captures the parent's drag state while this LazyVStack row stays free of
/// layout-driven state writes.
let tabDropDelegateFactory: (CGFloat) -> SidebarWorkspaceGroupHeaderDropDelegate
let onToggleCollapsed: () -> Void
let onFocusAnchor: () -> Void
Expand All @@ -70,7 +73,6 @@ struct SidebarWorkspaceGroupHeaderView: View, Equatable {
let onOpenDocs: () -> Void

@State private var isHovered = false
@State private var rowHeight: CGFloat = 1

private var metrics: SidebarWorkspaceGroupHeaderMetrics {
SidebarWorkspaceGroupHeaderMetrics(fontScale: fontScale)
Expand All @@ -94,18 +96,6 @@ struct SidebarWorkspaceGroupHeaderView: View, Equatable {
return "\(shortcutModifierSymbol)\(shortcutDigit)"
}

private var rowHeightProbe: some View {
GeometryReader { proxy in
Color.clear
.onAppear {
rowHeight = max(proxy.size.height, 1)
}
.onChange(of: proxy.size.height) { _, newHeight in
rowHeight = max(newHeight, 1)
}
}
}

var body: some View {
HStack(spacing: 4) {
Image(systemName: isCollapsed ? "chevron.right" : "chevron.down")
Expand Down Expand Up @@ -227,7 +217,6 @@ struct SidebarWorkspaceGroupHeaderView: View, Equatable {
offsetY: shortcutHintYOffset
)
.padding(.horizontal, 6)
.background { rowHeightProbe }
.shortcutHintVisibilityAnimation(value: showsShortcutHint)
.opacity(isBeingDragged ? 0.6 : 1)
.overlay(alignment: .top) {
Expand All @@ -239,7 +228,7 @@ struct SidebarWorkspaceGroupHeaderView: View, Equatable {
}
.onDrag(onDragStart)
.internalOnlyTabDrag()
.onDrop(of: SidebarTabDragPayload.dropContentTypes, delegate: tabDropDelegateFactory(rowHeight))
.onDrop(of: SidebarTabDragPayload.dropContentTypes, delegate: tabDropDelegateFactory(metrics.dropTargetHeight))
.onHover { hovering in
isHovered = hovering
}
Expand Down
Loading
Loading