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

[BUG] Confusing loading state in Battery Saver #4400

Open
graciouselectric opened this issue May 7, 2024 · 14 comments
Open

[BUG] Confusing loading state in Battery Saver #4400

graciouselectric opened this issue May 7, 2024 · 14 comments

Comments

@graciouselectric
Copy link

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:

com.owncloud.com.mov
@jesmrec
Copy link
Collaborator

jesmrec commented May 7, 2024

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.

@JuancaG05
Copy link
Collaborator

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

@lakshay6907
Copy link

I would like to contribute to resolving this issue.

@jesmrec
Copy link
Collaborator

jesmrec commented May 20, 2024

go ahead @lakshay6907

@lakshay6907
Copy link

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

@jesmrec
Copy link
Collaborator

jesmrec commented May 24, 2024

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

@JuancaG05 JuancaG05 linked a pull request May 27, 2024 that will close this issue
@JuancaG05 JuancaG05 removed a link to a pull request Jun 5, 2024
@lakshay6907
Copy link

Is this issue still open?

@jesmrec
Copy link
Collaborator

jesmrec commented Jun 24, 2024

Is this issue still open?

Yes, it is.

@JuancaG05
Copy link
Collaborator

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 👍

@lakshay6907
Copy link

lakshay6907 commented Jun 29, 2024

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.

@JuancaG05
Copy link
Collaborator

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?

@lakshay6907
Copy link

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.

@JuancaG05
Copy link
Collaborator

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

@lakshay6907
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants