Closed rohit9625 closed 1 week ago
I get Could not find com.dinuscxj:circleprogressbar:1.1.1
.
This repository/branch solves the problem, so would you mind pulling from it? git@github.com:kanahia1/apps-android-commons.git:issue5583 Or just the circleprogressbar parts if you prefer, but pulling the whole branch would be more efficient.
Thanks a lot! :-)
I get
Could not find com.dinuscxj:circleprogressbar:1.1.1
.This repository/branch solves the problem, so would you mind pulling from it? git@github.com:kanahia1/apps-android-commons.git:issue5583 Or just the circleprogressbar parts if you prefer, but pulling the whole branch would be more efficient.
Thanks a lot! :-)
That branch has so many changes and I looked at the progress bar part. The code was removed and commented out as I did on my local machine. What about I add a new progress bar using Jetpack and push the latest changes or wait for kanahia's PR to get merged?
"What about I add a new progress bar using Jetpack"
That would be fantastic, thanks a lot. π Even a simple label x/y would be enough for now.
"What about I add a new progress bar using Jetpack"
That would be fantastic, thanks a lot. π Even a simple label x/y would be enough for now.
The fragments were built in Java so I had to replace the circular progress bar with the material one(XML Views).
Hey @nicolas-raoul, please try opening settings from the app. On my device, the app is crashing and maybe, this is the reason that testSetTotalUploadCount
is failing.
It seems like I am able to open settings (GSoC branch):
I just tested this branch, it builds fine. π
The 3 stats (for an empty account) all appear differently, is it expected? If difficult don't worry, priority is low so we can make this a different issue. π
I just tested this branch, it builds fine. π
The 3 stats (for an empty account) all appear differently, is it expected? If difficult don't worry, priority is low so we can make this a different issue. π
I didn't checked for an empty account. Checking if it is minor bug, then I'll fix it.
Ah yes Settings are crashing on this branch.
It seems like I am able to open settings
Well, I think that upgrading to SDK 34 has some other changes for AppCompatActivity
class. The error I encountered:-
Β java.lang.IllegalStateException: AppCompat has already installed itself into the Window
That is because The AppCompatActivity class already takes care of creating and managing the AppCompatDelegate instance for us.
And we are managing AppCompatDelegate
in our settings activity. Removing that will solve the bug.
I just tested this branch, it builds fine. π
The 3 stats (for an empty account) all appear differently, is it expected? If difficult don't worry, priority is low so we can make this a different issue. π
I don't have an empty account. Please let me know if there's any way that I can test this case. Is there any dummy account or do I need to create a new account?
Don't worry about empty accounts. Would you mind just posting a screenshot showing how it looks like on your account?
Don't worry about empty accounts. Would you mind just posting a screenshot showing how it looks like on your account?
I have added the screenshot in the description of this PR.
I was able to test with a non-empty account now, looks great! :-) Do not worry about empty accounts.
I found out the reason behind the crash. I added this dependency when migrating to Compose because the activity having Composables
was crashing. As far as I remember the reason was related to lifecycle and on the internet I found this dependency to include.
But, now everything is working fine after removing this. However, AppCompat
automatically manages the AppCompatDelegate
. The SettingsActivity
manages that delegate itself, causing the crash.
Would you mind rebasing (or pulling) from main? Thanks! :-)
Unit tests pass and the app seems to work fine, I just uploaded a few pictures using this branch :-) Is there any blocker before merging this? The discussion on permissions?
I also think that migration is complete but I have a confusion about this service defined in the XML:- https://github.com/commons-app/apps-android-commons/pull/5790#discussion_r1739998508
I am waiting for @RitikaPahwa4444 suggestions on this. Do you have any idea about this?
Apologies for missing out on the comment. I've not worked closely with those services and will have to check and confirmπ
No problem, I am waiting for your review :)
Any idea why the app is no longer showing notification for the foreground service or even the ongoing uploads? We currently display two of them, but on this branch I see none.
Any idea why the app is no longer showing notification for the foreground service or even the ongoing uploads? We currently display two of them, but on this branch I see none.
No idea :( I am looking into it. Can you please test the location problem again?
Both notifications are showing when I am uploading new images. Make sure you have notification permission.
It didn't ask me for notification permission at all. Happened on a fresh installation. The app asks for it in the beginning itself on the main branch
But, it asked me when I opened that newly implemented screen for managing uploads. Same behavior on the main branch. I think the app is asking for notification permission only when needed or after some time, not sure either.
I think the app is asking for notification permission only when needed or after some time, not sure either.
I've tried reinstalling the app to observe if that notification permission pops up, but I'm still waiting. It's been over 15 minutes already, I'm uploading pictures but not getting any notification or any dialog requesting access.
Custom selector is now working fine π The location info is not getting redacted and I can see the Commons logo on already uploaded images now
Custom selector is now working fine π The location info is not getting redacted andi can see the Commons logo on already uploaded images now
What is the procedure to repeat that location reading process? Need to check if that is working fine on the main or not.
Closed and reopened the app and got this crash:
STACK_TRACE=com.bumptech.glide.load.engine.CallbackException: Unexpected exception thrown by non-Glide code
at com.bumptech.glide.load.engine.EngineJob.callCallbackOnResourceReady(EngineJob.java:161)
at com.bumptech.glide.load.engine.EngineJob$CallResourceReady.run(EngineJob.java:428)
at android.os.Handler.handleCallback(Handler.java:1000)
at android.os.Handler.dispatchMessage(Handler.java:104)
at android.os.Looper.loopOnce(Looper.java:242)
at android.os.Looper.loop(Looper.java:362)
at android.app.ActivityThread.main(ActivityThread.java:8393)
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:992)
Caused by: java.lang.IllegalStateException: Fragment ExploreMapFragment{dd4f3da} (c3906b03-bb95-4c56-8865-cf6135e5da37) not attached to a context.
at androidx.fragment.app.Fragment.requireContext(Fragment.java:900)
at androidx.fragment.app.Fragment.getResources(Fragment.java:964)
at fr.free.nrw.commons.explore.map.ExploreMapFragment.addMarkerToMap(ExploreMapFragment.java:649)
at fr.free.nrw.commons.explore.map.ExploreMapFragment.addMarkersToMap(ExploreMapFragment.java:636)
at fr.free.nrw.commons.explore.map.ExploreMapPresenter.onNearbyBaseMarkerThumbsReady(ExploreMapPresenter.java:182)
at fr.free.nrw.commons.explore.map.ExploreMapController$1.onResourceReady(ExploreMapController.java:180)
at fr.free.nrw.commons.explore.map.ExploreMapController$1.onResourceReady(ExploreMapController.java:170)
at com.bumptech.glide.request.SingleRequest.onResourceReady(SingleRequest.java:639)
at com.bumptech.glide.request.SingleRequest.onResourceReady(SingleRequest.java:578)
at com.bumptech.glide.load.engine.EngineJob.callCallbackOnResourceReady(EngineJob.java:159)
... 9 more
Please don't worry if this is not related to your changes. Let's keep a track of this, though. I'll raise a separate issue if I face it frequently on the main branch as well.
I think the app is asking for notification permission only when needed or after some time, not sure either.
I've tried reinstalling the app to observe if that notification permission pops up, but I'm still waiting. It's been over 15 minutes already, I'm uploading pictures but not getting any notification or any dialog requesting access.
Did you try the same on the main. Because even on main branch the app is not requesting the notification permission. I tried it myself just few minutes ago.
Please don't worry if this is not related to your changes. Let's keep a track of this, though. I'll raise a separate issue if I face it frequently on the main branch as well.
Okay π, that'll be nice :) Even I also noticed a behavior related to pausing uploads. I'll create a new issue for that.
Because even on main branch the app is not requesting the notification permission
But you still see the notifications, right? π€
But you still see the notifications, right? π€
No, I tried uploading a new picture from the main branch version but no notifications showed up. So, I opened the screen where we can manage uploads and then the app asked me for notification permission. After that, the uploading seemed to restart (Not sure) and both notifications were showing.
I've opened the pending uploads screen several times on this branch but didn't get that permission popup. Testing on main now.
I've opened the pending uploads screen several times on this branch but didn't get that permission popup. Testing on main now.
Just to be sure. Did you open that screen just after starting a new upload? Because ongoing uploads are the reason to ask for notification permission, not opening that screen only. However, it seems another issue and out of the scope of this PR. What do you think?
Did you open that screen just after starting a new upload?
Yes.
However, it seems another issue and out of the scope of this PR
Bumping versions does break things. And if notifications for foreground services have disappeared without the user dismissing them, we cannot rest assured that they would work as intended without tesing completely. I'm sorry if you found this unrelated - I'm just testing and sharing what is different on this branch.
Custom selector is now working fine π The location info is not getting redacted and I can see the Commons logo on already uploaded images now
I tried uploading a picture that has a location in its EXIF and the app can read the location info from EXIF of the image after the recent commit. However, I am getting these errors on the logs on this PR and main as well.
However, it seems another issue and out of the scope of this PR
Bumping versions does break things. And if notifications for foreground services have disappeared without the user dismissing them, we cannot rest assured that they would work as intended without testing completely. I'm sorry if you found this unrelated - I'm just testing and sharing what is different on this branch.
You are right, bumping to a new SDK definitely can break other parts. However, it's working on API 34 but the behavior could be different on other versions. What API level you are testing on? Thank you once again for your insights :)
I'm testing on Android 14 (API level 34).
I'm testing on Android 14 (API level 34).
Now, what am I supposed to do? I am so confused :(
@RitikaPahwa4444, I can make the app request Notification Permission
at the startup to solve the problem. However, users can decline the permission. Would you happen to have any reference to the current implementation of requesting notification permission?
Hey @rohit9625, I would need some time to check on the notifications issue along with the issue mentioned in this comment. Until then, please don't consider them as a blocker. The location issue seems sorted for me at least, but I'll share if I find any errors in my logs. Keeping the notifications issue aside, uploads seem to work fine for me while I'm using the app.
Thank you @RitikaPahwa4444 & @nicolas-raoul for your time on this PR :) Now, if everything seems sorted then what about merging this PR?
I believe Ritika wrote that there is no blocker, so I will merge this. I uploaded hundreds of pictures with this branch and it worked fine. Unit tests passing too. Thanks a lot @rohit9625, this was high priority because it was blocking further releases.
Description (required)
Fixes #5770
What changes did you make and why? 1. Add functionality to work with
Partial Access
on API >= 34ComposeView
.READ_MEDIA_VISUAL_USER_SELECTED
permission in Manifest File and limit some permissions to a specific SDK.2. Upgrade AGP to latest and targetSDK/compileSDK to 34
3. Minor refactoring
Partial Access
was causing some crashes for specific actions. That's why I need to make some changes to files under theCustom Selector
package.4. Replace deprecated circular progress bar
activity_quiz_result
andfragment_achievements
and add new material circular progress barTests performed (required)
Tested ProdDebug on Samsung A14(API 34), Realme Narzo 50(API 33), Pixel 5 Emulator(API 29), and Pixel 6 Emulator(API 31).
Screenshots (for UI changes only)