Add support for rendering messages from the top of the list#1530
Add support for rendering messages from the top of the list#1530nuno-vieira wants to merge 26 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
π WalkthroughWalkthroughChangesAdds a public Top-aligned message list
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AppConfigurationView
participant AppConfiguration
participant MessageListConfig
participant MessageListView
participant ScrollView
AppConfigurationView->>AppConfiguration: persist messagesStartAtTheTop
AppConfiguration->>MessageListConfig: build configured message-list settings
MessageListConfig->>MessageListView: enable top-alignment mode
MessageListView->>ScrollView: measure content and apply top-aligned layout
Suggested reviewers: π₯ Pre-merge checks | β 4 | β 1β Failed checks (1 warning)
β Passed checks (4 passed)
β¨ Finishing Touchesπ§ͺ Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
When enabled and the messages are shorter than the visible list, they are laid out from the top instead of the bottom, matching the UIKit SDK's Components.shouldMessagesStartAtTheTop. Once the messages grow past the visible height, the list behaves like a regular inverted message list again.
16ad1f2 to
8c81f31
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
π€ Prompt for all review comments with 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.
Inline comments:
In `@Sources/StreamChatSwiftUI/ChatMessageList/MessageListView.swift`:
- Around line 340-345: Update the message item transition near the list itemβs
.id in MessageListView so it uses the dynamic top-aligned state,
topAlignedMinHeight, rather than the static
messageListConfig.shouldMessagesStartAtTheTop flag. Preserve identity
transitions only while top-aligned mode is active, and use the normal opacity
transition once content overflows and topAlignedMinHeight is nil.
πͺ Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
βΉοΈ Review info
βοΈ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c6d85d0-d03e-45f7-98d4-ad1bf79e42c3
β Files ignored due to path filters (3)
StreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/MessageListView_Tests/test_messageListView_shouldMessagesStartAtTheTop_fewMessages.1.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/MessageListView_Tests/test_messageListView_shouldMessagesStartAtTheTop_fewMessagesWithBottomInset.1.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/MessageListView_Tests/test_messageListView_shouldMessagesStartAtTheTop_manyMessages.1.pngis excluded by!**/*.png
π Files selected for processing (7)
CHANGELOG.mdDemoAppSwiftUI/AppConfiguration/AppConfiguration.swiftDemoAppSwiftUI/AppConfiguration/AppConfigurationView.swiftSources/StreamChatSwiftUI/ChatChannel/Utils/ChatChannelHelpers.swiftSources/StreamChatSwiftUI/ChatMessageList/MessageListConfig.swiftSources/StreamChatSwiftUI/ChatMessageList/MessageListView.swiftStreamChatSwiftUITests/Tests/ChatChannel/MessageListView_Tests.swift
Add top padding to keep the first message clear of the navigation bar when the list is top-aligned, and extract the scroll anchors into named topAnchorView/bottomAnchorView view properties.
Rename topAnchor/bottomAnchor to topAnchorId/bottomAnchorId, move them back next to scrollAreaId, and document bottomAnchorView to match topAnchorView.
932d069 to
2a779ce
Compare
Key the message row's transition off the dynamic top-aligned regime instead of the static shouldMessagesStartAtTheTop flag, so the default insertion/removal fade returns once a channel's messages grow past the visible height.
There was a problem hiding this comment.
Actionable comments posted: 2
π§Ή Nitpick comments (2)
Sources/StreamChatSwiftUI/ChatMessageList/MessageListView.swift (2)
343-347: π Maintainability & Code Quality | π΅ Trivial | π€ Low valueAllow
messageListContentHeightto reset cleanly.Using
if let valuedrops thenildefault value that the preference key emits when the tracking modifier is disabled dynamically. While harmless right now (becauseresolveTopAlignedMinHeightguards against the config flag being false), removing the optional binding maintains cleaner state hygiene.β»οΈ Proposed fix
- .onPreferenceChange(MessageListContentHeightPreferenceKey.self) { value in - if let value, value != messageListContentHeight { - messageListContentHeight = value - } - } + .onPreferenceChange(MessageListContentHeightPreferenceKey.self) { value in + if value != messageListContentHeight { + messageListContentHeight = value + } + }π€ 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/StreamChatSwiftUI/ChatMessageList/MessageListView.swift` around lines 343 - 347, Update the onPreferenceChange handler for MessageListContentHeightPreferenceKey to assign the emitted optional value directly to messageListContentHeight, allowing it to reset to nil when tracking is disabled; retain the existing change guard only if it supports optional equality without preventing that reset.
596-601: π Maintainability & Code Quality | π΅ Trivial | π€ Low valueRemove unused
topAnchorView.
topAnchorIdandtopAnchorVieware defined and placed in the layout, but they are never referenced for scrolling or any other logic. Since top alignment is achieved purely through layout modifiers, this view is unused and can be removed.β»οΈ Proposed fix
- private var topAnchorView: some View { - Color.clear - .frame(height: 0) - .id(topAnchorId) - .accessibilityHidden(true) - }You can also safely remove the
topAnchorIddeclaration at line 82 and thetopAnchorViewusage inside theLazyVStackat line 211.π€ 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/StreamChatSwiftUI/ChatMessageList/MessageListView.swift` around lines 596 - 601, Remove the unused topAnchorView property, its topAnchorView placement in the LazyVStack, and the associated topAnchorId declaration; leave the existing layout modifiers and top-alignment behavior unchanged.
π€ Prompt for all review comments with 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.
Inline comments:
In `@CHANGELOG.md`:
- Line 6: Update the ββ
Addedβ heading in CHANGELOG.md from level three to level
two so it follows the top-level β# Upcomingβ heading correctly.
In `@Sources/StreamChatSwiftUI/ChatMessageList/MessageListView.swift`:
- Around line 590-592: Update the overflow check in the top-aligned layout logic
to compare contentHeight directly against containerHeight. Remove the
bottomInset subtraction while preserving the existing nil-return behavior when
contentHeight exceeds the available container height.
---
Nitpick comments:
In `@Sources/StreamChatSwiftUI/ChatMessageList/MessageListView.swift`:
- Around line 343-347: Update the onPreferenceChange handler for
MessageListContentHeightPreferenceKey to assign the emitted optional value
directly to messageListContentHeight, allowing it to reset to nil when tracking
is disabled; retain the existing change guard only if it supports optional
equality without preventing that reset.
- Around line 596-601: Remove the unused topAnchorView property, its
topAnchorView placement in the LazyVStack, and the associated topAnchorId
declaration; leave the existing layout modifiers and top-alignment behavior
unchanged.
πͺ Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
βΉοΈ Review info
βοΈ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 29d40160-1af8-4425-91fd-e2274501ce50
β Files ignored due to path filters (3)
StreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/MessageListView_Tests/test_messageListView_shouldMessagesStartAtTheTop_fewMessages.1.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/MessageListView_Tests/test_messageListView_shouldMessagesStartAtTheTop_fewMessagesWithBottomInset.1.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/MessageListView_Tests/test_messageListView_shouldMessagesStartAtTheTop_manyMessages.1.pngis excluded by!**/*.png
π Files selected for processing (7)
CHANGELOG.mdDemoAppSwiftUI/AppConfiguration/AppConfiguration.swiftDemoAppSwiftUI/AppConfiguration/AppConfigurationView.swiftSources/StreamChatSwiftUI/ChatChannel/Utils/ChatChannelHelpers.swiftSources/StreamChatSwiftUI/ChatMessageList/MessageListConfig.swiftSources/StreamChatSwiftUI/ChatMessageList/MessageListView.swiftStreamChatSwiftUITests/Tests/ChatChannel/MessageListView_Tests.swift
π§ Files skipped from review as they are similar to previous changes (5)
- Sources/StreamChatSwiftUI/ChatMessageList/MessageListConfig.swift
- Sources/StreamChatSwiftUI/ChatChannel/Utils/ChatChannelHelpers.swift
- DemoAppSwiftUI/AppConfiguration/AppConfiguration.swift
- DemoAppSwiftUI/AppConfiguration/AppConfigurationView.swift
- StreamChatSwiftUITests/Tests/ChatChannel/MessageListView_Tests.swift
martinmitrevski
left a comment
There was a problem hiding this comment.
fine with the code changes if it's the only way - but @testableapple should run a solid QA. Also, would be nice to measure the performance, we put another geometry reader, plus some additional ifs, anchors etc.
The topmost message's spacing was keyed off the dynamically resolved top-aligned height, which itself depended on the measured content height, causing the content height preference to update multiple times per frame. Key the spacing off the static configuration flag instead to break the loop.
Move the combined date/separator overlay spacing and top-of-list spacing out of an inline nested expression into a named helper for readability.
laevandus
left a comment
There was a problem hiding this comment.
I did some testing and works well. The initial shift of the whole message list is fixed as well. Only other alternative I could think of was dropping flipped handling for bottom alignment, but then many VStacks and message index handling logic needs to be updated as well (messagingOrder would change and then index logic does not match anymore). That seems like a much bigger change than what we have here. Therefore, I feel like we should go with this implementation if we don't find any other layout issues during testing.
martinmitrevski
left a comment
There was a problem hiding this comment.
LGTM β important one for @testableapple, both cases should be tested - starting from top and bottom
When messages fit on screen there is nothing to scroll, so keep the scrollbar hidden to match the UIKit message list behavior.
Insert a trailing spacer in the flipped stack so it becomes leading space under the navigation bar, keeping the time bubble from sitting flush against it.
a32eab5 to
81caf67
Compare
Replace the fits/overflow preference regime with a fill + UIKit clamp that pins short content without feeding layout feedback into SwiftUI, and latch insert animations once the list overflows to avoid boundary freezes.
This reverts commit 9cf7070.
Keep the minHeight fill approach, but never gate GeometryReader height on fits, latch overflow animations/indicators one-way, skip auto-scrollTo newest while fitting, and defer content-height preference updates.
Reinstate the UIKit scroll clamp that pins short (fitting) content to the visual top, but never disable scrolling or bouncing so the list can always be dragged. Remove the now-unused content-height preference key.
Replace the UIKit scroll-clamp bridge, forced scroll bounce, and top-aligned regime latches with a pure-SwiftUI fill. Short message lists now rest naturally at the top instead of being pinned by a UIScrollView bridge, removing the freeze and layout feedback loops. Also skip message insertion/removal animations while messages start at the top, so sending a message no longer shifts the whole list. Re-record the top-aligned snapshots to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This reverts commit d3737ed.
Public Interface @MainActor public final class MessageListConfig
-
+ public let shouldMessagesStartAtTheTop: Bool
-
+
- public init(messageListType: MessageListType = .messaging,typingIndicatorPlacement: TypingIndicatorPlacement = .automatic,groupMessages: Bool = true,messageDisplayOptions: MessageDisplayOptions = MessageDisplayOptions(),messagePaddings: MessagePaddings = MessagePaddings(),dateIndicatorPlacement: DateIndicatorPlacement = .messageList,pageSize: Int = 25,messagePopoverEnabled: Bool = true,doubleTapOverlayEnabled: Bool = false,becomesFirstResponderOnOpen: Bool = false,resignsFirstResponderOnScrollDown: Bool = true,updateChannelsFromMessageList: Bool = false,maxTimeIntervalBetweenMessagesInGroup: TimeInterval = 60,cacheSizeOnChatDismiss: Int = 1024 * 1024 * 100,iPadSplitViewEnabled: Bool = true,scrollingAnchor: UnitPoint = .center,showNewMessagesSeparator: Bool = true,highlightMessageWhenJumping: Bool = true,handleTabBarVisibility: Bool = true,messageListAlignment: MessageListAlignment = .standard,uniqueReactionsEnabled: Bool = false,localLinkDetectionEnabled: Bool = true,isMessageEditedLabelEnabled: Bool = true,markdownSupportEnabled: Bool = false,userBlockingEnabled: Bool = true,bouncedMessagesAlertActionsEnabled: Bool = true,skipEditedMessageLabel: @escaping (ChatMessage) -> Bool = { _ in false },draftMessagesEnabled: Bool = true,hidesCommandsOverlayOnMessageListTap: Bool = true,hidesAttachmentsPickersOnMessageListTap: Bool = true,attachmentPreviewWidth: CGFloat = 256,videoAttachmentCachingPolicy: VideoAttachmentCachingPolicy? = nil,navigationBarDisplayMode: NavigationBarItem.TitleDisplayMode = .inline,supportedMessageActions: @escaping @MainActor (SupportedMessageActionsOptions) -> [MessageAction] = { MessageAction.defaultActions(for: $0) })
+
+ public init(messageListType: MessageListType = .messaging,typingIndicatorPlacement: TypingIndicatorPlacement = .automatic,groupMessages: Bool = true,messageDisplayOptions: MessageDisplayOptions = MessageDisplayOptions(),messagePaddings: MessagePaddings = MessagePaddings(),dateIndicatorPlacement: DateIndicatorPlacement = .messageList,pageSize: Int = 25,messagePopoverEnabled: Bool = true,doubleTapOverlayEnabled: Bool = false,becomesFirstResponderOnOpen: Bool = false,resignsFirstResponderOnScrollDown: Bool = true,updateChannelsFromMessageList: Bool = false,maxTimeIntervalBetweenMessagesInGroup: TimeInterval = 60,cacheSizeOnChatDismiss: Int = 1024 * 1024 * 100,iPadSplitViewEnabled: Bool = true,scrollingAnchor: UnitPoint = .center,showNewMessagesSeparator: Bool = true,highlightMessageWhenJumping: Bool = true,handleTabBarVisibility: Bool = true,messageListAlignment: MessageListAlignment = .standard,uniqueReactionsEnabled: Bool = false,localLinkDetectionEnabled: Bool = true,isMessageEditedLabelEnabled: Bool = true,markdownSupportEnabled: Bool = false,userBlockingEnabled: Bool = true,bouncedMessagesAlertActionsEnabled: Bool = true,skipEditedMessageLabel: @escaping (ChatMessage) -> Bool = { _ in false },draftMessagesEnabled: Bool = true,hidesCommandsOverlayOnMessageListTap: Bool = true,hidesAttachmentsPickersOnMessageListTap: Bool = true,attachmentPreviewWidth: CGFloat = 256,videoAttachmentCachingPolicy: VideoAttachmentCachingPolicy? = nil,navigationBarDisplayMode: NavigationBarItem.TitleDisplayMode = .inline,supportedMessageActions: @escaping @MainActor (SupportedMessageActionsOptions) -> [MessageAction] = { MessageAction.defaultActions(for: $0) },shouldMessagesStartAtTheTop: Bool = false) |
SDK Size
|
There was a problem hiding this comment.
Actionable comments posted: 1
π€ Prompt for all review comments with 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.
Inline comments:
In `@Sources/StreamChatSwiftUI/ChatChannel/ChatChannelViewModel.swift`:
- Around line 812-820: The insert/remove suppression in the change-handling
switch should apply only while the channel is currently using the few-message
top-aligned layout, not whenever
utils.messageListConfig.shouldMessagesStartAtTheTop is enabled. Update the guard
around the .insert and .remove cases to use the existing current-layout or
message-count condition that identifies that eager top-aligned regime,
preserving animations after the list transitions to the normal lazy/inverted
layout.
πͺ Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
βΉοΈ Review info
βοΈ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 235a956a-1dfc-49f1-84c2-4d102104a265
β Files ignored due to path filters (3)
StreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/MessageListView_Tests/test_messageListView_shouldMessagesStartAtTheTop_fewMessages.1.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/MessageListView_Tests/test_messageListView_shouldMessagesStartAtTheTop_fewMessagesWithBottomInset.1.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/MessageListView_Tests/test_messageListView_shouldMessagesStartAtTheTop_manyMessages.1.pngis excluded by!**/*.png
π Files selected for processing (4)
DemoAppSwiftUI/AppConfiguration/AppConfiguration.swiftDemoAppSwiftUI/AppConfiguration/AppConfigurationView.swiftSources/StreamChatSwiftUI/ChatChannel/ChatChannelViewModel.swiftSources/StreamChatSwiftUI/ChatMessageList/MessageListView.swift
π§ Files skipped from review as they are similar to previous changes (2)
- DemoAppSwiftUI/AppConfiguration/AppConfiguration.swift
- DemoAppSwiftUI/AppConfiguration/AppConfigurationView.swift
StreamChatSwiftUI XCSize
|
|



π Issue Links
Resolves IOS-1058
π― Goal
Add support for rendering messages from the top of the list.
π Summary
MessageListConfig.shouldMessagesStartAtTheTop(defaults tofalse, fully opt-in)π Implementation
minHeightequal to the available list height and bottom-aligned; after the list's upside-down flip this places the messages at the topGeometryReaderwrapping the list content, so the top-aligned frame never lags behind the real viewport size on first layoutminHeightclamp has no effect by construction, so the list naturally degrades to the regular inverted behavior with no extra state to trackπ§· Scroll stability while rubber-banding
With few messages, dragging the top-aligned list up rubber-bands the rows under the navigation bar. When the rows are hosted in a
LazyVStack, that overscroll triggers a layout invalidation mid-gesture: the scroll view clamps the drag and the messages visibly jump back. The lazy container was isolated empirically as the cause β the jump reproduces with every scroll callback and preference handler in the list disabled, only with two or more messages, and never with an eager container. The exact trigger is internal to the lazy layout (it sizes and places children relative to the visible region, and an overscrolled, flipped, height-clamped viewport puts that machinery in an edge case), so the safe conclusion is behavioral: eager rendering of the visible rows keeps the layout stable during rubber-banding; a lazy one doesn't. Two solutions were explored; both fix the jump and both were validated with the same drag/hold E2E scenarios (including live message events arriving mid-drag).1. Container swap β currently used
The message stack renders as an eager
VStackwhile top-aligned with fewer than 20 messages, and as the regularLazyVStackotherwise. 20 messages always overflow the viewport, so beyond that top alignment (and the jump) is irrelevant.2. Eager chunk inside the lazy stack β explored and reverted (
d3737ed7, reverted inba0d5e85)The container stays a
LazyVStackalways; the newest 20 rows are grouped into a single plainVStackchild, with older history remaining individual lazy rows. A lazy stack manages its children per direct child, so the chunk is laid out as one opaque unit with no lazy-layout boundaries inside it β which was verified to keep the layout stable during overscroll, same as the full container swap.ForEaches with a nested container in betweenForEach, so its structural identity churns continuously as the conversation growsThe container swap was kept because its complexity is front-loaded and obvious (one
ifaround the container), while the chunk approach spreads subtle identity implications across the list body. If the one-time reflow at the threshold ever becomes noticeable, the chunk approach is a validated drop-in alternative.π¨ Showcase
Few messages render at the top of the list instead of the bottom, with the remaining space kept empty below them.
π§ͺ Manual Testing Notes
βοΈ Contributor Checklist
docs-contentrepoSummary by CodeRabbit
New Features
Documentation