Add download button

[beardhatcode]

Continuation of #702.

I addressed @skjnldsv comments and moved the download button to above the delete button (it felt better to me like that).

Works on my server and in the docker image.

Mentioning relevant people:
@fflorent @skjnldsv

[beardhatcode]

I do not immediately see why Cypress is failing. Does anyone have some pointers for me?

[szaimen]

Thanks for continuing this!
Maybe there could be a ActionSeparator between download and delete like e.g. shown here?
nextcloud/spreed#5285 (comment)

[beardhatcode]

The material design guidelines specify that separator should be used to "to create groupings rather than separate items" . I personally don't feel like "open sidebar" and "download" would form a group as they are very different actions. What do you think @szaimen ?

[szaimen]

That's a good guideline 👍 I just wanted to bring this up as an enhancement point but the better way (instead of a separator) would probably be to ask if you want to delete a file and not instantly deleting it. So I guess it is up to the @nextcloud/designers team to decide if a separator makes sense here...

[beardhatcode]

Oh, It should not instantly delete the file in the viewer without confirmation. That would be very bad, is there an issue open for that already?

edit: I read some more about this and there is a an open issue for this: #746

[marcoambrosini]

Hey, nice addition! What about having the download button always visible instead of within the actions menu? I think it would be very handy.

[skjnldsv]

@ma12-co not available in the current Modal component, but that would be nice indeed :)

[szaimen]

@ma12-co not available in the current Modal component, but that would be nice indeed :)

So then how to move this PR forward?
Just add it in the drop-down menu for now and change it later back into the Modal when it is available?

[skjnldsv]

See comments :)
Sorry it took me time, very nice work! 🚀

[skjnldsv]

/backport to stable21

[skjnldsv]

/backport to stable20

[skjnldsv]

/backport to stable19

[beardhatcode]

Got it working locally. I needed to add a download attribute the link to make cypress happy (but it is also just better like this).

  Running:  download.spec.js

  Download image.png in viewer
    ✓ See "image.png" in the list (8274ms)
    ✓ Open the viewer on file click (583ms)
    ✓ Does not see a loading animation
    ✓ Download the image (186ms)
    ✓ Compare downloaded file with asset by size (305ms)

(also works in the check run: https://github.com/nextcloud/viewer/pull/814/checks?check_run_id=2207888337#step:8:456 )

[beardhatcode]

@skjnldsv, some cypress tests are failing, but I don't know why. Could you take a look at it?

[skjnldsv]

Relevant bug
nextcloud-libraries/nextcloud-vue#1809

[skjnldsv]

Pfiouu, lots of fixes!

• There is a bug in our button library, so will be fixed later.
• I added a test, because we were not compliant to the hideDownload option on link shares
• Fixed Eslint
• And instead of hardcoding the file size, I directly read it from the fixtures 😉

Again, big thank you for all the work @beardhatcode and @fflorent. Couldn't have found the time if you had not done 99% of the work! 🎉

[skjnldsv]

Everything passes locally 🤔

[skjnldsv]

All good on master https://github.com/nextcloud/viewer/actions/runs/732228778

[skjnldsv]

I will stop at 21 unfortunately, 20 might be too different, let's see on #849