Skip to content
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

A number of link sharing improvements #1398

Merged
merged 4 commits into from
Jun 5, 2024

Conversation

micahmo
Copy link
Member

@micahmo micahmo commented May 29, 2024

Pull Request Description

This PR fixes a few issues with sharing that have been brought up in the Matrix chat and other PRs.

  • "Share Media" has been renamed to "Share Thumbnail As Image", since that's what it's really doing. (Note that this can be the same image as the full-size image, but the difference is that it's sharing the image itself, not a link.) This has changed slightly, please read the latest comments.
  • A new "Share Media Link" option has been added which shares the actual mediaUrl. This handles the fact that videos previously couldn't be shared. It also fixes a problem where you could only share the thumbnail-resolution variant of an image (if it was on your instance). Note that this can be the same link as "Share Thumbnail As Image" if the instance didn't generate a separate thumbnail, but that's ok because the share actions are distinct.
  • Only show "Share External Link" if it's not another kind of media (image/video), which should be sharable via "Share Media Link".
  • The "Share External Link" option can now be shown even if the post is not a link type (sometimes the original link differs from the media link, so it's helpful to have both).

@CTalvio I seem to recall you mentioned something about us either displaying or sharing the wrong resolution version of images, but I couldn't find the conversation in Matrix. Do you recall the exact problem, and does anything here fix that?

Screenshots

Ability to share thumbnail as image + link to article

image

Ability to share thumbnail as image + link to full-size image

image

Ability to share thumbnail as image + link to full-size image (where thumbnail and full-size are same)

image

Ability to share link to video + ability to share "original URL" when it's different

image

@CTalvio
Copy link
Collaborator

CTalvio commented May 29, 2024

The problem was Thunder currently doesn't use the actual post URL for images.

It always uses the cached copy on the pict-rs server of your instance.

There is hence no way to download, view, or share, the actual source media.

@micahmo
Copy link
Member Author

micahmo commented May 30, 2024

There is hence no way to download, view, or share, the actual source media.

Ok thanks for the reminder! I believe this PR fixes the "share" issue. I will see about viewing and downloading.

@micahmo
Copy link
Member Author

micahmo commented May 30, 2024

Alright, I pushed another change which adds a new imageUrl property to Media, preferring the originalUrl over the thumbnailUrl.

I also got rid of the "Share Thumbnail As Image" option from the share menu (since that didn't seem super useful to me) and added a "Share Image" option instead, using the new imageUrl property. Note that when the media type is not an image, this does still allow sharing the thumbnail as an image directly.

Examples

Image Post Link Post Video Post
image image image

Copy link
Member

@hjiangsu hjiangsu left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -162,7 +162,7 @@ class _MediaViewState extends State<MediaView> with SingleTickerProviderStateMix
},
pageBuilder: (BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {
return ImageViewer(
url: widget.postViewMedia.media.first.thumbnailUrl ?? widget.postViewMedia.media.first.originalUrl!,
url: widget.postViewMedia.media.first.imageUrl,
Copy link
Member

@hjiangsu hjiangsu Jun 5, 2024

Choose a reason for hiding this comment

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

Just as a quick note, this might bring back longer loading times for the image URL since we're prioritizing the full image now rather than the thumbnail. This might not be too big of a deal though considering we're deferring the image dimensions!

Regardless, we should monitor the impact this has on loading images 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, that's a good point. I'm not sure the best way to handle this, though, because people could be equally upset about the long load times as the fact that we weren't showing the full-resolution image. It's kind of a lose-lose. 😆

Copy link
Collaborator

Choose a reason for hiding this comment

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

Relay used to have and "HD" button in the image viewer which had to be pressed to load full-res images.
You could override this with a setting to always load the full-res image, if bandwidth and time weren't a concern.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's definitely a good idea for a future enhancement.

  • By default, load the thumbnail; have a button for the user to load full-res
  • Have an option to load full-res always, only on Wi-Fi, etc. (similar to video options)

@hjiangsu hjiangsu merged commit f3586b9 into thunder-app:develop Jun 5, 2024
1 check passed
@micahmo micahmo deleted the feature/improve-link-sharing branch June 5, 2024 18:35
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