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 Custom image selector #5576

Merged
merged 7 commits into from
Mar 16, 2024

Conversation

shashankiitbhu
Copy link
Contributor

@shashankiitbhu shashankiitbhu commented Feb 29, 2024

Description (required)

Fixes #5558

What changes did you make and why?

Tests performed (required)

Tested {build variant, e.g. ProdDebug} on {name of device or emulator} with API level {API level}.

Screenshots (for UI changes only)

WhatsApp.Video.2024-02-29.at.7.21.14.PM.mp4

@nicolas-raoul
Copy link
Member

Testing now...

Sometimes after marking many pictures as "not for upload" I see a large number of white squares at the bottom, not sire whether it is intended?
Also I had an instance where the last loaded picture at the bottom was not selectable.

This is a huge improvement, I already feel much more efficient thanks to this change. :-) I continue testing.

@nicolas-raoul
Copy link
Member

Examples:

Screenshot_20240303-151913.png

Screenshot_20240303-151822.png

@shashankiitbhu
Copy link
Contributor Author

shashankiitbhu commented Mar 4, 2024

Examples:

Screenshot_20240303-151913.png

Screenshot_20240303-151822.png

@nicolas-raoul
On My device, these white squires appear for a short time only (until they are loaded, and this happens only for folders with a lot of Images ), also these aren't empty white squares they are also loaded Images but for some of them the thumbnail processing takes a bit more time (which you can perform for any of those white squares immediately by clicking on any of them), I have tested it and came to a conclusion that all Images are loaded but some of them take some time to load the thumbnails.

Previously loading images was too slow and it had enough to process each thumbnail but now image is loaded much faster so it takes some time to load.

All the Images Eventually Load without fail, and the loading speed depends on the method processThumbnailForActionedImage(holder, position) and is unrelated to this, we can check the possibilities for optimisation for that method later on in another issue maybe?

@shashankiitbhu
Copy link
Contributor Author

WhatsApp.Video.2024-03-04.at.8.18.48.PM.mp4

Here is an example of how everything eventually loads up

@nicolas-raoul
Copy link
Member

nicolas-raoul commented Mar 5, 2024

Yes I also observed that moving the empty squares out of visible screen, and then back into visible screen, makes them load correctly. Waiting does not seem to make them appear.

Another small issue: pictures marked as "not for upload" are removed from the view even if "show already actioned pictures" is enabled:

1000038094.mp4

Both are not big issues compared to the benefit brought by this PR, so I am happy to file 2 new issues and merge for now, if you agree. 🙂

@shashankiitbhu
Copy link
Contributor Author

shashankiitbhu commented Mar 5, 2024

@nicolas-raoul Sure, I can work on it separately, and given my understanding of this issue and the changes I have made, I think we'll get a reliable solution soon.

Another small issue: pictures marked as "not for upload" are removed from the view even if "show already actioned pictures" is enabled:

Adding a commit to Fix this one here

@shashankiitbhu
Copy link
Contributor Author

shashankiitbhu commented Mar 5, 2024

Another small issue: pictures marked as "not for upload" are removed from the view even if "show already actioned pictures" is enabled:

@nicolas-raoul
Fixed this one in the latest Commit:

WhatsApp.Video.2024-03-05.at.8.45.57.PM.mp4

@shashankiitbhu
Copy link
Contributor Author

@nicolas-raoul If the Above works can you please review and merge it, thanks

Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

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

A few bugs subsist but hugely helpful fix, thank you! :-)

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.

Custom image selector: Do not reload everything after marking a picture as not for upload
3 participants