-
Notifications
You must be signed in to change notification settings - Fork 371
feat(ui): Add floating date divider builder. #2422
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.
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
feat(ui): Add floating date divider builder. #2422
Conversation
WalkthroughAdds a new public builder parameter Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App / Widget tree
participant ListView as StreamMessageListView
participant Floating as FloatingDateDivider
Note over ListView: new field\nfloatingDateDividerBuilder
App->>ListView: build()
ListView->>Floating: instantiate(floatingDateDividerBuilder, dateDividerBuilder)
alt floatingDateDividerBuilder provided
Floating->>Floating: render using floatingDateDividerBuilder(DateTime)
else fallback
Floating->>Floating: render using dateDividerBuilder(DateTime)
end
Floating-->>ListView: widget
ListView-->>App: composed UI
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/stream_chat_flutter/lib/src/message_list_view/floating_date_divider.dart(3 hunks)packages/stream_chat_flutter/lib/src/message_list_view/message_list_view.dart(3 hunks)
🔇 Additional comments (2)
packages/stream_chat_flutter/lib/src/message_list_view/floating_date_divider.dart (1)
21-21: LGTM! Consistent parameter rename.The rename from
dateDividerBuildertofloatingDateDividerBuilderimproves clarity and distinguishes this builder from the regular date divider builder. The change is applied consistently across the constructor, field declaration, and usage site.Also applies to: 46-46, 70-70
packages/stream_chat_flutter/lib/src/message_list_view/message_list_view.dart (1)
98-98: LGTM! New parameter added correctly.The
floatingDateDividerBuilderparameter is properly declared with clear documentation distinguishing it from the regulardateDividerBuilder. The separation of concerns between regular and floating date dividers is well-designed.Also applies to: 245-247
packages/stream_chat_flutter/lib/src/message_list_view/message_list_view.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/stream_chat_flutter/test/src/message_list_view/floating_date_divider_test.dart (1)
142-143: Update test description to match renamed parameter.The test description references "dateDividerBuilder" but the parameter was renamed to "floatingDateDividerBuilder". Update the description for consistency.
Apply this diff:
- testWidgets( - 'renders custom dateDividerBuilder when provided', - (tester) async { + testWidgets( + 'renders custom floatingDateDividerBuilder when provided', + (tester) async {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/stream_chat_flutter/test/src/message_list_view/floating_date_divider_test.dart(1 hunks)
🔇 Additional comments (1)
packages/stream_chat_flutter/test/src/message_list_view/floating_date_divider_test.dart (1)
163-169: LGTM! Builder parameter rename is correct.The parameter rename from
dateDividerBuildertofloatingDateDividerBuilderaligns with the PR objectives, and the test correctly verifies that the custom builder is used instead of the defaultStreamDateDivider.
packages/stream_chat_flutter/lib/src/message_list_view/message_list_view.dart
Outdated
Show resolved
Hide resolved
packages/stream_chat_flutter/lib/src/message_list_view/floating_date_divider.dart
Show resolved
Hide resolved
packages/stream_chat_flutter/test/src/message_list_view/floating_date_divider_test.dart
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2422 +/- ##
=======================================
Coverage 63.93% 63.93%
=======================================
Files 413 413
Lines 25925 25926 +1
=======================================
+ Hits 16575 16576 +1
Misses 9350 9350 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/stream_chat_flutter/lib/src/message_list_view/message_list_view.dart (1)
245-247: LGTM! Consider enhancing documentation (optional).The field declaration is correct and type-consistent with
dateDividerBuilder. For improved developer experience, consider adding a brief note about the fallback behavior (e.g., "Defaults todateDividerBuilderwhen not provided").
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/stream_chat_flutter/CHANGELOG.md(1 hunks)packages/stream_chat_flutter/lib/src/message_list_view/message_list_view.dart(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/stream_chat_flutter/CHANGELOG.md
🔇 Additional comments (2)
packages/stream_chat_flutter/lib/src/message_list_view/message_list_view.dart (2)
98-98: LGTM!The parameter is correctly positioned and follows the established pattern for builder parameters in this widget.
860-861: LGTM! Backward compatibility handled correctly.The fallback to
dateDividerBuilderensures existing code continues to work without modification, as suggested in previous review feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/stream_chat_flutter/lib/src/message_list_view/message_list_view.dart (1)
98-98: Consider adding test coverage for the new parameter.The implementation is correct and follows best practices. However, as noted in previous reviews, test coverage for the
floatingDateDividerBuilderparameter is still missing. Consider adding widget tests that verify:
- Custom builder is invoked when provided
- Falls back to
dateDividerBuilderwhenfloatingDateDividerBuilderis null- Falls back to default
StreamDateDividerwhen both are nullAlso applies to: 245-249, 862-865
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/stream_chat_flutter/CHANGELOG.md(1 hunks)packages/stream_chat_flutter/lib/src/message_list_view/message_list_view.dart(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/stream_chat_flutter/CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: analyze_legacy_versions
- GitHub Check: stream_chat_localizations
- GitHub Check: stream_chat_flutter_core
- GitHub Check: stream_chat
- GitHub Check: stream_chat_persistence
- GitHub Check: stream_chat_flutter
- GitHub Check: build (ios)
- GitHub Check: test
- GitHub Check: build (android)
- GitHub Check: format
- GitHub Check: analyze
🔇 Additional comments (3)
packages/stream_chat_flutter/lib/src/message_list_view/message_list_view.dart (3)
98-98: Parameter addition looks good.The new
floatingDateDividerBuilderparameter is correctly added to the constructor and follows the existing pattern for optional builder parameters.
245-249: Well-documented parameter.The documentation clearly explains the purpose and fallback behavior of the
floatingDateDividerBuilderparameter. The type signature is consistent withdateDividerBuilder.
862-865: Fallback implementation is correct.The switch expression properly implements the fallback behavior to
dateDividerBuilderwhenfloatingDateDividerBuilderis not provided, ensuring backward compatibility. This addresses the feedback from the previous review.
CLA
Description of the pull request
Summary by CodeRabbit