Skip to content

fix(chat): Remove remaining FlexFrame anti-patterns from LazyVStack cell views #25947

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 16, 2026
Merged

fix(chat): Remove remaining FlexFrame anti-patterns from LazyVStack cell views #25947

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
0a84585
Remove remaining FlexFrame anti-patterns from LazyVStack cell views
devin-ai-integration[bot] Apr 15, 2026
bc5583f
Fix InlineVideoAttachmentView: use .overlay for save button instead o…
devin-ai-integration[bot] Apr 15, 2026
255496f
Fix review feedback: restore responsive maxWidth and fill card tap areas
devin-ai-integration[bot] Apr 15, 2026
6758e1f
Fix InlineChatErrorAlert: add Spacer to fill card width
devin-ai-integration[bot] Apr 15, 2026
fd3bfc8
Fix failedView indentation inside ZStack wrapper
devin-ai-integration[bot] Apr 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
22 changes: 13 additions & 9 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 @@ -108,19 +108,23 @@ struct CodePreviewView: View {
Group {
if isLong {
ScrollView {
HStack(spacing: 0) {
Text(displayCode)
.font(VFont.bodySmallDefault)
.foregroundStyle(VColor.contentSecondary)
Spacer(minLength: 0)
}
.padding(VSpacing.sm)
}
.frame(height: 120)
} else {
HStack(spacing: 0) {
Text(displayCode)
.font(VFont.bodySmallDefault)
.foregroundStyle(VColor.contentSecondary)
.frame(maxWidth: .infinity, alignment: .leading)
.padding(VSpacing.sm)
Spacer(minLength: 0)
}
.frame(height: 120)
} else {
Text(displayCode)
.font(VFont.bodySmallDefault)
.foregroundStyle(VColor.contentSecondary)
.frame(maxWidth: .infinity, alignment: .leading)
.padding(VSpacing.sm)
.padding(VSpacing.sm)
}
}
.background(VColor.surfaceOverlay.opacity(0.6))
Expand Down
30 changes: 16 additions & 14 deletions clients/macos/vellum-assistant/Features/Chat/MediaEmbeds/InlineAudioAttachmentView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,23 +90,25 @@ struct InlineAudioAttachmentView: View {
playPauseButton

// Center: filename + progress bar
VStack(alignment: .leading, spacing: 3) {
Text(attachment.filename)
.font(VFont.labelDefault)
.foregroundStyle(VColor.contentDefault)
.lineLimit(1)
.truncationMode(.middle)

if let failure {
Text(failure.userMessage)
.font(VFont.labelSmall)
.foregroundStyle(VColor.contentTertiary)
HStack(spacing: 0) {
VStack(alignment: .leading, spacing: 3) {
Text(attachment.filename)
.font(VFont.labelDefault)
.foregroundStyle(VColor.contentDefault)
.lineLimit(1)
} else {
progressBar
.truncationMode(.middle)

if let failure {
Text(failure.userMessage)
.font(VFont.labelSmall)
.foregroundStyle(VColor.contentTertiary)
.lineLimit(1)
} else {
progressBar
}
}
Spacer(minLength: 0)
}
.frame(maxWidth: .infinity, alignment: .leading)

// Right: time display or save button
if isHovering && (failure == nil || localFileURL != nil || cachedFileURL != nil) {
Expand Down
76 changes: 42 additions & 34 deletions clients/macos/vellum-assistant/Features/Chat/MediaEmbeds/InlineVideoAttachmentView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ struct InlineVideoAttachmentView: View {
}

var body: some View {
ZStack(alignment: .topTrailing) {
ZStack {
RoundedRectangle(cornerRadius: VRadius.md)
.fill(VColor.surfaceOverlay)
.overlay(
Expand All @@ -108,7 +108,8 @@ struct InlineVideoAttachmentView: View {
} else {
placeholderView
}

}
.overlay(alignment: .topTrailing) {
if failure == nil && !isLoading && isHovering {
Button(action: saveVideo) {
if isSaving {
Expand Down Expand Up @@ -146,6 +147,8 @@ struct InlineVideoAttachmentView: View {

private var placeholderView: some View {
ZStack {
Color.clear

if let thumbnailImage {
Image(nsImage: thumbnailImage)
.resizable()
Expand All @@ -165,7 +168,6 @@ struct InlineVideoAttachmentView: View {
.lineLimit(1)
}
}
.frame(maxWidth: .infinity, maxHeight: .infinity)
.contentShape(Rectangle())
.onTapGesture {
prepareAndPlay()
Comment thread
ashleeradka marked this conversation as resolved.
Expand All @@ -176,46 +178,52 @@ struct InlineVideoAttachmentView: View {
}

private var loadingView: some View {
VStack(spacing: VSpacing.sm) {
ProgressView()
.controlSize(.regular)
ZStack {
Color.clear

Text("Loading video...")
.font(VFont.labelDefault)
.foregroundStyle(VColor.contentSecondary)
VStack(spacing: VSpacing.sm) {
ProgressView()
.controlSize(.regular)

Text("Loading video...")
.font(VFont.labelDefault)
.foregroundStyle(VColor.contentSecondary)
}
}
.frame(maxWidth: .infinity, maxHeight: .infinity)
}

private func failedView(_ failure: VideoPlaybackFailure) -> some View {
VStack(spacing: VSpacing.xs) {
if case .port_missing = failure {
VIconView(.refreshCw, size: 20)
.foregroundStyle(VColor.contentSecondary)
} else {
VIconView(.triangleAlert, size: 20)
ZStack {
Color.clear

VStack(spacing: VSpacing.xs) {
if case .port_missing = failure {
VIconView(.refreshCw, size: 20)
.foregroundStyle(VColor.contentSecondary)
} else {
VIconView(.triangleAlert, size: 20)
.foregroundStyle(VColor.contentSecondary)
}

Text(failure.userMessage)
.font(VFont.labelDefault)
.foregroundStyle(VColor.contentSecondary)
}

Text(failure.userMessage)
.font(VFont.labelDefault)
.foregroundStyle(VColor.contentSecondary)

if case .port_missing = failure {
Text("Tap to retry")
.font(VFont.labelSmall)
.foregroundStyle(VColor.contentTertiary)
} else if case .invalid_media = failure {
Text("Tap to open externally")
.font(VFont.labelSmall)
.foregroundStyle(VColor.contentTertiary)
} else {
Text(hasRetriedOnce ? "Tap to open externally" : "Tap to retry")
.font(VFont.labelSmall)
.foregroundStyle(VColor.contentTertiary)
if case .port_missing = failure {
Text("Tap to retry")
.font(VFont.labelSmall)
.foregroundStyle(VColor.contentTertiary)
} else if case .invalid_media = failure {
Text("Tap to open externally")
.font(VFont.labelSmall)
.foregroundStyle(VColor.contentTertiary)
} else {
Text(hasRetriedOnce ? "Tap to open externally" : "Tap to retry")
.font(VFont.labelSmall)
.foregroundStyle(VColor.contentTertiary)
}
}
}
.frame(maxWidth: .infinity, maxHeight: .infinity)
.contentShape(Rectangle())
.onTapGesture {
handleFailedTileTap(failure)
Expand Down
2 changes: 0 additions & 2 deletions clients/macos/vellum-assistant/Features/Chat/MediaEmbeds/InlineVideoEmbedCard.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ struct InlineVideoEmbedCard: View {

stateContent
}
.frame(maxWidth: .infinity)
.frame(height: cardHeight)
.clipShape(RoundedRectangle(cornerRadius: VRadius.md))
.animation(.easeInOut(duration: 0.25), value: cardHeight)
Expand Down Expand Up @@ -116,7 +115,6 @@ struct InlineVideoEmbedCard: View {

private var fallbackPlaceholder: some View {
VColor.auxBlack.opacity(0.8)
.frame(maxWidth: .infinity, maxHeight: .infinity)
}

/// Single view for both .initializing and .playing so SwiftUI preserves
Expand Down
21 changes: 12 additions & 9 deletions clients/shared/Features/Chat/GuardianDecisionBubble.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,23 +131,26 @@ public struct GuardianDecisionBubble: View {
Group {
if previewIsLong {
ScrollView {
HStack(spacing: 0) {
Text(preview)
.font(.system(size: 12, design: .monospaced))
.foregroundStyle(VColor.contentSecondary)
.textSelection(.enabled)
Spacer(minLength: 0)
}
}
.frame(height: 120)
} else {
HStack(spacing: 0) {
Text(preview)
.font(.system(size: 12, design: .monospaced))
.foregroundStyle(VColor.contentSecondary)
.frame(maxWidth: .infinity, alignment: .leading)
.textSelection(.enabled)
Spacer(minLength: 0)
}
.frame(height: 120)
} else {
Text(preview)
.font(.system(size: 12, design: .monospaced))
.foregroundStyle(VColor.contentSecondary)
.frame(maxWidth: .infinity, alignment: .leading)
.textSelection(.enabled)
}
}
.padding(VSpacing.sm)
.frame(maxWidth: .infinity, alignment: .leading)
.background(
RoundedRectangle(cornerRadius: VRadius.sm)
.fill(VColor.surfaceOverlay)
Expand Down
23 changes: 14 additions & 9 deletions clients/shared/Features/Chat/InlineChatErrorAlert.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,19 +127,23 @@ public struct InlineChatErrorAlert: View {
Group {
if detailIsLong {
ScrollView {
HStack(spacing: 0) {
Text(details)
.font(.system(size: 11, design: .monospaced))
.foregroundStyle(VColor.contentSecondary)
.textSelection(.enabled)
Spacer(minLength: 0)
}
}
.frame(height: 160)
} else {
HStack(spacing: 0) {
Text(details)
.font(.system(size: 11, design: .monospaced))
.foregroundStyle(VColor.contentSecondary)
.frame(maxWidth: .infinity, alignment: .leading)
.textSelection(.enabled)
Spacer(minLength: 0)
}
.frame(height: 160)
} else {
Text(details)
.font(.system(size: 11, design: .monospaced))
.foregroundStyle(VColor.contentSecondary)
.frame(maxWidth: .infinity, alignment: .leading)
.textSelection(.enabled)
}
}
.padding(.horizontal, VSpacing.sm)
Expand Down Expand Up @@ -170,8 +174,9 @@ public struct InlineChatErrorAlert: View {
.padding(.leading, VSpacing.md)
.padding(.trailing, VSpacing.lg)
.padding(.vertical, VSpacing.md)

Spacer(minLength: 0)
Copy link
Copy Markdown
Contributor Author

@devin-ai-integration devin-ai-integration Bot Apr 16, 2026

Choose a reason for hiding this comment

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

🚩 HStack+Spacer width proposal difference for long wrapping text

The replacement of .frame(maxWidth: .infinity, alignment: .leading) with HStack(spacing: 0) { Content; Spacer(minLength: 0) } has a subtle difference in how SwiftUI distributes width among flexible children. With .frame(maxWidth: .infinity), the content view was the sole flexible child and received all remaining width. With the Spacer pattern, the content view theoretically competes with the Spacer for space. In practice, SwiftUI's layout algorithm sizes less-flexible children first and Spacer absorbs remaining space, so for most content (especially text that wraps), the behavior is equivalent. However, for InlineChatErrorAlert.swift:178 specifically, the Spacer(minLength: 0) sits alongside a padded VStack containing error messages. If an error message's ideal single-line width is very large, the VStack could receive a narrower proposal than before, causing slightly different text wrapping. This is worth a quick visual check on long error messages with debug details to confirm the card still looks correct.

Open in Devin Review

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

Copy link
Copy Markdown
Contributor Author

@devin-ai-integration devin-ai-integration Bot Apr 16, 2026

Choose a reason for hiding this comment

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

Not a regression — the VStack's text children have .fixedSize(horizontal: false, vertical: true), which means they accept any proposed width and wrap to fit. HStack proposes the full remaining width (after the 3pt accent bar) to the VStack first because it's the less-flexible child (it has content-dependent sizing), then the Spacer(minLength: 0) absorbs whatever space the VStack doesn't claim. In practice, wrapping text always claims the full proposed width, so the Spacer gets 0pt — identical to the previous .frame(maxWidth: .infinity) behavior.

Worth confirming visually with long error messages in Xcode (flagged in the human review checklist), but not a functional regression.

}
.frame(maxWidth: .infinity, alignment: .leading)
.background(
RoundedRectangle(cornerRadius: VRadius.md)
.fill(accentColor.opacity(0.06))
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Expand Down