Skip to content

Comments

Multiplefiles#25

Draft
manandharsudhir wants to merge 6 commits intoDiversido:mainfrom
manandharsudhir:multiplefiles
Draft

Multiplefiles#25
manandharsudhir wants to merge 6 commits intoDiversido:mainfrom
manandharsudhir:multiplefiles

Conversation

@manandharsudhir
Copy link

No description provided.

@manandharsudhir
Copy link
Author

i have added sending and getting multiple files on chats

@asmodeoux
Copy link
Contributor

Thank you!
@martintrollip @Brandon-Wolff can you guys test these changes please?

///
/// If you specify [MessageOptions.withMedia] then you will not be able to specify [MessageOptions.withBody] because they are mutually exclusive message types. Created message type will be [MessageType.MEDIA].
void withMedia(File input, String mimeType) {
void withMedia(List<File> input, List<String> mimeType,List<String> filename) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for the feature @manandharsudhir,

This line will cause a breaking change since the parameters changed to lists. This is evident when compiling the project example. Is there a way to keep the method paramaters the same and chain addMedia calls? See how this builder from the Twilio docs adds multiple files.

In addition, the filename used to be optional. This might be another bug since it's not used on Kotlin and on Swift it's hardcoded to image.jpg. Is it possible to keep it optional and apply defaults if it was not provided?

And finally, this feature has to be applied on the Android plugin as well so that both platforms have the same implementation.

@manandharsudhir
Copy link
Author

manandharsudhir commented Feb 19, 2024 via email

@martintrollip
Copy link
Contributor

It might be best to create new functions taking list parameters then. The change as it is currently stands would break existing usages of the package.

@asmodeoux asmodeoux marked this pull request as draft July 8, 2024 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants