-
-
Notifications
You must be signed in to change notification settings - Fork 283
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
[BUG] Images flash white when post is interacted with #1352
Comments
@whattherestimefor I would like to take on this issue. But first, I want to discuss a few things. This bug occurs because, after applying a new snapshot to UITableView, DiffableDataSource reloads the cell instead of reconfiguring it. We could potentially use the new However, I’m considering a different approach. Inside I also dislike the idea of removing views from the view hierarchy. We could simply toggle their isHidden property. A better solution would be to use different views for different types of media content, but that would also require substantial changes. What do you think about the proposed solution? Or are there other methods I might have overlooked or am unaware of? |
@tvrrp Thanks so much. I think the simpler approach is worth a try, but let me provide some background/additional info: The current state of things is kind of in-between. The app was originally developed with everything in CoreData and the cells updating their own state whenever they observed changes. CoreData has been mostly removed (too much overhead and we really didn't need to save most of that info across app launches at all), but the system of mutable data objects updating the cells remained. That system was (is) very hard to read and very brittle when attempting to make new changes. What you're seeing now is the first effort at moving towards the cells being much simpler, displaying immutable data that gets replaced when changes happen. That transition is definitely introducing some bugs, like this one. Additionally, I'm currently overhauling the notifications view. We will be using SwiftUI at least for the individual cells, and might end up using SwiftUI List rather than UITableView overall. The data model is also changing (or at least starting to), with the Mastodon objects being cached based on their id instead of wrapped up together with all their related objects and view state (like whether a particular content warning should be blurred out or not). Which is all to say, I hope that almost none of the code involved in this bug will still be around by the end of this year. However, it would be great to fix it in the meantime, and your plan sounds reasonable to me if you are still willing to take it on. |
@whattherestimefor some updates. I kinda failed with a simpler approach 🙁 It won’t work with the current stack — a diffable data source (DDD) with large and heavy cells. Simply ignoring prepareForReuse will not work, because the original view will be replaced with a new one. I think this could be fixed by splitting the large content cell into smaller, separate parts. With this approach, DDD might only recreate the cells containing the favorites count, as the hashes for the other cells wouldn’t change. However, this would require extensive changes and feels somewhat pointless since it would eventually be replaced by new stuff. |
Is there an existing issue for this?
Current Behavior
When I favourite a post that contains an image, the image briefly flashes white then reappears. This is especially annoying for GIFs because it also resets the play position to the beginning. For static images it feels a bit glitchy/buggy, but isn’t a big problem.
Expected Behavior
Favouriting a post should not make images flash or reset the play position of GIFs.
Steps To Reproduce
Environment
Anything else?
No response
The text was updated successfully, but these errors were encountered: