Skip to content
Closed
Changes from all commits
Commits
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
77 changes: 47 additions & 30 deletions clients/shared/Features/Chat/MediaEmbeds/InlineImageEmbedView.swift

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

🚩 Pre-existing accessibility gap: onTapGesture without VoiceOver traits

The .onTapGesture at lines 87-93 opens the image URL in the browser but lacks .accessibilityAddTraits(.isButton), .accessibilityLabel(), and .accessibilityAction {} required by the project's accessibility rules in clients/AGENTS.md. This is a pre-existing issue not introduced by this PR (the tap gesture code is unchanged), but worth noting since the view body was restructured. A follow-up could add proper accessibility traits so VoiceOver users can activate the image link.

Open in Devin Review

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

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.

Agreed this is a valid accessibility gap, but as noted it's entirely pre-existing and unrelated to this one-character fix. Worth tracking for a future accessibility pass across inline media views.

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.

Agreed — this is pre-existing and out of scope for this PR. The .onTapGesture should have .accessibilityAddTraits(.isButton) and an .accessibilityLabel for VoiceOver. Good candidate for a follow-up accessibility pass.

Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,34 @@ import AppKit
/// image at once.
///
/// Tapping the image opens the URL in the user's default browser.
private struct ImageHeightKey: PreferenceKey {
static var defaultValue: CGFloat = 0
static func reduce(value: inout CGFloat, nextValue: () -> CGFloat) {
value = nextValue()

/// Caps content height at a maximum using the Layout protocol (O(1)).
/// Unlike `.frame(maxHeight:)` which creates `_FlexFrameLayout` with its
/// O(n × depth) alignment cascade, this measures the child, caps the
/// reported height, and re-measures with the capped height so the child
/// can adjust its width (e.g. to preserve aspect ratio).
/// Unlike GeometryReader + PreferenceKey, this resolves in a single layout
/// pass — no @State round-trip, so no first-render flicker.
private struct HeightCapLayout: Layout {
let cap: CGFloat

func sizeThatFits(proposal: ProposedViewSize, subviews: Subviews, cache: inout ()) -> CGSize {
guard let child = subviews.first else { return .zero }
let childSize = child.sizeThatFits(proposal)
guard childSize.height > cap else { return childSize }
// Re-measure with capped height so the child can adjust its width
// (e.g. a resizable image with .fit will narrow to preserve aspect ratio).
let cappedSize = child.sizeThatFits(ProposedViewSize(width: proposal.width, height: cap))
return CGSize(width: cappedSize.width, height: min(cap, cappedSize.height))
}

func placeSubviews(in bounds: CGRect, proposal: ProposedViewSize, subviews: Subviews, cache: inout ()) {
guard let child = subviews.first else { return }
child.place(
at: bounds.origin,
anchor: .topLeading,
proposal: ProposedViewSize(width: bounds.width, height: bounds.height)
)
}
}

Expand All @@ -32,38 +56,31 @@ public struct InlineImageEmbedView: View {
/// Flipped to `true` by `onAppear`; prevents eager network fetches
/// for images that are off-screen in long chat histories.
@State private var isVisible = false
/// Tracks intrinsic image height for capping without `_FlexFrameLayout`.
@State private var imageIntrinsicHeight: CGFloat = 0

public var body: some View {
Group {
if isVisible {
AsyncImage(url: url) { phase in
switch phase {
case .empty:
placeholderSkeleton
.overlay(ProgressView())
case .success(let image):
image
.resizable()
.aspectRatio(contentMode: .fit)
case .failure:
EmptyView()
@unknown default:
EmptyView()
HeightCapLayout(cap: 300) {
Group {
if isVisible {
AsyncImage(url: url) { phase in
switch phase {
case .empty:
placeholderSkeleton
.overlay(ProgressView())
case .success(let image):
image
.resizable()
.aspectRatio(contentMode: .fit)
case .failure:
EmptyView()
@unknown default:
EmptyView()
}
}
} else {
placeholderSkeleton
}
} else {
placeholderSkeleton
}
}
.background(
GeometryReader { geo in
Color.clear.preference(key: ImageHeightKey.self, value: geo.size.height)
}
)
.onPreferenceChange(ImageHeightKey.self) { imageIntrinsicHeight = $0 }
.frame(height: imageIntrinsicHeight > 300 ? 300 : nil)
.clipped()
.clipShape(RoundedRectangle(cornerRadius: 8))
.onAppear { isVisible = true }
Expand Down