feat: Jump to Message for Conversation Attachments#3087
Open
DeveloperBlue wants to merge 9 commits into
Open
Conversation
Member
Any reason not to resolve this before the PR gets merged? |
Contributor
Author
|
Good point, resolved 👍 |
Contributor
Author
|
Also related: would you like me to submit the Samsung parity issues in this PR too, or just do one separate followup PR?
They're a little mixed on scope but pretty minor changes. Not sure if this adds bloat to the PR for your review. Otherwise, good to go on my end. edit: Added UI for Samsung variant |
# Conflicts: # lib/app/layouts/fullscreen_media/fullscreen_image.dart
Member
|
yeah, if you can, i'd like to get the feature implemented for all skins and all without leaking the name when redacted mode is enabled |
Contributor
Author
|
Added the missing media viewer buttons and fixed the redacted mode leaks for conversations with this PR 👍 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds the ability to jump to messages for attachments under the conversation details pages.
It uses the existing
jumpToMessagefunction found inMessagesService.It is only shown for attachments opened from the conversation details pages.
MediaGalleryCard,ConversationFullscreenHolder, andImageDisplaynow have ashowJumpToMessageprop.Material: Adds a "See in chat" overlay button, similar to Google Messages
Samsung: The attachment header is now tappable to trigger jumpToMessage
qemu-system-x86_64_3mzpjNObrd.mp4
iOS and Material UI
Links, Locations, and Other Files Not Included
1 The other types of attachments (Links, Locations, and Other Files) all directly open their attachments externally. There are currently no places to add the jumpToMessage UI. This is the only blocker.
One solution would be a long-press context menu, similar to the "Attachment Popups" feature showcased in #2895,. This is out of scope for this PR.
Developer Notes
The current
jumpToMessagemethod loads all the messages from the current cursor to the target message contiguously. If the target message is far away (hundreds or even thousands of messages away), this could potentially take up to minutes to load all the messages in between.The conversation view depends on these loaded messages being contiguous, so I was not able to just load a specific window around the target message.
Aside from that, the feature works fine. If we wanted to address this, the ideal fix would be that the Messages service gets refactored to handle loading non-contiguous messages. Less ideally, there is also the way the search feature loads specific isolated message windows. This has it's own share of issues and limitations (#2269) .
Additional Fixes:
Tested on an android mobile emulator and a physical android tablet