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

Fix jank when using full height images, and other misc media preview imrpovements #1250

Merged
merged 6 commits into from
Mar 31, 2024

Conversation

hjiangsu
Copy link
Member

Pull Request Description

This PR fixes an issue with layout jank that occurs when using full height images and a few other minor issues.

  • Fix layout jank due to thumbnailUrl not being present in some occasions
  • Added height values to ViewMode enums to remove our scattered usage of constants
  • Added a timeout when attempting to fetch image dimensions - this should help with some loading times if the response times of the image URL is slow. This does mean that some images may look squished or stretched if a timeout occurs but it might be worth it for faster loading. Ideally, the Lemmy API should provide the height/width dimensions so that we don't have to manually fetch these dimensions
  • Added a background color to the image container when the image has not loaded yet

If any image dimensions look off after this change, let me know!

Issue Being Fixed

Issue Number: N/A

Screenshots / Recordings

Checklist

  • Did you update CHANGELOG.md?
  • Did you use localized strings where applicable?
  • Did you add semanticLabels where applicable for accessibility?

Copy link
Member

@micahmo micahmo left a comment

Choose a reason for hiding this comment

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

Hmm, I tried testing these changes and right off the bat the feed is filled with stretched images. Is it worth the tradeoff of faster loading?

image

@hjiangsu
Copy link
Member Author

Nice catch! I didn't test that out. I'll check it out and see - the stretching should only happen on card view

@hjiangsu
Copy link
Member Author

Hmm, it looks correct on my device. I'm not sure what's causing the stretching. I'm out at the moment so I can't check on the emulator!

image

@hjiangsu
Copy link
Member Author

hjiangsu commented Mar 30, 2024

Just tested it on Android as well and ti looks okay to me. Could you double check if you can reproduce the stretching?

image image

@micahmo
Copy link
Member

micahmo commented Mar 31, 2024

Alright I'm not sure what was going on, but I can't repro any more either!

@hjiangsu hjiangsu merged commit f7686f2 into develop Mar 31, 2024
1 check passed
@hjiangsu hjiangsu deleted the fix/full-image-height-jank branch March 31, 2024 19:42
@hjiangsu hjiangsu mentioned this pull request Apr 3, 2024
3 tasks
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.

2 participants