fix(ui): improve scroll to bottom and floating date divider #2289
Conversation
This commit addresses two issues: 1. **Scroll to Bottom:** The logic for determining if the last item is fully visible was flawed, causing the "scroll to bottom" button to appear incorrectly in some cases. This has been fixed by checking if the last item's leading edge is greater than or equal to 0. 2. **Floating Date Divider:** The floating date divider was not correctly calculating the date for messages in case the message widget exceeds the viewport height. This has been fixed by adjusting the index calculations to account for these items and ensuring that the divider only considers actual message widgets. Additionally, the `isThreadConversation` property in `FloatingDateDivider` has been deprecated as it's no longer used. The utility functions `getTopElementIndex` and `getBottomElementIndex` have also been updated to ignore items that are not actually rendering anything in the viewport.
WalkthroughThis update refines the message list view logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MessageListView
participant FloatingDateDivider
participant ScrollToBottomButton
User->>MessageListView: Scrolls through messages
MessageListView->>FloatingDateDivider: Determines if date divider should display
FloatingDateDivider-->>MessageListView: Returns date divider widget or empty
MessageListView->>ScrollToBottomButton: Checks if last item is visible
alt Last item not fully visible
ScrollToBottomButton-->>User: Show "Scroll to Bottom" button
else Last item fully visible
ScrollToBottomButton-->>User: Hide button, call last item visible handler
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (10)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/stream_chat_flutter/lib/src/message_list_view/mlv_utils.dart (1)
46-54: Consider combining filter conditions for minor optimization.The filtering logic works correctly, but you could slightly optimize by combining the conditions into a single filter:
- final inView = values.where((position) { - if (position.itemLeadingEdge == position.itemTrailingEdge) { - // If the item's leading and trailing edges are the same, it means the - // item isn't actually rendering anything in the viewport. - return false; - } - - return position.itemTrailingEdge > 0; - }); + final inView = values.where((position) { + // Exclude items that aren't rendering anything (zero size) and + // items that are not visible in the viewport + return position.itemLeadingEdge != position.itemTrailingEdge && + position.itemTrailingEdge > 0; + });Apply the same pattern to
getBottomElementIndex. However, the current approach is perfectly fine and more readable.Also applies to: 64-72
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/stream_chat_flutter/CHANGELOG.md(1 hunks)packages/stream_chat_flutter/lib/src/message_list_view/floating_date_divider.dart(2 hunks)packages/stream_chat_flutter/lib/src/message_list_view/message_list_view.dart(1 hunks)packages/stream_chat_flutter/lib/src/message_list_view/mlv_utils.dart(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: analyze_legacy_versions
- GitHub Check: stream_chat_localizations
- GitHub Check: stream_chat_flutter_core
- GitHub Check: stream_chat_flutter
- GitHub Check: stream_chat_persistence
- GitHub Check: stream_chat
- GitHub Check: build (android)
- GitHub Check: build (ios)
- GitHub Check: analyze
- GitHub Check: format
- GitHub Check: test
🔇 Additional comments (8)
packages/stream_chat_flutter/CHANGELOG.md (1)
1-9: LGTM! Clear and well-documented bug fixes.The changelog entries accurately document the bug fixes for UI behavior when messages exceed viewport size. The descriptions are specific and follow the established formatting conventions.
packages/stream_chat_flutter/lib/src/message_list_view/mlv_utils.dart (2)
46-54: Good filtering logic to exclude zero-size items.The addition of the zero-size item filter makes sense and improves the accuracy of viewport visibility calculations. The comment clearly explains the reasoning.
64-72: Consistent filtering logic applied correctly.The same zero-size filtering logic is properly applied to the bottom element detection, maintaining consistency between the two functions.
packages/stream_chat_flutter/lib/src/message_list_view/message_list_view.dart (2)
1454-1466: LGTM! Improved scroll-to-bottom visibility logic.The new logic correctly addresses the issue mentioned in the PR objectives. Using a direct positional comparison with
itemLeadingEdge >= 0is more reliable than the previous helper-based approach for determining if the last item is fully visible.The hardcoded
lastItemIndex = 2is well-documented and consistent with the item structure described in the comments (lines 649-666).
854-860: Properly removed deprecated parameter.The removal of the
isThreadConversationparameter aligns with its deprecation in theFloatingDateDividercomponent. This cleanup maintains backward compatibility while moving toward the simplified implementation.packages/stream_chat_flutter/lib/src/message_list_view/floating_date_divider.dart (3)
19-20: Proper deprecation of unused property.The
isThreadConversationproperty is correctly marked as deprecated with clear annotation. This provides a smooth transition path while maintaining backward compatibility.Also applies to: 25-26
52-76: Excellent refactor of index calculation logic.The new explicit approach for handling special list items is much clearer and more maintainable than the previous conditional logic. Each special item type (parent message, header, loaders, footer) is explicitly handled, and the message index calculation
index - 2properly accounts for the extra non-message items.This addresses the PR objective of fixing the floating date divider when messages exceed viewport height.
78-82: Simplified and improved date divider builder logic.The streamlined approach using pattern matching with
caseis cleaner and more readable than the previous implementation. The direct builder call and fallback toStreamDateDivideris well-structured.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2289 +/- ##
=========================================
Coverage ? 63.51%
=========================================
Files ? 408
Lines ? 25518
Branches ? 0
=========================================
Hits ? 16207
Misses ? 9311
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| if (index == null) return const Empty(); | ||
|
|
||
| // We can only calculate the date divider if the element is a message | ||
| // widget and not one of the special items. | ||
|
|
||
| // Parent Message | ||
| if (index == itemCount - 1) return const Empty(); | ||
| // Header Builder | ||
| if (index == itemCount - 2) return const Empty(); | ||
| // Top Loader Builder | ||
| if (index == itemCount - 3) return const Empty(); | ||
| // Bottom Loader Builder | ||
| if (index == 1) return const Empty(); | ||
| // Footer Builder | ||
| if (index == 0) return const Empty(); |
There was a problem hiding this comment.
I would create a different method for it, this is adding quite some noise
Just like this:
if(!_shouldShowDivider(index)) return const Empty();with:
bool _shouldShowDivider(int index){
return index != null &&
// We can only calculate the date divider if the element is a message
// widget and not one of the special items.
// Parent Message
index != itemCount - 1 &&
// Header Builder
index != itemCount - 2 &&
// Top Loader Builder
index != itemCount - 3 &&
// Bottom Loader Builder
index != 1 &&
// Footer Builder
index != 0;
}There was a problem hiding this comment.
fixed in: chore: apply review suggestions
Description of the pull request
This PR addresses two issues:
Scroll to Bottom: The logic for determining if the last item is fully visible was flawed, causing the "scroll to bottom" button to appear incorrectly in some cases. This has been fixed by checking if the last item's leading edge is greater than or equal to 0.
Floating Date Divider: The floating date divider was not correctly calculating the date for messages in case the message widget exceeds the viewport height. This has been fixed by adjusting the index calculations to account for these items and ensuring that the divider only considers actual message widgets.
Additionally, the
isThreadConversationproperty inFloatingDateDividerhas been deprecated as it's no longer used. The utility functionsgetTopElementIndexandgetBottomElementIndexhave also been updated to ignore items that are not actually rendering anything in the viewport.Screenshots / Videos
Screen.Recording.2025-06-26.at.05.07.03.mov
Screen.Recording.2025-06-26.at.05.05.02.mov
Summary by CodeRabbit
Bug Fixes
Documentation
Refactor
Tests