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

Custom picker: Show differently pictures that are currently being uploaded #5618

Merged
merged 11 commits into from
Mar 22, 2024

Conversation

shashankiitbhu
Copy link
Contributor

Description (required)

Fixes #5277

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}.

@nicolas-raoul
Copy link
Member

I keep getting this crash with this branch:

screen-20240316-222824.mp4
STACK_TRACE=java.lang.IllegalStateException: Cannot access database on the main thread since it may potentially lock the UI for a long period of time.
at androidx.room.RoomDatabase.assertNotMainThread(RoomDatabase.kt:435)
at androidx.room.RoomDatabase.query(RoomDatabase.kt:476)
at androidx.room.util.DBUtil.query(DBUtil.kt:75)
at fr.free.nrw.commons.contributions.ContributionDao_Impl$7.call(ContributionDao_Impl.java:1467)
at fr.free.nrw.commons.contributions.ContributionDao_Impl$7.call(ContributionDao_Impl.java:1464)
at androidx.room.RxRoom$5.subscribe(RxRoom.java:232)
at io.reactivex.internal.operators.single.SingleCreate.subscribeActual(SingleCreate.java:39)
at io.reactivex.Single.subscribe(Single.java:3603)
at io.reactivex.Single.blockingGet(Single.java:2834)
at fr.free.nrw.commons.customselector.ui.selector.ImageFragment.handleResult(ImageFragment.kt:272)
at fr.free.nrw.commons.customselector.ui.selector.ImageFragment.onCreateView$lambda$1(ImageFragment.kt:203)
at fr.free.nrw.commons.customselector.ui.selector.ImageFragment.$r8$lambda$S7kzNRAlIUqfrtaQwFc4UhtsOKM(Unknown Source:0)
at fr.free.nrw.commons.customselector.ui.selector.ImageFragment$$ExternalSyntheticLambda0.onChanged(Unknown Source:4)
at androidx.lifecycle.LiveData.considerNotify(LiveData.java:133)
at androidx.lifecycle.LiveData.dispatchingValue(LiveData.java:151)
at androidx.lifecycle.LiveData.setValue(LiveData.java:309)
at androidx.lifecycle.MutableLiveData.setValue(MutableLiveData.java:50)
at androidx.lifecycle.LiveData$1.run(LiveData.java:93)
at android.os.Handler.handleCallback(Handler.java:959)
at android.os.Handler.dispatchMessage(Handler.java:100)
at android.os.Looper.loopOnce(Looper.java:232)
at android.os.Looper.loop(Looper.java:317)
at android.app.ActivityThread.main(ActivityThread.java:8501)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:552)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:878)

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.

Would you mind checking what might be causing the crash? Thanks!

@shashankiitbhu
Copy link
Contributor Author

Sure

@shashankiitbhu
Copy link
Contributor Author

@nicolas-raoul I Fixed the crash, Please Review

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.

Thanks for fixing the crash!
I tested like this:

  1. Launch the upload of a picture
  2. Immediately go to the custom picker. Uploading overlay unfortunately not shown
  3. Go back to contributions
  4. Go to the custom picker again. This time the uploading overlay is visible.

Any idea why the uploading overlay was not visible at step 2?
Thanks!

@shashankiitbhu
Copy link
Contributor Author

shashankiitbhu commented Mar 17, 2024

@nicolas-raoul did that upload Fail while the custom Image Picker was loading ?

@shashankiitbhu
Copy link
Contributor Author

@nicolas-raoul Fixed it to handle all cases, Please Review

WhatsApp Image 2024-03-17 at 8 56 49 PM

@shashankiitbhu
Copy link
Contributor Author

@nicolas-raoul Please Review and Let me know if these changes works as Intended, thanks

nicolas-raoul

This comment was marked as resolved.

@nicolas-raoul
Copy link
Member

Ah sorry it was my mistake, I can see the white arrows now :-)

@nicolas-raoul
Copy link
Member

I also see unnecessary arrows on pics that I uploaded several days ago. Any idea why?

Screenshot_20240321-231818.png

@shashankiitbhu
Copy link
Contributor Author

@nicolas-raoul Thanks for spotting the issue, fixed it in the latest commit, please review, thanks

@nicolas-raoul
Copy link
Member

The arrows disappear when disabling then enabling "Show already actioned pictures".

By the way, if not difficult: Ideally pictures that are being uploaded should be hidden when "Show already actioned pictures" is disabled. That would allow the user to focus on pictures that need to be uploaded from now. 🙂 That can be a different GitHub issue.

screen-20240322-074002.mp4

@shashankiitbhu
Copy link
Contributor Author

By the way, if not difficult: Ideally pictures that are being uploaded should be hidden when "Show already actioned pictures" is disabled. That would allow the user to focus on pictures that need to be uploaded from now. 🙂 That can be a different GitHub issue.

@nicolas-raoul
That's a great idea, I think I can fix that in this PR only (no need for a new Issue), that would save quite some time and will also make reviewing easier.

# Conflicts:
#	app/src/main/java/fr/free/nrw/commons/customselector/ui/selector/ImageFragment.kt
@shashankiitbhu
Copy link
Contributor Author

@nicolas-raoul I have completed following in above commits:

  1. Fixed this Issue

The arrows disappear when disabling then enabling "Show already actioned pictures".

  1. Added this Functionality

Ideally pictures that are being uploaded should be hidden when "Show already actioned pictures" is disabled.

Please Review and Merge, 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.

It is working great now, thanks a lot!

@nicolas-raoul nicolas-raoul merged commit 2d333a2 into commons-app:main Mar 22, 2024
1 check passed
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 picker: Show differently pictures that are currently being uploaded
2 participants