refactor(ui)!: add support for non-attachment types in attachment picker #2293
Conversation
WalkthroughThe attachment picker system for the Flutter chat UI was extensively refactored and modularized. New controller, option, and result classes were introduced, supporting both tabbed and system picker modes. The API was extended for custom picker options and results, with improved configuration and state management. Deprecated logic was removed, and exports updated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant StreamMessageInput
participant AttachmentPickerModal
participant StreamAttachmentPickerController
participant CustomOptionHandler
User->>StreamMessageInput: Press attachment button
StreamMessageInput->>AttachmentPickerModal: showStreamAttachmentPickerModalBottomSheet
AttachmentPickerModal->>StreamAttachmentPickerController: Initialize or update controller
AttachmentPickerModal->>User: Show picker UI (tabbed or system)
User->>AttachmentPickerModal: Select option / pick attachments / create poll
alt Attachments picked
AttachmentPickerModal->>StreamMessageInput: AttachmentsPicked result
StreamMessageInput->>StreamAttachmentPickerController: Update attachments
else Poll created
AttachmentPickerModal->>StreamMessageInput: PollCreated result
StreamMessageInput->>StreamAttachmentPickerController: Update poll
else Custom option
AttachmentPickerModal->>CustomOptionHandler: CustomAttachmentPickerResult
CustomOptionHandler->>StreamMessageInput: Handle custom result
else Error
AttachmentPickerModal->>StreamMessageInput: AttachmentPickerError
StreamMessageInput->>User: Show error
end
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected related to the linked issue. Possibly related PRs
Poem
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 review |
✅ Actions performedReview triggered.
|
xsahil03x
force-pushed
the
refactor/stream-attachment-picker
branch
from
1ba1ab7 to
0a2e2db
Compare
xsahil03x
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v10.0.0 #2293 +/- ##
==========================================
Coverage ? 63.97%
==========================================
Files ? 414
Lines ? 25633
Branches ? 0
==========================================
Hits ? 16398
Misses ? 9235
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/stream_chat_flutter/lib/src/message_input/attachment_picker/options/stream_gallery_picker.dart (1)
144-147: Test coverage gap for configuration usage.While the implementation is correct, the static analysis correctly identifies that these lines lack test coverage. Consider adding tests to verify that the config properties are properly passed through to the
StreamPhotoGallerywidget.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/stream_chat_flutter/lib/src/bottom_sheets/edit_message_sheet.dart(0 hunks)packages/stream_chat_flutter/lib/src/message_input/attachment_picker/options/stream_gallery_picker.dart(5 hunks)packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart(7 hunks)packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker_bottom_sheet.dart(8 hunks)packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker_controller.dart(1 hunks)packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker_option.dart(1 hunks)packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker_result.dart(1 hunks)packages/stream_chat_flutter/lib/src/message_input/stream_message_input.dart(4 hunks)packages/stream_chat_flutter/lib/src/utils/extensions.dart(1 hunks)packages/stream_chat_flutter/lib/stream_chat_flutter.dart(1 hunks)
💤 Files with no reviewable changes (1)
- packages/stream_chat_flutter/lib/src/bottom_sheets/edit_message_sheet.dart
✅ Files skipped from review due to trivial changes (1)
- packages/stream_chat_flutter/lib/stream_chat_flutter.dart
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/stream_chat_flutter/lib/src/message_input/stream_message_input.dart
- packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker_result.dart
- packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker_option.dart
- packages/stream_chat_flutter/lib/src/utils/extensions.dart
- packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker_controller.dart
🧰 Additional context used
🪛 GitHub Check: codecov/patch
packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart
[warning] 14-14: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L14
Added line #L14 was not covered by tests
[warning] 28-31: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L28-L31
Added lines #L28 - L31 were not covered by tests
[warning] 33-33: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L33
Added line #L33 was not covered by tests
[warning] 35-39: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L35-L39
Added lines #L35 - L39 were not covered by tests
[warning] 41-41: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L41
Added line #L41 was not covered by tests
[warning] 43-45: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L43-L45
Added lines #L43 - L45 were not covered by tests
[warning] 61-61: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L61
Added line #L61 was not covered by tests
[warning] 83-83: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L83
Added line #L83 was not covered by tests
[warning] 90-91: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L90-L91
Added lines #L90 - L91 were not covered by tests
[warning] 93-95: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L93-L95
Added lines #L93 - L95 were not covered by tests
[warning] 97-97: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L97
Added line #L97 was not covered by tests
[warning] 99-100: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L99-L100
Added lines #L99 - L100 were not covered by tests
[warning] 108-108: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L108
Added line #L108 was not covered by tests
[warning] 112-112: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L112
Added line #L112 was not covered by tests
[warning] 122-122: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L122
Added line #L122 was not covered by tests
[warning] 124-124: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L124
Added line #L124 was not covered by tests
[warning] 135-135: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L135
Added line #L135 was not covered by tests
[warning] 154-155: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L154-L155
Added lines #L154 - L155 were not covered by tests
[warning] 166-167: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L166-L167
Added lines #L166 - L167 were not covered by tests
[warning] 171-172: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L171-L172
Added lines #L171 - L172 were not covered by tests
[warning] 176-176: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L176
Added line #L176 was not covered by tests
[warning] 194-195: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L194-L195
Added lines #L194 - L195 were not covered by tests
[warning] 197-200: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L197-L200
Added lines #L197 - L200 were not covered by tests
[warning] 202-202: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L202
Added line #L202 was not covered by tests
[warning] 426-426: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L426
Added line #L426 was not covered by tests
[warning] 434-434: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L434
Added line #L434 was not covered by tests
[warning] 439-440: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L439-L440
Added lines #L439 - L440 were not covered by tests
[warning] 442-442: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L442
Added line #L442 was not covered by tests
[warning] 446-446: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart#L446
Added line #L446 was not covered by tests
packages/stream_chat_flutter/lib/src/message_input/attachment_picker/options/stream_gallery_picker.dart
[warning] 144-147: packages/stream_chat_flutter/lib/src/message_input/attachment_picker/options/stream_gallery_picker.dart#L144-L147
Added lines #L144 - L147 were not covered by tests
🔇 Additional comments (11)
packages/stream_chat_flutter/lib/src/message_input/attachment_picker/options/stream_gallery_picker.dart (2)
27-30: Excellent API consolidation with backward compatibility.The refactoring to consolidate thumbnail-related parameters into a single
GalleryPickerConfigobject significantly improves the API design. Using a default config ensures backward compatibility while making the interface cleaner and more maintainable.
163-184: Well-designed configuration class with sensible defaults.The
GalleryPickerConfigclass provides a clean abstraction for gallery picker configuration with appropriate default values that match the original behavior. The const constructor and immutable design follow Flutter best practices.packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker_bottom_sheet.dart (4)
7-60: Comprehensive documentation for dual picker modes.The extensive documentation clearly explains the two picker modes (tabbed vs system) with practical examples. This will significantly help developers understand when and how to use each mode.
118-145: Robust type validation for system picker options.The validation logic that ensures only
SystemAttachmentPickerOptioninstances are used with the system picker is excellent. The clear error message will help developers quickly identify configuration issues.
147-164: Consistent validation pattern for tabbed picker options.The validation logic mirrors the system picker validation, ensuring type safety across both picker modes. The consistent error messaging pattern improves the developer experience.
180-217: Clean builder refactoring with enhanced controller management.The renamed
StreamAttachmentPickerBottomSheetBuilderproperly handles the newinitialExtraDataparameter and maintains good controller lifecycle management. The controller update logic correctly handles all scenarios.packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart (5)
12-54: Excellent architectural separation with clear responsibilities.The new
StreamSystemAttachmentPickerBottomSheetandStreamTabbedAttachmentPickerBottomSheetclasses provide a clean separation of concerns between system-integrated and tabbed picker interfaces. The ValueListenableBuilder approach ensures reactive UI updates.
170-178: Modern Dart switch expressions improve readability.The use of modern Dart switch expressions for determining colors and enabled states makes the code more concise and readable compared to traditional if-else chains.
434-444: Improved error handling with structured results.The
_handleSingePickfunction properly wraps operations in try-catch blocks and returns structuredStreamAttachmentPickerResulttypes. This provides better error handling and type safety compared to the previous implementation.
458-564: Enhanced option configuration with predicates.The new
TabbedAttachmentPickerOptionapproach withisEnabledpredicates provides much more flexibility for controlling when options are available based on current state. This is a significant improvement over static configuration.
588-660: Comprehensive system picker implementation.The
systemAttachmentPickerBuilderprovides a clean alternative interface for platforms that prefer native file dialogs. The consistent error handling and result types maintain API uniformity across picker modes.
...es/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/stream_chat_flutter/CHANGELOG.md (1)
11-11: Consider switching the migration-guide link to an absolute URL.Using a relative link (
../../migrations/v10-migration.md) can break when the changelog is rendered on external sites (e.g., pub.dev). An absolute GitHub URL guarantees the link always resolves.-For more details, please refer to the [migration guide](../../migrations/v10-migration.md). +For more details, see the [migration guide](https://github.com/GetStream/stream-chat-flutter/blob/main/migrations/v10-migration.md).migrations/v10-migration.md (1)
31-32: Remove trailing colons in headings to satisfy markdownlint (MD026)Headings ending with
:violate MD026. Dropping the colon keeps the text identical while clearing the linter noise:-#### Key Changes: +#### Key Changes ... -#### Migration Steps: +#### Migration StepsApply the same tweak to all other headings flagged by the linter (lines listed in this comment).
This keeps the guide tidy and CI-friendly.Also applies to: 37-38, 63-64, 69-70, 118-119, 123-124, 162-163, 167-168, 369-370, 374-375
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
migrations/v10-migration.md(3 hunks)packages/stream_chat_flutter/CHANGELOG.md(1 hunks)packages/stream_chat_flutter/lib/stream_chat_flutter.dart(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/stream_chat_flutter/lib/stream_chat_flutter.dart
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
migrations/v10-migration.md
31-31: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
37-37: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
63-63: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
69-69: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
118-118: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
123-123: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
162-162: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
167-167: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
369-369: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
374-374: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🪛 LanguageTool
packages/stream_chat_flutter/CHANGELOG.md
[style] ~17-~17: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...bs like contact and location pickers. - Added onCustomAttachmentPickerResult callba...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ 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). (3)
- GitHub Check: analyze
- GitHub Check: build (android)
- GitHub Check: test
🔇 Additional comments (2)
packages/stream_chat_flutter/CHANGELOG.md (2)
5-9: Changelog entries are clear and follow existing style.The wording, back-tick formatting for identifiers, and period usage match the convention used throughout earlier sections. No action required.
15-17: Minor wording duplicate, but acceptable.Three consecutive bullets start with “Added …”, which is fine for enumerations and consistent with prior sections. No change needed.
Linear: FLU-179
Description of the pull request
This commit introduces significant enhancements to the attachment picker:
Core Changes:
StreamAttachmentPickerController: Now handles a more genericAttachmentPickerValuewhich includespoll,attachments, andextraDatafor custom values.AttachmentPickerOption: A new base class for picker options, with two concrete implementations:TabbedAttachmentPickerOption: For options that display custom UI within a tabbed interface (common on mobile).SystemAttachmentPickerOption: For options that integrate with system pickers (e.g., file dialogs, camera).AttachmentPickerType: A sealed class hierarchy representing the types of attachments an option can support (e.g., images, videos, files, poll, and custom types).StreamAttachmentPickerResult: A sealed class hierarchy for results from the picker, includingAttachmentsPicked,PollCreated,AttachmentPickerError, and a baseCustomAttachmentPickerResultfor extensions.UI and Integration:
StreamMessageInput:customAttachmentPickerOptionsto extend the picker.onCustomAttachmentPickerResultto handle results from custom options.showStreamAttachmentPickerModalBottomSheet:useSystemAttachmentPicker.TabbedAttachmentPickerOptionfor tabbed interface).StreamAttachmentPickerResult.StreamMobileAttachmentPickerBottomSheetwithStreamTabbedAttachmentPickerBottomSheet.StreamWebOrDesktopAttachmentPickerBottomSheetwithStreamSystemAttachmentPickerBottomSheet.StreamGalleryPickernow accepts aGalleryPickerConfigfor its settings.Screenshots / Videos
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Refactor
Documentation
Style