feat: use direct links to download all files #1894

hacdias · 2022-01-16T20:20:39Z

This closes #1887 by using the gateway URLs to download the files. There's two things to mention:

I tried creating an anchor element to download the file with a name via the download attribute, but it seems to still use the name get.tar.gz sadly. Any suggestions?
There's some other code that used to show download progress. Should I remove it? I don't think it is used anywhere else in the code so it feels safe to remove unless it can be used for any planned feature.

License: MIT
Signed-off-by: Henrique Dias hacdias@gmail.com

lidel

Re 1 – see inline comment
Re 2 – ok to remove the dowload-file.js, do you have something more in mind?

src/files/FilesPage.js

hacdias · 2022-11-10T11:13:23Z

The TARs are a thing on the gateway now: ipfs/kubo#9029

This PR can be unblocked and worked on 😃

License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>

hacdias · 2022-11-10T13:40:58Z

I decided to revitalise this PR since we now have TAR response formats on our gateway (ipfs/kubo#9029). That lets us download directories directly from the gateway without the need to use the API v0 POST method. Therefore, we do not need to keep anything in memory and it is all streamed to the user. This allows to massively simplify the code, removing a lot of old bits that are no longer relevant 🪩

This can be tested with Kubo from master, and with 0.17.0 RC-1. I'll mark this as ready for review as its been open for many months, but it's finally possible to review and test this PR.

lidel

lgtm, small UX question below

src/lib/files.js

SgtPooki

The changes look okay, but we're removing some functionality: progress updates and download aborting. Is that intended, or am I missing something?

Note: I do agree that this is a significant improvement, and resolves #1887 but I'm wondering if there is a way we can keep the ability to cancel a download, or display download progress. Fine with a separate issue for those items

SgtPooki · 2022-11-10T16:32:53Z

src/files/FilesPage.js

+    const url = await doFilesDownloadLink(files)
+    const link = document.createElement('a')
+    link.href = url
+    link.click()


non-blocking Q: instead of creating an element and then programmatically clicking on it can we have the thing the users click have the appropriate URL?

Note: I do agree that this is a significant improvement, and resolves #1887 but I'm wondering if there is a way we can keep the ability to cancel a download, or display download progress. Fine with a separate issue for those items

Downloads will be handled by the browser UI. The user can just cancel it through the browser UI. We only had the option to cancel and display progress on the Web UI itself because we were storing every single byte of data in-memory before downloading.

non-blocking Q: instead of creating an element and then programmatically clicking on it can we have the thing the users click have the appropriate URL?

Yes and no. If you select multiple files, what link do you put? You can't as we calculate the CID dynamically. Then we'd need different behaviour according to the number of files selected and that's a bit of a hassle. Let's keep it like this to be uniform with the CAR downloading behaviour.

Can't we just redirect the users to download? it will prompt the user to download and not go away from current page too?

@whizzzkid I changed to window.location.href = url. However, it has the same behaviour in my browser as clicking a link. Because clicking a link just leads to redirection. I'm interested in knowing what is happening in your browser, because I can't understand. I'll post a video here in a bit (see #1894 (comment)).

whizzzkid

non-blocking nits

src/bundles/files/actions.js

whizzzkid · 2022-11-10T18:15:31Z

src/files/FilesPage.js

+    const url = await doFilesDownloadLink(files)
+    const link = document.createElement('a')
+    link.href = url
+    link.click()


Can't we just redirect the users to download? it will prompt the user to download and not go away from current page too?

whizzzkid · 2022-11-10T18:16:02Z

src/files/selected-actions/SelectedActions.js

  render () {
-    const { t, tReady, animateOnStart, count, size, unselect, remove, share, setPinning, download, downloadProgress, rename, inspect, className, style, isMfs, ...props } = this.props
+    const { t, tReady, animateOnStart, count, size, unselect, remove, share, setPinning, download, rename, inspect, className, style, isMfs, ...props } = this.props


Can we sort these alphabetically?

Unless you know how to do it automatically, please don't make me do it 😅

eslint can do it automatically, we should not be forcing devs to do code style management

hacdias · 2022-11-11T10:13:30Z

@SgtPooki @whizzzkid I addressed most of your comments (see replies). This is the behaviour of the current PR:

cast.mp4

SgtPooki

Thanks a ton for remembering this PR and coming back to close it out @hacdias !

SgtPooki · 2022-11-11T19:13:21Z

Do we expect any issues with how this will work within electron?

hacdias · 2022-11-14T08:48:29Z

Do we expect any issues with how this will work within electron?

I think it should be fine. We already had download links we this behaviour before and they were not a problem.

## [2.21.0](v2.20.0...v2.21.0) (2022-12-09) CID `bafybeiequgo72mrvuml56j4gk7crewig5bavumrrzhkqbim6b3s2yqi7ty` --- ### Features * use direct links to download all files ([#1894](#1894)) ([d1bcbbf](d1bcbbf)) ### Bug Fixes * support /quic-v1 ([#2073](#2073)) ([04eb7b3](04eb7b3)) ### Trivial Changes * bump playwright deps ([#2066](#2066)) ([f138960](f138960)) * **ci:** fix flaky unit test ([#2068](#2068)) ([bd038cd](bd038cd)), closes [/github.com//issues/2065#issuecomment-1315933342](https://github.com/ipfs//github.com/ipfs/ipfs-webui/issues/2065/issues/issuecomment-1315933342) * Pull transifex translations ([#2069](#2069)) ([36f3641](36f3641)) * revert [#2032](#2032) ([#2064](#2064)) ([9473d7d](9473d7d)), closes [/github.com//pull/2032#issuecomment-1278928440](https://github.com/ipfs//github.com/ipfs/ipfs-webui/pull/2032/issues/issuecomment-1278928440)

ipfs-gui-bot · 2022-12-09T23:17:49Z

🎉 This PR is included in version 2.21.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

hacdias requested a review from lidel January 16, 2022 20:20

hacdias temporarily deployed to Deploy January 16, 2022 20:25 Inactive

lidel requested changes Jan 18, 2022

View reviewed changes

src/files/FilesPage.js Outdated Show resolved Hide resolved

lidel added the status/blocked Unable to be worked further until needs are met label Jan 18, 2022

hacdias force-pushed the fix/1887 branch from d802388 to e3fa393 Compare June 2, 2022 09:10

hacdias temporarily deployed to Deploy June 2, 2022 09:15 Inactive

This was referenced Jun 9, 2022

feat: download as CAR via the context menu #1837

Merged

fix: use gateway links to download standalone files #1945

Merged

lidel force-pushed the main branch 4 times, most recently from dabaee3 to 7ddf870 Compare June 29, 2022 22:19

MarcelRaschke approved these changes Oct 28, 2022

View reviewed changes

hacdias removed the status/blocked Unable to be worked further until needs are met label Nov 10, 2022

fix: use direct link to gateway to download files

480b955

License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>

hacdias force-pushed the fix/1887 branch from e3fa393 to 480b955 Compare November 10, 2022 13:00

hacdias temporarily deployed to Deploy November 10, 2022 13:04 Inactive

hacdias added 2 commits November 10, 2022 14:26

feat: massively simplify file downloading

b8ec0cf

fix: download filename

d24c948

hacdias requested a review from lidel November 10, 2022 13:28

hacdias changed the title ~~fix: use direct link to gateway to download files~~ feat: use direct links to download all files Nov 10, 2022

refactor: remove download progress things

6bbb524

hacdias temporarily deployed to Deploy November 10, 2022 13:36 Inactive

hacdias self-assigned this Nov 10, 2022

hacdias marked this pull request as ready for review November 10, 2022 13:41

hacdias requested a review from a team as a code owner November 10, 2022 13:41

hacdias requested review from SgtPooki and whizzzkid November 10, 2022 13:43

lidel approved these changes Nov 10, 2022

View reviewed changes

src/lib/files.js Show resolved Hide resolved

SgtPooki reviewed Nov 10, 2022

View reviewed changes

whizzzkid reviewed Nov 10, 2022

View reviewed changes

refactor: remove return await

1f72e4b

hacdias temporarily deployed to Deploy November 11, 2022 10:07 Inactive

refactor: use window.location.href instead

194ac18

hacdias had a problem deploying to Deploy November 11, 2022 10:11 Error

hacdias requested review from whizzzkid and SgtPooki November 11, 2022 10:14

SgtPooki approved these changes Nov 11, 2022

View reviewed changes

Merge branch 'main' into fix/1887

9bec6b6

SgtPooki had a problem deploying to Deploy November 17, 2022 23:18 Error

SgtPooki merged commit d1bcbbf into main Nov 22, 2022

SgtPooki deleted the fix/1887 branch November 22, 2022 18:35

ipfs-gui-bot added the released label Dec 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use direct links to download all files #1894

feat: use direct links to download all files #1894

hacdias commented Jan 16, 2022 •

edited

Loading

lidel left a comment

hacdias commented Nov 10, 2022

hacdias commented Nov 10, 2022

lidel left a comment

SgtPooki left a comment •

edited

Loading

SgtPooki Nov 10, 2022

hacdias Nov 10, 2022

hacdias Nov 10, 2022 •

edited

Loading

whizzzkid Nov 10, 2022

hacdias Nov 11, 2022 •

edited

Loading

whizzzkid left a comment

whizzzkid Nov 10, 2022

whizzzkid Nov 10, 2022

hacdias Nov 11, 2022

SgtPooki Nov 11, 2022

hacdias commented Nov 11, 2022

SgtPooki left a comment

SgtPooki commented Nov 11, 2022

hacdias commented Nov 14, 2022

ipfs-gui-bot commented Dec 9, 2022

feat: use direct links to download all files #1894

feat: use direct links to download all files #1894

Conversation

hacdias commented Jan 16, 2022 • edited Loading

lidel left a comment

Choose a reason for hiding this comment

hacdias commented Nov 10, 2022

hacdias commented Nov 10, 2022

lidel left a comment

Choose a reason for hiding this comment

SgtPooki left a comment • edited Loading

Choose a reason for hiding this comment

SgtPooki Nov 10, 2022

Choose a reason for hiding this comment

hacdias Nov 10, 2022

Choose a reason for hiding this comment

hacdias Nov 10, 2022 • edited Loading

Choose a reason for hiding this comment

whizzzkid Nov 10, 2022

Choose a reason for hiding this comment

hacdias Nov 11, 2022 • edited Loading

Choose a reason for hiding this comment

whizzzkid left a comment

Choose a reason for hiding this comment

whizzzkid Nov 10, 2022

Choose a reason for hiding this comment

whizzzkid Nov 10, 2022

Choose a reason for hiding this comment

hacdias Nov 11, 2022

Choose a reason for hiding this comment

SgtPooki Nov 11, 2022

Choose a reason for hiding this comment

hacdias commented Nov 11, 2022

SgtPooki left a comment

Choose a reason for hiding this comment

SgtPooki commented Nov 11, 2022

hacdias commented Nov 14, 2022

ipfs-gui-bot commented Dec 9, 2022

hacdias commented Jan 16, 2022 •

edited

Loading

SgtPooki left a comment •

edited

Loading

hacdias Nov 10, 2022 •

edited

Loading

hacdias Nov 11, 2022 •

edited

Loading