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

Add support for managing uploaded media #1323

Open
wants to merge 21 commits into
base: develop
Choose a base branch
from

Conversation

micahmo
Copy link
Member

@micahmo micahmo commented Apr 23, 2024

Pull Request Description

This PR adds the ability to delete uploaded images. There is a new section under account settings for managing media. Navigating to this new page will display all uploaded images with the option to delete any of them. This is only supported for instances running >= 0.19.4.

See: thunder-app/lemmy_api_client#21.

Issue: #1447

Screenshots / Recordings

manage-media-demo.mp4

P.S. While working on this feature, I discovered something really interesting! I've always been annoyed at how seemingly quickly our image caches reset despite having the new aggressive image caching setting. It turns out that this is less about image caching and more about widget caching. The default behavior of SliverLists is to pretty aggressive dispose and rebuild widgets as they're scrolled off/on screen, and this naturally causes some loading artifacts, even for cached images. However, this trick seems to make things much better.

Pass these paramters to the SliverList.

addSemanticIndexes: false,
addAutomaticKeepAlives: false,
addRepaintBoundaries: false,

And then wrap whatever is the root widget in the list with a KeepAlive(keepAlive: true).

For now I've only done it here, but maybe we could consider doing this in other places of the app (respecting the aggressive image caching option). Here's a before/after of just that difference.

Before

scroll-reload-before.mp4

After

scroll-reload-after.mp4

@micahmo
Copy link
Member Author

micahmo commented Apr 24, 2024

I decided to add on a slight improvement to this feature. I added the ability to search for usages of the media link. It can help you to decide whether you really want to delete it. It should return cases where the link appears as the URL of a post, in the body of a post, or in the body of a comment. Of course, this method is not foolproof, as the link could be used in some other way (including not even on Lemmy). But it should at least help the user to make a more informed decision.

qemu-system-x86_64_TahRys0qlj.mp4

@micahmo micahmo mentioned this pull request Jun 14, 2024
7 tasks
@micahmo
Copy link
Member Author

micahmo commented Jun 25, 2024

@hjiangsu Would you be able to give me a hand figuring out why the tests are failing here? They're saying something about not finding freezed. But I didn't remove that package from the pubspec here, nor in the corresponding change in the API repo. So I'm a bit stuck! 😊

@hjiangsu
Copy link
Member

Hmm, not entirely sure what's causing the failures. I glanced at the CI logs and did see this: https://github.com/thunder-app/thunder/actions/runs/9664871877/job/26661495345?pr=1323#step:5:170

I'm not sure if this is what's causing the subsequent failures, but it could be something!

image

@micahmo
Copy link
Member Author

micahmo commented Jun 25, 2024

I saw that too, which is what made me think I removed it by accident, but I didn't. 😆 Maybe I should just add it?

@micahmo
Copy link
Member Author

micahmo commented Jun 25, 2024

Maybe I should just add it?

Ok that seems to have worked! It really just changed the reference type in the pubspec.lock from transitive to direct main.

Now it's failing at the linting step, despite apparently not showing any errors (only info/warning). Lol!

EDIT: Nevermind, I see the error. Not sure why I'm not getting it locally.

https://github.com/thunder-app/thunder/actions/runs/9666355363/job/26665643096?pr=1323#step:10:198

@micahmo
Copy link
Member Author

micahmo commented Jun 26, 2024

Finally got this building! 😌

@micahmo micahmo force-pushed the feature/delete-media branch from 1d670bb to da9b900 Compare July 12, 2024 22:10
@micahmo
Copy link
Member Author

micahmo commented Oct 21, 2024

@hjiangsu Alrighty, this is my next PR based on age. I know this is a big one in terms of changes, but it's one that I'm very excited to have. I think it's especially useful since it's related to the new (well, not so new any more 😆) Lemmy version, so folks might be expecting this behavior to be available once their instance is upgraded. Let me know what you think!

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