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

Fix Image Loading When Offline #14502

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

alexknop
Copy link
Collaborator

@alexknop alexknop commented Feb 5, 2025

Problem:

Although Videos were loading immediately when offline, Images were not

Solution:
Found that checking for whether file is offline was passing, but the ! in the code was making it false, forcing a sync which would timeout (taking extra time) and preventing immediate access to images

Fixes #14470

@alexknop
Copy link
Collaborator Author

alexknop commented Feb 6, 2025

It still takes a long time to load images when on cellular.

My server address is 192.168.1.2 and as I test this on a guest Wi-fi network with 192.168.1.0/24 as well, it seems Nextcloud is quicker to detect my server unreachable on here versus on Cellular. However, load times are much better when on the guest Wi-Fi and loading images due to this code change.

Videos always load immediately regardless of WiFi or Cellular

@alexknop
Copy link
Collaborator Author

alexknop commented Feb 6, 2025

I dug more into it and found the timeouts for ReadFileRemoteOperation were extensive by default but you can customize the timeouts. By lowering them, the connection error happens sooner and the app responds quicker.

Images are now loading fine when on cellular and I cannot reach my server.
Please consider these changes and lets discuss.

@alexknop
Copy link
Collaborator Author

alexknop commented Feb 6, 2025

Found another area to reduce timeouts and speed up the app, that is the timeouts for OCVersionCheck.

Changed them to 1000 and app is responding better when offline.

@alperozturk96
Copy link
Collaborator

alperozturk96 commented Feb 7, 2025

Thank you for the PR.

RefreshFolderOperation/ReadFileRemoteOperation may need more time. If we lower the default session time out value, the user may not get the latest result.

Default = SessionTimeOut(60000, 15000)
PR = SessionTimeOut(1000, 1000)

@tobiasKaminsky Most likely it will fail, and 1 second is very low. What do you think?

@alexknop alexknop force-pushed the ImageTimeout branch 3 times, most recently from 89643a1 to 56aaebb Compare February 11, 2025 14:57
Copy link

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

@tobiasKaminsky
Copy link
Member

@alexknop As I see that you are actively contribution, I'd like to invite you to our org, so that you can directly create PR within nextcloud repository.
This makes contributing and reviewing easier (as then CI directly runs).

@alexknop
Copy link
Collaborator Author

@alexknop As I see that you are actively contribution, I'd like to invite you to our org, so that you can directly create PR within nextcloud repository. This makes contributing and reviewing easier (as then CI directly runs).

@tobiasKaminsky I was actually invited into the org yesterday. I will be doing my other PRs directly from here on out.
What do you think about my proposed changes?

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

Successfully merging this pull request may close these issues.

Cannot Open Offline Photos After Video
4 participants