Codecov Report

Merging #4771 (a878eaf) into master (4f85f7a) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head a878eaf differs from pull request most recent head d42d3c0. Consider uploading reports for the commit d42d3c0 to get more accurate results

@@            Coverage Diff             @@
##           master    #4771      +/-   ##
==========================================
- Coverage   56.80%   56.79%   -0.01%     
==========================================
  Files         138      138              
  Lines       17142    17143       +1     
==========================================
- Hits         9737     9736       -1     
- Misses       7405     7407       +2     

@jancborchardt @nimishavijay

I have added 3 videos (2 for cases of error handling and one for successful opening of a local file).

Could you provide your feedback from the design perspective?

Note: Errors are displayed in both popup and a tray window, however, the tray window can only display an error if there is an account and a folder present, as the error gets bound to a specific local sync folder and it can't be displayed otherwise.

Super nice work! :)

scenario of an incorrect account in a URL or no account added to the Nextcloud desktop app

This looks good! Would it be possible to have 2 buttons: "Connect account" and "Cancel" instead of an "OK" button? Clicking "Connect account" can open the setup window and close the error dialogue. This would eliminate the step of navigating to the tray menu to set up your account.

scenario of a wrong (non-existing or not yet synced) file in a URL

Is it possible to force sync the file automatically like it is mentioned in the linked issue? Otherwise we could also have a "Sync now" button instead of "OK" which force syncs.

successful handling of a file that is present and is synced locally for the account in the URL and the file gets opened

This is also super cool! One question: Can the file be opened in the foreground instead of the background?

cc @jancborchardt

Is it possible to force sync the file automatically like it is mentioned in the linked issue? Otherwise, we could also have a "Sync now" button instead of "OK" which force syncs.

We've been discussing this part with @tobiasKaminsky. The issue also mentions that we can not sync something that's excluded via selective sync settings. The problem here is, that we don't have the functionality to sync just one file in the desktop client. So, let's say we want to sync a file document.docx that resides in the subfolder RemoteRoot/B/b1/ but we have this folder RemoteRoot/B excluded. If we would want to sync that file, that means we will have to sync every other file and folder within the RemoteRoot/B. This might require quite some time to sync everything in case there are many files and the size is significant. So, I don't really have a solution here. @mgallien maybe there is a solution, but I am just not thinking out of the box?

This is also super cool! One question: Can the file be opened in the foreground instead of the background?

I also have noticed that we almost always open a popup that is not topmost and is blinking in the taskbar. I don't see any way to make it happen differently, as it is up to the OS to decide how it gets opened. [QDesktopServices](https://doc.qt.io/qt-6/qdesktopservices.html)::openUrl( does not have other parameters but QUrl. I'd expect to be able to control the topmost behavior based on parameters, but they are not supported. Is there any other way to trigger the OS to open a file via the default app? @mgallien

Is it possible to force sync the file automatically like it is mentioned in the linked issue? Otherwise, we could also have a "Sync now" button instead of "OK" which force syncs.

We've been discussing this part with @tobiasKaminsky. The issue also mentions that we can not sync something that's excluded via selective sync settings. The problem here is, that we don't have the functionality to sync just one file in the desktop client. So, let's say we want to sync a file document.docx that resides in the subfolder RemoteRoot/B/b1/ but we have this folder RemoteRoot/B excluded. If we would want to sync that file, that means we will have to sync every other file and folder within the RemoteRoot/B. This might require quite some time to sync everything in case there are many files and the size is significant. So, I don't really have a solution here. @mgallien maybe there is a solution, but I am just not thinking out of the box?

This is okay for now.
If we see that many users use this and run into this problem, then we might need to come up with a file-based selective sync, or have it then only synced in a temp folder and afterwards deleted.
But right now, let's wait for server and see how user adopt it.

This is also super cool! One question: Can the file be opened in the foreground instead of the background?

I also have noticed that we almost always open a popup that is not topmost and is blinking in the taskbar. I don't see any way to make it happen differently, as it is up to the OS to decide how it gets opened. [QDesktopServices](https://doc.qt.io/qt-6/qdesktopservices.html)::openUrl( does not have other parameters but QUrl. I'd expect to be able to control the topmost behavior based on parameters, but they are not supported. Is there any other way to trigger the OS to open a file via the default app? @mgallien

MS Teams and other solutions show then a new page which explains that file/call/meeting is handled via Desktop app.
Also they then state that if this does not work, one can install the Desktop app.
This is something we should also do, but on server side.

The problem here is, that we don't have the functionality to sync just one file in the desktop client.

Ah okay, understood. There would be an issue if the "Make sure it is synced" message keeps showing up even after the user has tried to force sync multiple times. In this case of excluded folder would it be possible to warn in the dialogue that the file they are trying to edit is not available locally? "Cannot open file locally as it is excluded from sync" or something like that (definitely need the help of @jancborchardt for the wording 🤔)
Or better yet, show the "Edit locally" action in the action menu in the browser only if the file is in a synced folder, is that possible?

MS Teams and other solutions show then a new page which explains that file/call/meeting is handled via Desktop app.

Great idea!

Ah okay, understood. There would be an issue if the "Make sure it is synced" message keeps showing up even after the user has tried to force sync multiple times. In this case of excluded folder would it be possible to warn in the dialogue that the file they are trying to edit is not available locally? "Cannot open file locally as it is excluded from sync" or something like that (definitely need the help of @jancborchardt for the wording 🤔)

We can surely refine the message in the popup and right now, the dialog is indeed shown when the file is not present locally because it is not synced yet or it is excluded. I am just unsure if we can detect that the file is just excluded or if it is going to be synced soon. When the syncing happens, we visit subfolders recursively. For example, if the file is in the nested subfolder RemoteRoot/B/b1/b2/b_whatever/ then, we will only discover that file after we first go to RemoteRoot/B, then to b1, and so on.
Long story short, I can extend the message to propose the user either wait till the file is synced or check if the file is in the excluded folder and make sure it is included. I can also display a full path to a file so the user will see which folder needs to be synced if that helps?

Nevertheless, I'll see if I can match the file's path in a URL with the list of excluded folders, and try to display an exact message if the file's folder is present in the list of excluded folders.

Or better yet, show the "Edit locally" action in the action menu in the browser only if the file is in a synced folder, is that possible?

This functionality is not supported by server-side AFAIK. We don't really exchange messages with the server. We either authenticate, sync files, fetch capabilities, receive notifications, and reply to chat mentions. But, we don't have full bi-directional communication such that the desktop client would let the server know which files are synced.

I can extend the message to propose the user either wait till the file is synced or check if the file is in the excluded folder and make sure it is included. I can also display a full path to a file so the user will see which folder needs to be synced if that helps?

Yep, this sounds perfect! The message can say something like "Cannot open file locally. Try syncing again and make sure the file is not in an excluded folder. [path to file] [Sync now button] [Show excluded folders button] [Cancel button]" @jancborchardt

Yep, this sounds perfect! The message can say something like "Cannot open file locally. Try syncing again and make sure the file is not in an excluded folder. [path to file] [Sync now button] [Show excluded folders button] [Cancel button]"

So, I was able to find a way to distinguish between non-existing file and the file that might have been excluded via selective sync. I have replaced the wrong file error handling video with 2 videos for these separate use cases.

Looks good, I will add the macOS integration

Works with latest commit :)

@alex please clean history before any merge
I again pressed the approve button too early

AppImage file: Nextcloud-PR-4771-d42d3c057f0daacea92aed9fa97263bdc8b38841-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
6 Code Smells

0.0% Coverage
0.0% Duplication