Skip to content

Fix ScrollView.frame(maxHeight:) anti-pattern in 5 LazyVStack cell files #24091

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
ashleeradka merged 5 commits into from
Apr 7, 2026
Merged

Fix ScrollView.frame(maxHeight:) anti-pattern in 5 LazyVStack cell files #24091

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6fa370a
fix: replace ScrollView.frame(maxHeight:) with definite heights in 4 …
devin-ai-integration[bot] Apr 7, 2026
c45dfe6
fix: apply two-path pattern to VDiffView for short diffs
devin-ai-integration[bot] Apr 7, 2026
24d54af
fix: add single-line charThreshold guard to all three two-path views
devin-ai-integration[bot] Apr 7, 2026
2563c5d
fix: apply two-path pattern to CodeBlockView for capped height in Laz…
devin-ai-integration[bot] Apr 7, 2026
6eb4f24
fix: use conservative divisor for VDiffView threshold, compute CodeBl…
devin-ai-integration[bot] Apr 7, 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
26 changes: 19 additions & 7 deletions clients/macos/vellum-assistant/Features/Chat/ChatWidgetViews.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,14 +103,26 @@ struct CodePreviewView: View {
let code: String

var body: some View {
ScrollView {
Text(displayCode)
.font(VFont.bodySmallDefault)
.foregroundStyle(VColor.contentSecondary)
.frame(maxWidth: .infinity, alignment: .leading)
.padding(VSpacing.sm)
let lineCount = displayCode.utf8.reduce(1) { $0 + ($1 == 0x0A ? 1 : 0) }
let isLong = lineCount > 7 || (lineCount == 1 && displayCode.utf8.count > 50_000)
Group {
if isLong {
ScrollView {
Text(displayCode)
.font(VFont.bodySmallDefault)
.foregroundStyle(VColor.contentSecondary)
.frame(maxWidth: .infinity, alignment: .leading)
.padding(VSpacing.sm)
}
.frame(height: 120)
} else {
Text(displayCode)
.font(VFont.bodySmallDefault)
.foregroundStyle(VColor.contentSecondary)
.frame(maxWidth: .infinity, alignment: .leading)
.padding(VSpacing.sm)
}
}
.frame(maxHeight: 120)
.background(VColor.surfaceOverlay.opacity(0.6))
.clipShape(RoundedRectangle(cornerRadius: VRadius.sm))
.overlay(
Expand Down
50 changes: 36 additions & 14 deletions clients/macos/vellum-assistant/Features/Chat/MarkdownSegmentView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,14 @@ private struct CodeBlockView: View, Equatable {
return ceil(lm.defaultLineHeight(for: VFont.nsMono))
}()

/// Maximum height for long code blocks inside LazyVStack cells.
/// Content exceeding this is vertically scrollable.
private static let maxCodeBlockHeight: CGFloat = 400

/// Line threshold derived from maxCodeBlockHeight / codeLineHeight.
/// Content above this count takes the capped-height ScrollView path.
private static let lineThreshold: Int = Int(maxCodeBlockHeight / codeLineHeight)

@devin-ai-integration devin-ai-integration Bot Apr 7, 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.

🟡 CodeBlockView lineThreshold ignores padding, causing short-path height to exceed maxCodeBlockHeight

The lineThreshold at MarkdownSegmentView.swift:788 is computed as Int(maxCodeBlockHeight / codeLineHeight) but doesn't account for the VSpacing.sm * 2 (16pt) padding added in the short-path height formula at line 835: CGFloat(codeLineCount) * Self.codeLineHeight + VSpacing.sm * 2. For code blocks at exactly lineThreshold lines, the short-path height is lineThreshold * codeLineHeight + 16, which exceeds maxCodeBlockHeight (400pt) by up to 16pt. For example, if codeLineHeight = 16pt, then lineThreshold = 25 and a 25-line block renders at 416pt (short path), while a 26-line block renders at 400pt (long path) — a visual discontinuity where adding one line makes the view shorter.

Suggested change
private static let lineThreshold: Int = Int(maxCodeBlockHeight / codeLineHeight)
private static let lineThreshold: Int = Int((maxCodeBlockHeight - VSpacing.sm * 2) / codeLineHeight)
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


static func == (lhs: CodeBlockView, rhs: CodeBlockView) -> Bool {
lhs.language == rhs.language
&& lhs.code == rhs.code
Expand Down Expand Up @@ -806,21 +814,35 @@ private struct CodeBlockView: View, Equatable {
.padding(.top, VSpacing.xs)
}

// Pre-compute height from line count so the ScrollView has a
// definite size during LazyVStack's sizeThatFits pass, avoiding
// expensive Core Text measurement for off-screen cells.
let codeLineCount = code.components(separatedBy: "\n").count
let codeBlockHeight = CGFloat(codeLineCount) * Self.codeLineHeight + VSpacing.sm * 2

ScrollView(.horizontal, showsIndicators: false) {
Text(code)
.font(.custom("DMMono-Regular", size: 13))
.foregroundStyle(textColor)
.textSelection(.enabled)
.fixedSize(horizontal: true, vertical: true)
.padding(VSpacing.sm)
let codeLineCount = code.utf8.reduce(1) { $0 + ($1 == 0x0A ? 1 : 0) }
let isLong = codeLineCount > Self.lineThreshold || (codeLineCount == 1 && code.utf8.count > 50_000)

if isLong {
// Long code: both horizontal and vertical scroll, capped height.
// .frame(height:) compiles to _FixedSizeLayout — O(1), never
// measures children. Safe inside LazyVStack cells.
ScrollView([.horizontal, .vertical], showsIndicators: true) {
Text(code)
.font(.custom("DMMono-Regular", size: 13))
.foregroundStyle(textColor)
.textSelection(.enabled)
.fixedSize(horizontal: true, vertical: true)
.padding(VSpacing.sm)
}
.frame(height: Self.maxCodeBlockHeight)
} else {
// Short code: horizontal scroll only, natural height.
let codeBlockHeight = CGFloat(codeLineCount) * Self.codeLineHeight + VSpacing.sm * 2
ScrollView(.horizontal, showsIndicators: false) {
Text(code)
.font(.custom("DMMono-Regular", size: 13))
.foregroundStyle(textColor)
.textSelection(.enabled)
.fixedSize(horizontal: true, vertical: true)
.padding(VSpacing.sm)
}
.frame(height: codeBlockHeight)
}
.frame(height: codeBlockHeight)
}
.optionalMaxWidth(maxContentWidth)
.background(codeBackgroundColor)
Expand Down
10 changes: 8 additions & 2 deletions clients/shared/DesignSystem/Components/Display/VDiffView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,14 @@ public struct VDiffView: View {
public var body: some View {
let lines = text.split(separator: "\n", omittingEmptySubsequences: false)
if let maxHeight {
diffScrollView(lines: lines, axes: [.horizontal, .vertical])
.frame(maxHeight: maxHeight)
let lineThreshold = Int(maxHeight / 18)
if lines.count > lineThreshold {
diffScrollView(lines: lines, axes: [.horizontal, .vertical])
.frame(height: maxHeight)
} else {
diffScrollView(lines: lines, axes: .horizontal)
.fixedSize(horizontal: false, vertical: true)
Comment thread
ashleeradka marked this conversation as resolved.
}
} else {
diffScrollView(lines: lines, axes: .horizontal)
.fixedSize(horizontal: false, vertical: true)
Expand Down
26 changes: 19 additions & 7 deletions clients/shared/Features/Chat/GuardianDecisionBubble.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,26 @@ public struct GuardianDecisionBubble: View {

// Command preview code block
if let preview = decision.commandPreview, !preview.isEmpty {
ScrollView {
Text(preview)
.font(.system(size: 12, design: .monospaced))
.foregroundStyle(VColor.contentSecondary)
.frame(maxWidth: .infinity, alignment: .leading)
.textSelection(.enabled)
let previewLineCount = preview.utf8.reduce(1) { $0 + ($1 == 0x0A ? 1 : 0) }
let previewIsLong = previewLineCount > 7 || (previewLineCount == 1 && preview.utf8.count > 50_000)
Group {
if previewIsLong {
ScrollView {
Text(preview)
.font(.system(size: 12, design: .monospaced))
.foregroundStyle(VColor.contentSecondary)
.frame(maxWidth: .infinity, alignment: .leading)
.textSelection(.enabled)
}
.frame(height: 120)
} else {
Text(preview)
.font(.system(size: 12, design: .monospaced))
.foregroundStyle(VColor.contentSecondary)
.frame(maxWidth: .infinity, alignment: .leading)
.textSelection(.enabled)
}
}
.frame(maxHeight: 120)
.padding(VSpacing.sm)
.frame(maxWidth: .infinity, alignment: .leading)
.background(
Expand Down
26 changes: 19 additions & 7 deletions clients/shared/Features/Chat/InlineChatErrorAlert.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,26 @@ public struct InlineChatErrorAlert: View {
.padding(.horizontal, VSpacing.sm)
.padding(.top, VSpacing.xs)

ScrollView {
Text(details)
.font(.system(size: 11, design: .monospaced))
.foregroundStyle(VColor.contentSecondary)
.frame(maxWidth: .infinity, alignment: .leading)
.textSelection(.enabled)
let detailLineCount = details.utf8.reduce(1) { $0 + ($1 == 0x0A ? 1 : 0) }
let detailIsLong = detailLineCount > 10 || (detailLineCount == 1 && details.utf8.count > 50_000)
Group {
if detailIsLong {
ScrollView {
Text(details)
.font(.system(size: 11, design: .monospaced))
.foregroundStyle(VColor.contentSecondary)
.frame(maxWidth: .infinity, alignment: .leading)
.textSelection(.enabled)
}
.frame(height: 160)
} else {
Text(details)
.font(.system(size: 11, design: .monospaced))
.foregroundStyle(VColor.contentSecondary)
.frame(maxWidth: .infinity, alignment: .leading)
.textSelection(.enabled)
}
}
.frame(maxHeight: 160)
.padding(.horizontal, VSpacing.sm)
.padding(.bottom, VSpacing.sm)
}
Expand Down