Add a new file details window, unify file activity and sharing

[claucambra]

This rather big PR contains the following changes:

1. The sharing dialog has been completely rewritten in QML.
2. The sharing backend has been almost completely rewritten to be compatible with the new UI components
3. The file activities dialog has been combined with the sharing dialog

These changes have the following advantages:

1. The share dialog now looks far, far nicer :)
2. We properly separate share editing logic from the UI components
3. We use models to present this data, letting us better react to changes
4. All the advantages of QML (now very easy to integrate with the rest of the tray, far better support for touch-screens and gestures, far easier customisation and responsiveness)

Some screengrabs:

https://user-images.githubusercontent.com/70155116/189705620-97c33538-4d9d-43a7-8c8c-a74f3c080e85.mov

https://user-images.githubusercontent.com/70155116/189704899-5bf27fa7-0996-4f30-9dc1-6c693f6955f8.mov

https://user-images.githubusercontent.com/70155116/189704938-e0db4927-d40d-43ac-b6ed-caee85e78712.mov

https://user-images.githubusercontent.com/70155116/189704944-e7075537-4407-4e99-9099-b37243b535ef.mov

https://user-images.githubusercontent.com/70155116/189705209-e1992c4f-9c5b-47eb-ab89-25105426af00.mov

https://user-images.githubusercontent.com/70155116/189705220-eb7e67cf-a85c-4acb-b279-4b8603953ffc.mov

Closes #3845, closes #4777, closes #4947

Signed-off-by: Claudio Cambra [email protected]

[codecov[bot]]

Codecov Report

Merging #4929 (6d85d91) into master (1dbdd88) will increase coverage by 0.44%. The diff coverage is n/a.

:exclamation: Current head 6d85d91 differs from pull request most recent head d79312b. Consider uploading reports for the commit d79312b to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4929      +/-   ##
==========================================
+ Coverage   57.22%   57.66%   +0.44%     
==========================================
  Files         138      138              
  Lines       17441    17418      -23     
==========================================
+ Hits         9980    10044      +64     
+ Misses       7461     7374      -87     

Impacted Files	Coverage Δ
src/libsync/syncengine.h	43.75% <0.00%> (-6.25%)	:arrow_down:
src/libsync/vfs/cfapi/cfapiwrapper.cpp	74.09% <0.00%> (-0.23%)	:arrow_down:
src/libsync/account.h	33.33% <0.00%> (ø)
src/libsync/clientsideencryption.h	38.46% <0.00%> (ø)
src/libsync/syncengine.cpp	84.23% <0.00%> (+0.07%)	:arrow_up:
src/libsync/propagatedownload.cpp	64.75% <0.00%> (+0.14%)	:arrow_up:
src/libsync/discoveryphase.cpp	73.46% <0.00%> (+0.27%)	:arrow_up:
src/libsync/account.cpp	38.00% <0.00%> (+0.40%)	:arrow_up:
src/libsync/networkjobs.cpp	50.70% <0.00%> (+0.42%)	:arrow_up:
src/libsync/clientsideencryption.cpp	26.80% <0.00%> (+0.47%)	:arrow_up:
... and 6 more

[jancborchardt]

@nimishavijay @marcoambrosini would love to get your feedback on this cause I looked at it so much already, would be cool to get new input. :)

[nimishavijay]

Super super great work! Love the animation for switching between activity and sharing :D I have some feedback, but not sure how much of it is feasible:

• [x] Hover feedback for the 3 dot icon (and all icon buttons) can be a circle if possible

• [ ] The colour of the email icon can be --color-primary-light background and --color-primary icon (similar to the share icon in the main dialogue) to distinguish easier between share links and individual emails

• [x] "Link share" --> "Share link" just for consistency with Files :)

• [x] What happens if an invalid date is entered?

• [ ] The background for the password error message is very strong. Maybe we could use 10% opacity of the same colour and a 100% opacity border-left? Similar to a callout

• [x] Is it possible to show the password error message nearer to the password field? Otherwise there could be something like a modal with the message (What do you think? also cc @jancborchardt )

• [ ] Is it possible to shorten the password error message to something like "Your password is common. Use a unique password which is at least 10 characters long"

• [ ] The checkbox style looks different to for eg. in the settings. Any way to make it look more similar to the native style? Like in windows it can look similar to this:

• [x] In the hover feedback for Activity and Sharing tab, the top edge is very close to the icon. Maybe 4-6px more padding? And possibly border-radius of 8px or so if possible?

What do you think?

[marcoambrosini]

So I'm lacking context here and this is a quick impression from the top of my head. I think that what's wrong here is that we're trying again to squeeze very complex flows into a tiny tooltip. This is a clear problem in files already but here even more. This would require a whole share settings "page" or dialog, with nice captions and explanations on what's going on when you toggle things on and off.

[marcoambrosini]

eg: we could have a share settings page for each share similar to google drive. And in files this should prob be a dialog

https://user-images.githubusercontent.com/26852655/190430629-56fe9211-7f03-436e-af83-e69cb5128e57.mov

[claucambra]

eg: we could have a share settings page for each share similar to google drive. And in files this should prob be a dialog

Screen.Recording.2022-09-15.at.16.27.59.mov

Yes, @jancborchardt has already shown mockups of a share settings page rather than a pop-up. However this PR is already massive and I thought it best for the sake of my fellow desktop team members' sanity to leave this for a different pull request :)

[claucambra]

@nimishavijay

• [ ] Hover feedback for the 3 dot icon (and all icon buttons) can be a circle if possible

This is doable

• [ ] The colour of the email icon can be --color-primary-light background and --color-primary icon (similar to the share icon in the main dialogue) to distinguish easier between share links and individual emails

Sorry, do you have an example of what this would look like? We don't use these variables in the desktop client

Are you sure we also want to differ from the way it looks in the server?

• [ ] "Link share" --> "Share link" just for consistency with Files

Sure, no problem

• [ ] What happens if an invalid date is entered?

As soon as you confirm the input this gets set to the server, which replies with a valid date. So if today is 15/09/2022 and you try to set 14/09/2022 the date field will send this data, show a loading indicator, and reset to the 15/09/2022 date which the server forces. If you enforce a maximum 7 day expire days and try to set an expire date further in the future than that, the max expire date gets set

• [ ] The background for the password error message is very strong. Maybe we could use 10% opacity of the same colour and a 100% opacity border-left? Similar to a callout

This is an independent component that is used elsewhere in the client, not just in this PR. We can certainly change the design -- I will open an issue and we can put it on the task list :)

• [ ] Is it possible to show the password error message nearer to the password field? Otherwise there could be something like a modal with the message (What do you think? also cc @jancborchardt )

Yes, this should be possible.

• [ ] Is it possible to shorten the password error message to something like "Your password is common. Use a unique password which is at least 10 characters long"

Not possible. The error messages come from the server and we have no way of knowing, from the desktop client side, why exactly the password has been rejected specifically. We only receive a notice that the password was rejected and a string with the error message from the server

• [ ] The checkbox style looks different to for eg. in the settings. Any way to make it look more similar to the native style? Like in windows it can look similar to this:

Yes, we have discussed this. But like with Marco's feedback this is already a massive PR and it is something that can be worked on separately. Also, more specifically, native style will take a lot of work to do at the moment. We are planning to move to Qt 6 in the near future which should make this much more trivial to implement -- right now we would be forced to design checkboxes specific to each operating system, which is IMO not the way to go

• [ ] In the hover feedback for Activity and Sharing tab, the top edge is very close to the icon. Maybe 4-6px more padding? And possibly border-radius of 8px or so if possible?

Sure, can do

I have added the stuff we can do in this PR to the PR description at the top

[claucambra]

@nimishavijay here is a video showcasing the changes you requested 🙂

https://user-images.githubusercontent.com/70155116/190476094-7f2d7e41-7bc9-421f-b050-78f42456dfbb.mov

[nimishavijay]

It looks pretty great now! Only one more point: move the password error message below the password field, other than that it's good to go design-wise :)

[claucambra]

It looks pretty great now! Only one more point: move the password error message below the password field, other than that it's good to go design-wise 🙂

Okay, I changed it to be below:

Feel free to approve if you are happy with it :)

[jancborchardt]

@marcoambrosini considering your comment, the mockup at https://github.com/nextcloud/server/issues/26691 is still current. :)

What @claucambra is doing currently is keeping feature-parity while moving from QtWidgets to QML. We just need to make sure to separate big moves in tech from big moves in design, so we don’t lose a lot of stuff along the way.

[allexzander]

@claucambra While testing the link sharing I am getting a crash because of a nullptr in src/3rdparty/kirigami/wheelhandler.cpp line 46. The handler is null.

[allexzander]

@claucambra Also, I am getting infinite loading when trying to share a link for one of the files in Notes

[camilasan]

@claucambra Also, I am getting infinite loading when trying to share a link for one of the files in Notes

• I am getting a similar behavior when trying to change the date using up key (which always seems to change to today, but the page down key just moves the focus to the next item in the menu?) - it changes it but then it stays like this even after closing the menu and opening it again:

The same happens with the label:

(On the bright side, the password and note to recipient options seem to be working as expected :))

• The date is not matching the server one (change it in the server, open the share dialog):

• If you type a new date and press enter, it changes the date to the next day again. After closing the dialog and displaying it again, the checkbox 'Set expiration date' is still checked with the date, as if setting it worked. If you look in the server, there is no date set but checkbox is also checked (could this be an issue on the server side?)

[claucambra]

@camilasan @allexzander The share dialog should now be better at recovering from server errors

@camilasan regarding the date change, did you change the date of the share while the share dialog was already open? Or did you change the date on the server and then open the share dialog?

[allexzander]

@claucambra This PR needs conflicts resolved.

[claucambra]

@claucambra This PR needs conflicts resolved.

Conflicts resolved

[allexzander]

@claucambra Is the LockFile test randomly failing, or is it consistent (on Windows)?

[claucambra]

@claucambra Is the LockFile test randomly failing, or is it consistent (on Windows)?

I have seen it fail across several of the latest PRs

[nextcloud-desktop-bot]

AppImage file: nextcloud-PR-4929-d79312b2f7d32af4565221aab71cf694cad917cf-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.

[sonarqubecloud[bot]]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
130 Code Smells

48.1% Coverage
0.0% Duplication

[AndyXheli]

HI @claucambra The auto password generator is not working on client 3.7.4 tested on windows once i click on share via lin i get an option to set password and dose not auto generate