Skip to content

feat: add max height and "Show more" to user message bubbles#24003

Merged
TirmanSidhu merged 1 commit into
TirmanSidhu/claude-style-chat-scrollfrom
run-plan/msg-smart-spacer/pr-1
Apr 7, 2026
Merged

feat: add max height and "Show more" to user message bubbles#24003
TirmanSidhu merged 1 commit into
TirmanSidhu/claude-style-chat-scrollfrom
run-plan/msg-smart-spacer/pr-1

Conversation

@TirmanSidhu
Copy link
Copy Markdown
Contributor

@TirmanSidhu TirmanSidhu commented Apr 7, 2026

Summary

  • Add 150pt max collapsed height to user message bubbles
  • Long messages clipped with ghost Show more/Show less button
  • Assistant messages unaffected

Part of plan: user-msg-maxheight-smart-spacer.md (PR 1 of 3)


Open with Devin

@TirmanSidhu TirmanSidhu merged commit f96c2af into TirmanSidhu/claude-style-chat-scroll Apr 7, 2026
4 checks passed
@TirmanSidhu TirmanSidhu deleted the run-plan/msg-smart-spacer/pr-1 branch April 7, 2026 16:27
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 2 additional findings in Devin Review.

Open in Devin Review

Comment on lines +567 to +569
.background(GeometryReader { geo in
Color.clear.preference(key: IntrinsicHeightKey.self, value: geo.size.height)
})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 Uses GeometryReader in .background() for measurement, violating the project's pitfall guidelines

The new userMessageHeightWrapper at ChatBubble.swift:567-568 uses the GeometryReader-in-.background() pattern with a Color.clear + PreferenceKey wrapper to measure intrinsic content height. This is explicitly listed as a pitfall to avoid in clients/AGENTS.md:448: the pitfalls table says to use .onGeometryChange(for:body:action:) instead, which fires on initial layout and on changes automatically without needing the Color.clear wrapper, the IntrinsicHeightKey preference key, or the .onPreferenceChange handler. The entire IntrinsicHeightKey type (ChatBubble.swift:24-29) and the .onPreferenceChange call (ChatBubble.swift:585) could be eliminated by using .onGeometryChange(for: CGFloat.self) { $0.size.height } action: { userMessageIntrinsicHeight = $0 } on the content view directly.

Prompt for agents
The `userMessageHeightWrapper` method uses `GeometryReader` in `.background()` with a `Color.clear` preference key wrapper to measure the intrinsic content height. Per the project pitfalls table in clients/AGENTS.md, this pattern should be replaced with `.onGeometryChange(for:of:action:)` (macOS 14+ / iOS 17+).

The fix involves:
1. Remove the `IntrinsicHeightKey` PreferenceKey struct (lines 24-29) as it becomes unnecessary.
2. In `userMessageHeightWrapper`, replace the `.background(GeometryReader { ... })` modifier on `content()` (line 567-569) with `.onGeometryChange(for: CGFloat.self) { geo in geo.size.height } action: { height in userMessageIntrinsicHeight = height }`.
3. Remove the `.onPreferenceChange(IntrinsicHeightKey.self) { ... }` call on line 585, since the action closure in onGeometryChange handles the state update directly.

The resulting code in userMessageHeightWrapper would be:
  content()
      .onGeometryChange(for: CGFloat.self) { geo in geo.size.height } action: { height in userMessageIntrinsicHeight = height }
      .frame(maxHeight: isUserMessageExpanded ? nil : userMessageMaxCollapsedHeight, alignment: .top)
      .clipped()
Open in Devin Review

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant