Fix #6409: Add listener call in ImageAdapter to update UI and upload #6420

Kota-Jagadeesh · 2025-09-06T20:09:37Z

Changes made:

Fixed Issue Custom selector: Cannot deselect pictures #6409 by adding imageSelectListener.onSelectedImagesChanged in the deselection branch of onThumbnailClicked in ImageAdapter.kt. This ensures the UI updates correctly when an image is deselected, including:
- Updating the title (e.g., from "21 pictures selected" to "20 pictures selected").
- Hiding the "MAX: 20" warning when the selected image count drops to 20 or below.
- Enabling the upload button when the selection is within the limit and no images are marked as not for upload.

Verification:

Selected 21 images; verified "MAX: 20" warning appears and upload button is disabled.
Deselected one image; confirmed title updates to "20 pictures selected", warning disappears, and upload button is enabled.
Tested with the "Show already actioned images" switch on/off to ensure consistent behavior.

VIDEO

COMMONS.TEST.mp4

Fixes #6409

…and upload button on deselection

Kota-Jagadeesh · 2025-09-08T19:11:48Z

@nicolas-raoul If you are free, Can you review my PR : )

nicolas-raoul · 2025-09-10T14:02:41Z

Thanks! :-) And sorry for the delay.

Any idea what is the problem here?

ImageAdapterTest > selectOrRemoveImage FAILED
    java.lang.NoSuchMethodException at ImageAdapterTest.kt:174

nicolas-raoul

Just tested, it is working great when Show already actioned pictures is enabled. :-)

Would you mind checking the unit test failure?

nicolas-raoul · 2025-09-10T14:15:57Z

app/src/main/java/fr/free/nrw/commons/customselector/ui/adapter/ImageAdapter.kt

-        } else {
-            selectOrRemoveImage(holder, position)
-        }
-    }


Isn't this code useful when Show already actioned pictures (at the top of the activity) is disabled? Parts of this mode have been broken for a few months though, so it is not easy to test. By the way if you find a way to fix that it would be wonderful! ^_^

You’are right that the new code in onThumbnailClicked is intended to handle the case when "Show already actioned pictures" is disabled, ensuring clicks are only processed for actionable images in actionableImagesMap. I’ve noticed that this mode has issues (e.g., images not loading or clicks not registering properly). I’ll debug the processThumbnailForActionedImage and imageLoader.nextActionableImage logic to identify why actionable images aren’t populating correctly. I’ll add a fix to ensure the disabled switch mode works as expected, allowing users to select only actionable images, and include tests for this scenario.

I’ll update the PR with these fixes and provide details in the comments. Please let me know if there are specific test cases or behaviors for the disabled switch mode you’d like me to prioritize! ^_^

Fantastic!
I am a big user of that switch (or at least I was until it got broken in disabled mode), so I will be sure to send feedback quickly for your fixes. :-)

My upload workflow for context: I mark all family/work/blurry/uninteresting pictures as "Not for upload", then upload the rest. With "Show already actioned pictures" disabled, I have a clear view of what I need to upload or mark. It is a great time saver.

"I’ll add a fix to ensure the disabled switch mode works as expected"

Did you get a chance to try this, by any chance? :-) The latest commit still has the issue that if I tap a picture 2 times, it is counted as 2 uploads.

"I’ll add a fix to ensure the disabled switch mode works as expected"

Did you get a chance to try this, by any chance? :-) The latest commit still has the issue that if I tap a picture 2 times, it is counted as 2 uploads.

Hey @nicolas-raoul , thank you for flagging the issue again! I’ve addressed the double-tap problem in the latest commit by adding a check in selectOrRemoveImage to prevent duplicate image selections when showAlreadyActionedImages is false. I have tested it and it works for me ,Could you please test it and let me know if it resolves the issue? Thanks!

Kota-Jagadeesh · 2025-09-10T15:50:53Z

Just tested, it is working great when Show already actioned pictures is enabled. :-)

Would you mind checking the unit test failure?

Hi @nicolas-raoul,

Thank you for testing and for the feedback! I'm glad the fix works well when "Show already actioned pictures" is enabled. 😊

Regarding the Unit Test Failure:
I’ll investigate the failing unit test(s) in ImageAdapterTest.kt to identify the root cause. I suspect it might be related to mocking the ImageSelectListener or ImageLoader interactions in the deselection logic. I’ll check the test logs, update the test to align with the new onThumbnailClicked logic, and ensure coverage for both switch states. I’ll push an updated commit with the fixed test(s) shortly.

…ommons-app#6409)

Kota-Jagadeesh · 2025-09-13T11:50:53Z

@nicolas-raoul Updated the PR : )

Changes Made

Modified the selectOrRemoveImage function in ImageAdapter.kt to call imageSelectListener.onSelectedImagesChanged after an image is deselected, ensuring the parent UI is notified of the updated selection count.
Ensured numberOfSelectedImagesMarkedAsNotForUpload is correctly updated during deselection.
Preserved existing functionality, including single selection mode, SHA1 checks for uploaded images, handling of showAlreadyActionedImages switch states, and RecyclerView notifications.
The fix allows the UI to reflect the correct number of selected images and enables the upload button when the count drops to 20 or below.

Testing

Tested by selecting 21 images, verifying the "MAX: 20" error appears, then deselecting one image to confirm the error disappears and the upload button becomes active.
Verified that single selection mode, "not for upload" states, and already uploaded image checks remain unaffected.
Ensured both showAlreadyActionedImages switch states (true/false) work as expected.

Fixes #6409

nicolas-raoul · 2025-09-13T14:22:36Z

app/src/main/java/fr/free/nrw/commons/customselector/ui/adapter/ImageAdapter.kt

                            uploadingContributionList
                        )
                        _isLoadingImages.value = false
-                        // If the position is already visited, that means the image is already present


This PR removes a lot of comments here and below, is that intended?

By the way it seems to be working, I will test more tomorrow.

I apologize for the confusion regarding the removed comments in the PR. The removal was unintentional and likely occurred during code cleanup to focus on the core fix for issue #6409. I’ve added the comments back to maintain clarity and documentation as per the original code. The changes still address the deselection issue by ensuring the UI updates correctly via the listener. Please let me know if there’s anything else I should adjust!

…hen showAlreadyActionedImages is off (commons-app#6409)

Kota-Jagadeesh · 2025-09-13T19:30:41Z

Changes Made

Fixed selectOrRemoveImage in ImageAdapter.kt to stop counting the same image multiple times on repeated taps.
Ensured compatibility with both "Show already actioned images" states.
Preserved existing features: single selection mode, SHA1 checks, and "not for upload" states.

Testeng

Rapidly tapped images to confirm no duplicate selections.
Verified smooth select/deselect behavior.
Checked single selection mode, uploaded checks, and "not for upload" states.
Confirmed earlier fix for Custom selector: Cannot deselect pictures #6409 still works.

github-actions · 2025-09-13T19:35:34Z

✅ Generated APK variants!

🤖 Download betaDebugAPK
🤖 Download prodDebugAPK

nicolas-raoul · 2025-09-14T13:41:26Z

This commit worked great in my tests today! :-)
Testing a bit more just to make sure before merging...

Kota-Jagadeesh · 2025-09-14T14:01:21Z

This commit worked great in my tests today! :-) Testing a bit more just to make sure before merging...

Okay 👍

Kota-Jagadeesh requested review from nicolas-raoul and removed request for nicolas-raoul September 6, 2025 20:10

Fix commons-app#6409: Add listener call in ImageAdapter to update UI …

cd09c81

…and upload button on deselection

Merge branch 'main' into Fix/custom-selector-maximum-limit

b2341c8

nicolas-raoul reviewed Sep 10, 2025

View reviewed changes

Fix image deselection issue in ImageAdapter to update UI correctly (c…

633210d

…ommons-app#6409)

Merge branch 'main' into Fix/custom-selector-maximum-limit

a91d728

nicolas-raoul reviewed Sep 13, 2025

View reviewed changes

Prevent duplicate image selections on multiple taps in ImageAdapter w…

a1d37e1

…hen showAlreadyActionedImages is off (commons-app#6409)

nicolas-raoul approved these changes Sep 15, 2025

View reviewed changes

nicolas-raoul merged commit 657af4f into commons-app:main Sep 15, 2025
1 check passed

Fix #6409: Add listener call in ImageAdapter to update UI and upload #6420

Fix #6409: Add listener call in ImageAdapter to update UI and upload #6420

Uh oh!

Conversation

Kota-Jagadeesh commented Sep 6, 2025

VIDEO

Uh oh!

Kota-Jagadeesh commented Sep 8, 2025

Uh oh!

nicolas-raoul commented Sep 10, 2025

Uh oh!

nicolas-raoul left a comment

Choose a reason for hiding this comment

Uh oh!

nicolas-raoul Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

Kota-Jagadeesh Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

nicolas-raoul Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

nicolas-raoul Sep 13, 2025

Choose a reason for hiding this comment

Uh oh!

Kota-Jagadeesh Sep 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kota-Jagadeesh commented Sep 10, 2025

Uh oh!

Kota-Jagadeesh commented Sep 13, 2025

Changes Made

Testing

Uh oh!

nicolas-raoul Sep 13, 2025

Choose a reason for hiding this comment

Uh oh!

Kota-Jagadeesh Sep 13, 2025

Choose a reason for hiding this comment

Uh oh!

Kota-Jagadeesh commented Sep 13, 2025

Changes Made

Testeng

Uh oh!

github-actions bot commented Sep 13, 2025

Uh oh!

nicolas-raoul commented Sep 14, 2025

Uh oh!

Kota-Jagadeesh commented Sep 14, 2025

Uh oh!

Uh oh!

Uh oh!

Kota-Jagadeesh Sep 13, 2025 •

edited

Loading