-
Notifications
You must be signed in to change notification settings - Fork 123
Expanded Conversation Attachments #2895
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: development
Are you sure you want to change the base?
Expanded Conversation Attachments #2895
Conversation
An awesome addition to the app! The only change I'll request is that you center the filters in the popup window, to be consistent with other popup UIs (i.e. the search filtering). I have a couple of questions as well:
Does this add it to all media viewer pages or just the one in the chat details?
If we make changes to the other popups (UI-wise), I assume we'll also need to also update the new message popup holder?
Our jump to message code is sometimes bugged, which is why some people get freezes/crashes when certain chats have it enabled. I don't have a fix at this time, so I think maybe removing it as a feature until we figure out how to better do it. Otherwise... it looks great! Thank you for your contribution! |
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.
See my comments above
Is this what you mean by centering the filters? I've gone ahead and horizontally centered them with the last push, but I see that the filter for searching conversations is left-aligned on my other devices. I've also removed/hid any UI for jumping to the attachment's message for now.
It is added to all media viewer pages. (Or more specifically, it's added to the existing media viewer page that all images/videos navigate to when you tap them.) Since I removed the Jump to Message feature for now, the button/label is now gone has been reverted to how it was before this PR.
Yes. In the future, I think a global popup component could be designed and then extended from for both components. I wouldn't mind looking into this. Please let me know if you have any more feedback, questions, or changes! Thanks again |
You're right... i gotta fix that then ;)
Perfect, thank ya
Sounds good, thank you for the contribution! I'll check it out after we do this Desktop release (happening shortly). Thanks Joel! Then we'll have to take a crack at the next Android update, which we'll include this in it. P.S. We know that we need to do a better job of making our components better and more "static" in a way. Less code within them and focus on minimizing widget tree updates to improve performance. Only issue is that it's a fairly large effort to take on with how complex the code base is. We also want to improve our chat manager services and what not to better optimize how the widget tree is updated. |
3d0369c
to
b251ba4
Compare
Resolved the changes to build.gradle 👍 |
Hope everyone's doing well, this is my first flutter PR.
This PR allows you to:
Viewing More Attachments
BB.-.More.Attachments.mp4
Filtering Attachments
BB.-.Attachment.Filtering.1.mp4
BB.-.Attachment.Filtering.2.mp4
Attachment Popups
BB.-.Attachment.Popup.mp4
Jump to Message
BB.-.Jump.To.Message.mp4
Additional Information
Issues
[ ] ⚠ Sometimes the "Jump To Message" action gets stuck in an infinite pending state. Exiting the conversation and jumping again seems to fix this issue. This should be resolved before the PR is merged, and I need a little help figuring out why it does this. Area 1, Area 2And some more extensive testing is required, since I was only able to test on my android emulator. I have yet to test the changes on the web or desktop builds. I'm @DeveloperBlue on the Discord as well.