-
Notifications
You must be signed in to change notification settings - Fork 346
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): Rework custom attachments builders #1826
feat(ui): Rework custom attachments builders #1826
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1826 +/- ##
==========================================
- Coverage 60.28% 60.28% -0.01%
==========================================
Files 317 317
Lines 18478 18479 +1
==========================================
Hits 11140 11140
- Misses 7338 7339 +1 ☔ View full report in Codecov by Sentry. |
Review and everything else takes too long, If someone from package developers can manage this PR by themselves, please do it! Thank you, in advance! |
@Mounix99 If I am not mistaken, this PR is the solution to if you want to have you custom attachment builder like a audioAttachmentBuilder, right now it will only have your custom builder working and there is no way to have the defaults + your custom ones right? // ONLY AudioAttachmentBuilder()
StreamMessageListView(
messageBuilder: (context, details, messages, defaultMessage) {
return defaultMessage.copyWith(
attachmentBuilders: const [
AudioAttachmentBuilder(),
],
);
},
),
==================================
// ONLY default builders work
StreamMessageListView(), I also tried this but the "on tap" functionality of the default attachments don't work (you only see the thumbnails): StreamMessageListView(
messageBuilder: (context, details, messages, defaultMessage) {
return defaultMessage.copyWith(
attachmentBuilders: const [
ImageAttachmentBuilder(),
VideoAttachmentBuilder(),
GiphyAttachmentBuilder(),
GalleryAttachmentBuilder(),
FileAttachmentBuilder(),
UrlAttachmentBuilder(),
AudioAttachmentBuilder(),
],
);
},
), |
@aminraeisi Yes, this PR is directly for this. For now my builders looks like this class MessageBuilderWidget extends StatelessWidget {
const MessageBuilderWidget({super.key, required this.defaultMessage});
final StreamMessageWidget defaultMessage;
@override
Widget build(BuildContext context) {
return defaultMessage.copyWith(
attachmentBuilders: myBuilders(context, defaultMessage.message, StreamChannel.of(context).channel));
}
List<StreamAttachmentWidgetBuilder> myBuilders(BuildContext context, Message message, Channel channel) {
final channelName = channel.name;
return [
AudioAttachmentBuilder(),
LocationAttachmentBuilder(channelName: channelName),
...StreamAttachmentWidgetBuilder.defaultBuilders(
message: message,
/// callback to handle attachment tap, copied from package
onAttachmentTap: (message, attachment) {
// If the current attachment is a url preview attachment, open the url
// in the browser.
final isUrlPreview = attachment.type == AttachmentType.urlPreview;
if (isUrlPreview) {
final url = attachment.ogScrapeUrl ?? '';
launchURL(context, url);
return;
}
final isImage = attachment.type == AttachmentType.image;
final isVideo = attachment.type == AttachmentType.video;
final isGiphy = attachment.type == AttachmentType.giphy;
// If the current attachment is a media attachment, open the media
// attachment in full screen.
final isMedia = isImage || isVideo || isGiphy;
if (isMedia) {
final attachments = message.toAttachmentPackage(
filter: (it) {
final isImage = it.type == AttachmentType.image;
final isVideo = it.type == AttachmentType.video;
final isGiphy = it.type == AttachmentType.giphy;
return isImage || isVideo || isGiphy;
},
);
Navigator.of(context).push(
MaterialPageRoute(
builder: (context) {
return StreamChannel(
channel: channel,
child: StreamFullScreenMediaBuilder(
userName: message.user!.name,
mediaAttachmentPackages: attachments,
startIndex: attachments.indexWhere(
(it) => it.attachment.id == attachment.id,
),
),
);
},
),
);
return;
}
})
];
}
}
/// Extension on [Message] copied from package
extension on Message {
List<StreamAttachmentPackage> toAttachmentPackage({
bool Function(Attachment)? filter,
}) {
// Create a copy of the attachments list.
var attachments = [...this.attachments];
if (filter != null) {
attachments = [...attachments.where(filter)];
}
// Create a list of StreamAttachmentPackage from the attachments list.
return [
...attachments.map((it) {
return StreamAttachmentPackage(
attachment: it,
message: this,
);
})
];
}
} I've extracted some package code to handle onTap logic for default builders, I think it can suit for you too |
@Mounix99 thanks a lot for the code! |
@aminraeisi You are Welcome!) |
Hey @Mounix99, apologies for the delay on our end. Thanks for your contributions, I'm reviewing the attachment builders on our end at the moment and I will review these immediately after. |
Just a note - I'm working on this issue on a separate PR (#1938) where I'm also working to avoid any of the onTap() code that needs to be implemented for the defaults. |
An update @Mounix99 and @aminraeisi: In #1938 we are changing the defaults to always include the custom attachment builders. You will ONLY need to pass along the custom builders. However, passing the normal attachment builders should not break your app either. So, once the PR is merged, the code from @Mounix99 should be simplified to: class MessageBuilderWidget extends StatelessWidget {
const MessageBuilderWidget({super.key, required this.defaultMessage});
final StreamMessageWidget defaultMessage;
@override
Widget build(BuildContext context) {
return defaultMessage.copyWith(
attachmentBuilders: [
AudioAttachmentBuilder(),
LocationAttachmentBuilder(),
],
}
}
} I will let you know once the PR is merged. |
@Mounix99 / @aminraeisi this PR is now merged. You should be able to use the above code to add custom attachments on the master branch. The change will be rolled out in the next release but you can of course fetch the package from git if it is an immediate need. I am closing this PR since this issue is resolved, but of course, feel free to raise the issue again if something feels off. Once again - apologies from our end for taking some time to get to this. Additionally, thanks for the PR as well. |
Submit a pull request
changes requested in issue
CLA
Description of the pull request