-
Notifications
You must be signed in to change notification settings - Fork 371
feat(ui): add attachmentPickerOptionsBuilder
#2415
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
base: v10.0.0
Are you sure you want to change the base?
Conversation
…rResult` Introduces `attachmentPickerOptionsBuilder` to allow for full customization of the attachment picker options. This builder receives the context and a list of default options, which can then be modified, reordered, or extended. The `customAttachmentPickerOptions` property is now deprecated in favor of `attachmentPickerOptionsBuilder`. A new callback, `onAttachmentPickerResult`, is added. This callback is triggered when any result is returned from the attachment picker. It allows developers to handle custom attachment types by returning `true`. If `false` is returned, the default internal handling proceeds.
WalkthroughIntroduces an optionsBuilder-based customization for attachment picker options across tabbed and system pickers, replaces previous customOptions parameters, updates result handling to FutureOr via onAttachmentPickerResult, adjusts builders and bottom sheet wiring, and updates sample app usage accordingly while preserving default behaviors when no builder is supplied. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant MI as StreamMessageInput
participant BS as AttachmentPicker BottomSheet
participant PK as Picker (Tabbed/System)
participant OB as optionsBuilder
participant AH as onAttachmentPickerResult
U->>MI: Tap attach
MI->>BS: showStreamAttachmentPickerModalBottomSheet(optionsBuilder)
BS->>PK: Build picker (tabbed/system)
PK->>OB: Request options (context, defaultOptions)
OB-->>PK: List<AttachmentPickerOption>
U->>PK: Select option / complete action
PK-->>MI: StreamAttachmentPickerResult
MI->>AH: onAttachmentPickerResult(result)?
alt AH returns true (handled)
MI-->>U: Done (no further handling)
else AH absent or returns false
MI->>MI: Internal default handling (if any)
MI-->>U: Done
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
@coderabbitai review |
✅ Actions performedReview triggered.
|
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
🧹 Nitpick comments (7)
packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker_result.dart (1)
7-8
: Clarify return semantics and minor doc nitPlease document the boolean meaning (true = handled, false = continue default). Also fix “a attachment” -> “an attachment”.
Example:
-/// Signature for a function that is called when a attachment picker result -/// is received. +/// Signature for a function that is called when an attachment picker result +/// is received. +/// +/// Return `true` to indicate the result was handled by the caller; +/// return `false` to let the SDK perform its default handling. typedef OnAttachmentPickerResult<T extends StreamAttachmentPickerResult> = FutureOr<bool> Function(T result);packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart (4)
432-433
: Docs still reference customOptions; update to optionsBuilderPublic docs above still mention [customOptions]. Please update to [optionsBuilder] to avoid confusion.
Proposed doc tweak (outside this hunk):
-/// [customOptions], [galleryPickerConfig], [pollConfig], and +/// [optionsBuilder], [galleryPickerConfig], [pollConfig], and
494-496
: Typo: rename_handleSingePick
to_handleSinglePick
Minor but public-facing readability. Update call sites.
- final result = await _handleSingePick(controller, file); + final result = await _handleSinglePick(controller, file);- final result = await _handleSingePick(controller, image); + final result = await _handleSinglePick(controller, image);- final result = await _handleSingePick(controller, video); + final result = await _handleSinglePick(controller, video);And update the declaration (outside selected lines):
// rename Future<StreamAttachmentPickerResult> _handleSinglePick( StreamAttachmentPickerController controller, Attachment? attachment, ) async { ... }Also applies to: 511-514, 530-532
571-575
: Error text mentions “custom options”; align to builder APIUpdate error message to guide users correctly.
- 'custom options must be of type TabbedAttachmentPickerOption when using ' - 'the tabbed attachment picker (default on mobile).', + 'optionsBuilder must return only TabbedAttachmentPickerOption when using ' + 'the tabbed attachment picker (default on mobile).',
680-683
: Error text mentions “custom options”; align to builder APISame here for system picker.
- 'custom options must be of type SystemAttachmentPickerOption when using ' - 'the system attachment picker (enabled explicitly or on web/desktop).', + 'optionsBuilder must return only SystemAttachmentPickerOption when using ' + 'the system attachment picker (enabled explicitly or on web/desktop).',packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker_bottom_sheet.dart (1)
71-94
: Docs still reference customOptions; update to optionsBuilder and refresh examplesThe function docs still instruct passing [customOptions]. Replace with [optionsBuilder] and adjust examples to show modifying defaultOptions via the builder.
I can submit a doc update patch for the examples if helpful.
sample_app/lib/pages/channel_page.dart (1)
175-184
: Rename helper to reflect new API (optional)Rename
_onCustomAttachmentPickerResult
to_onAttachmentPickerResult
for consistency.- bool _onCustomAttachmentPickerResult( + bool _onAttachmentPickerResult( Channel channel, StreamAttachmentPickerResult result, ) { // Notify that the result was not handled. if (result is! LocationPicked) return false; _onShareLocationPicked(channel, result.location).ignore(); return true; // Notify that the result was handled. }
📜 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 (5)
packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart
(4 hunks)packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker_bottom_sheet.dart
(3 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)sample_app/lib/pages/channel_page.dart
(2 hunks)
🔇 Additional comments (6)
packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker_bottom_sheet.dart (2)
7-16
: Good: flexible options builder APIThe generic AttachmentPickerOptionsBuilder signature is clear and future‑proof.
126-139
: Correct wiring of builder selection and optionsBuilder passthroughPicker selection by platform flag and passing optionsBuilder looks solid.
sample_app/lib/pages/channel_page.dart (1)
138-140
: Correct usage of onAttachmentPickerResult (bool short‑circuit)Early return pattern is correct and matches new API.
packages/stream_chat_flutter/lib/src/message_input/stream_message_input.dart (3)
161-163
: New customization points look goodAdding attachmentPickerOptionsBuilder and onAttachmentPickerResult improves extensibility.
991-999
: Correctly passes optionsBuilder through to modalGood wiring to the bottom sheet builder.
1003-1006
: Good: user handler gets first dibsShort‑circuiting when the handler returns true aligns with the new contract.
return switch (result) { | ||
// Add the attachments to the controller. | ||
AttachmentsPicked() => _onAttachmentsPicked(result.attachments), | ||
// Send the created poll in the channel. | ||
PollCreated() => _onPollCreated(result.poll), | ||
// Handle custom attachment picker results. | ||
CustomAttachmentPickerResult() => _onCustomAttachmentPickerResult(result), | ||
// Handle/Notify returned errors. | ||
AttachmentPickerError() => _onAttachmentPickerError(result), | ||
_ => () {}, // Ignore other results. | ||
}; |
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.
Bug: returning a function from an async method via switch expression
_onAttachmentButtonPressed
is Future<void>
. The default branch returns a function literal () {}
, yielding a non-void value. Mixed branches (void
, Future<void>
, and function) in a switch expression cause a type error. Use a switch statement and await
where needed.
Apply:
- return switch (result) {
- // Add the attachments to the controller.
- AttachmentsPicked() => _onAttachmentsPicked(result.attachments),
- // Send the created poll in the channel.
- PollCreated() => _onPollCreated(result.poll),
- // Handle/Notify returned errors.
- AttachmentPickerError() => _onAttachmentPickerError(result),
- _ => () {}, // Ignore other results.
- };
+ switch (result) {
+ // Add the attachments to the controller.
+ case AttachmentsPicked():
+ _onAttachmentsPicked(result.attachments);
+ // Send the created poll in the channel.
+ case PollCreated():
+ await _onPollCreated(result.poll);
+ // Handle/Notify returned errors.
+ case AttachmentPickerError():
+ _onAttachmentPickerError(result);
+ default:
+ // no-op for other/custom results not handled by the user.
+ break;
+ }
+ return;
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/stream_chat_flutter/lib/src/message_input/stream_message_input.dart
around lines 1015 to 1023, the switch expression returns mixed types (void,
Future<void>, and a function literal `() {}`) causing a type error in the async
Future<void> method. Replace the switch expression with a switch statement:
handle each case by calling or awaiting the appropriate handler (await
_onAttachmentsPicked(...), await _onPollCreated(...), await
_onAttachmentPickerError(...)) and for the default case do nothing (no return
value), ensuring all branches produce void/Future<void> and the method remains
async-compatible.
These changes should address our usecase - which is basically just being able to update the ordering of widgets in the picker. 🍻 |
Description of the pull request
Introduces
attachmentPickerOptionsBuilder
to allow for full customization of the attachment picker options. This builder receives the context and a list of default options, which can then be modified, reordered, or extended.The
customAttachmentPickerOptions
property is now deprecated in favor ofattachmentPickerOptionsBuilder
.A new callback,
onAttachmentPickerResult
, is added. This callback is triggered when any result is returned from the attachment picker. It allows developers to handle custom attachment types by returningtrue
. Iffalse
is returned, the default internal handling proceeds.Summary by CodeRabbit
New Features
Refactor
Documentation