android icon indicating copy to clipboard operation
android copied to clipboard

[BUG] Confusing loading state in Battery Saver

Open graciouselectric opened this issue 1 year ago • 14 comments

Actual behaviour

Progress bar appears and animates

Expected behaviour

Nothing or non-animating circle is shown

Steps to reproduce

  1. Enable Battery Saver on e.g Android 8.1
  2. Enter server details, submit

Can this problem be reproduced with the official owncloud server? (url: https://demo.owncloud.org, user: test, password: test)

Yes

Environment data

Android version: 8.1

Device model: Pixel 7 Emulator

Stock or customized system: Stock

ownCloud app version: 4.2.1

ownCloud server version: -

Additional info

I noticed that when I enable Battery Saver on Android 8.1, the indeterminate ProgressBars in the app are not properly shown. This is a known problem in Android API level <28, see e.g. this StackOverflow question. Battery Saver disables animations, also on progress bars on these versions. This is quite confusing because the loading state is not properly represented. It is fixed in later Android versions, where progress bars do appear and animate.

Looking at the code, indeterminate progress bars are created here:

className='com.owncloud.android.ui.preview.FileDownloadFragment$listenForTransferProgress$1$1', lineNumber=222
className='com.owncloud.android.presentation.files.filelist.MainFileListFragment', lineNumber=1333
className='com.owncloud.android.presentation.files.filelist.MainFileListFragment$subscribeToViewModels$10', lineNumber=601)

To fix this issue, one can check ValueAnimator.areAnimatorsEnabled() and provide a different UI element, such as a text label, when animations are disabled.

I also recorded a video, showing the bug in practice:

https://github.com/owncloud/android/assets/165037835/812ef0f0-dd02-431d-a293-7f08ba58afc4

graciouselectric avatar May 07 '24 12:05 graciouselectric

Thanks for the report. This could be interesting for some community contribution.

Anyway, mind that this is an issue that only happen in old versions (Android 8 dated 2017) and with battery save mode, so, not many users will suffer this.

jesmrec avatar May 07 '24 15:05 jesmrec

Hi @graciouselectric! Thanks for opening a new issue! 🍻

I can see the code lines you refer to are not pointing to progress bars in the current code in master branch. Also, the video you recorded shows a static icon, but that's for every device in every mode, no need to be in battery saver mode in devices with Android API <28 (just tested in a Samsung Galaxy S9 with Android 9 and the icon appears as in your video). Progress bars are different stuff.

In any case, as @jesmrec said, contributions are always welcome 😄. You seem to have some idea of the code, if you feel like, I encourage you to send your own PR and we'll review it 🚀.

JuancaG05 avatar May 07 '24 15:05 JuancaG05

I would like to contribute to resolving this issue.

lakshay6907 avatar May 18 '24 11:05 lakshay6907

go ahead @lakshay6907

jesmrec avatar May 20 '24 06:05 jesmrec

I would like to know the server, login, and password to check the app. Could you please provide me with this information? @jesmrec

lakshay6907 avatar May 24 '24 06:05 lakshay6907

@lakshay6907 you can use the public server: https://ocis.owncloud.works with credentials einstein/relativity

jesmrec avatar May 24 '24 06:05 jesmrec

Is this issue still open?

lakshay6907 avatar Jun 22 '24 06:06 lakshay6907

Is this issue still open?

Yes, it is.

jesmrec avatar Jun 24 '24 06:06 jesmrec

Is this issue still open?

Hi @lakshay6907! It is open, but we need to clarify its scope before developing any solution. As commented above, the spinning wheel shown in the video is not a progress bar, so the video and the description of the issue are not aligned. Once we define exactly what we want to do here, we can go for a PR 👍

JuancaG05 avatar Jun 25 '24 06:06 JuancaG05

Is this issue still open?

Hi @lakshay6907! It is open, but we need to clarify its scope before developing any solution. As commented above, the spinning wheel shown in the video is not a progress bar, so the video and the description of the issue are not aligned. Once we define exactly what we want to do here, we can go for a PR 👍

Hello @JuancaG05 , thank you for your reply.

I am currently reviewing the entire login page activity and noticed that an in-built resource is defined in the TextView within the XML file. Additionally, multiple functions that reuse this TextView element also attempt to override the vector resource. This repeated definition of a single vector resource might be causing the issue.

If you need any further information or assistance, please feel free to contact me. I find this project very interesting and am eager to contribute.

XML file:- account_setup.xml TextView id:- server_status_text activity name:- LoginActivity.kt

Thank you.

lakshay6907 avatar Jun 29 '24 05:06 lakshay6907

Hi @lakshay6907! Glad to hear that! We'll be happy to receive contributions from you 😄.

Regarding the issue, yes, I see the server_status_text has a drawableStart which contains the icon shown there, and it is modified in the Kotlin activity depending on the status. It seems as well that the icon set in the XML file is not a vector but a PNG. So, what is your proposal here to improve it?

JuancaG05 avatar Jul 01 '24 06:07 JuancaG05

Hi @JuancaG05,

As I noticed the icon is modified frequently, I suggest using a vector image for the loading icon instead. This would ensure consistency in our loading icon and eliminate the need for repeated modifications. Additionally, this approach would free up space in the TextView for any other icon at the start. We can still utilize the existing PNG icon as needed.

I could be wrong here, so I would recommend seeking the opinions of others as well.

lakshay6907 avatar Jul 03 '24 10:07 lakshay6907

Hi @lakshay6907!

Changing the PNG by its corresponding vector could be an improvement. But:

  1. The PNG set by default in the XML file doesn't refer to the loading status, for which we use progress_small.xml that is already a vector
  2. That won't fix the problem stated in this issue, which BTW is not clear yet

So I'd keep asking for different thoughts and approaches, since the scope of the issue and the problem is not clear as I said

JuancaG05 avatar Jul 03 '24 12:07 JuancaG05

What I think is that the problem revolves around the fact that while the battery saver is on, the animations in the project must be eliminated, but in the application, animations are still working.

lakshay6907 avatar Jul 06 '24 04:07 lakshay6907