ipfs-shipyard / ipfs-share-files

Share files via IPFS
https://share.ipfs.io
MIT License
145 stars 29 forks source link

feat: enable dir listing #47

Closed fsdiogo closed 5 years ago

fsdiogo commented 5 years ago

⚠️ PR https://github.com/ipfs-shipyard/ipfs-share-files/pull/44 needs to be merged first!

This is a first step towards the integration of Share Files in Web UI.

Share app is now able to list a mix of files and/or directories:

folders

Things of note:

fsdiogo commented 5 years ago

I'm giving this one a second thought, instead of blocking the download of individual folders, why not use the public gateway to download them too?

Is there any downside?

fsdiogo commented 5 years ago

In my last commit I've added the ability to download individual folders, as per my previous comment.

ss

As @olizilla pointed out in IRC, the go/js HTTP API supports archiving files so we'll use the local daemon when possible. Only when using a fallback js-ipfs instance that we'll use the public gateways (only when we need archiving, i.e, folders or download all) 🎉

olizilla commented 5 years ago

Unless we can figure out a way to make the "download all" multi-click trick work in FF we may want to to tigger a download of the root cid from the url hash, so there is just a single item to download, that contains all the things, regardless of the structure.

fsdiogo commented 5 years ago

To be honest I think that should be the way to go anyway!

The multi-click trick was just because I didn't know the HTTP API supported archiving files (my bad), but as it does it's best to use that don't you think?

I'll make that change in another PR after merging the two pending ones.

EDIT: I'll do this in this PR.

olizilla commented 5 years ago

@fsdiogo go for it!

fsdiogo commented 5 years ago

Current status:

olizilla commented 5 years ago

Minor UX optimisation, If there is only one thing, The download all button should say "download" and should just download the actual thing, rather than a *.tar.gz.

We should think of a human friendly name for the downloaded tar where there is more than one thing... download_QmXF1xadYVFMogoSFgWf7jCZS2qUsbNB5hcbZWE8Tv5pq1.tar.gz is hard on the brain.

Also when expanded the directory is QmXF1xadYVFMogoSFgWf7jCZS2qUsbNB5hcbZWE8Tv5pq1 which, while accurate, isn't helpful once the thing is outside of IPFS. We don't show the root CID on the UI right now, it only appears in the hash frament of the url, so casual users won't spot that QmZF1... dir came from .../#/QmZF1... share url.

Perhaps when we create it should double wrap, so we can have a named directory like share-ipfs-2018-11-22 ? If the user downloads a bunch of thingds on the same day, the OS will add an index or something sensible, and it connects the folder name in your Downloads foler to how it got there.

lidel commented 5 years ago

:+1: for downloading original unwrapped file if we download all from a share that has single item.

:thinking: I have mixed feeling about removing CID from .tar.gz filename and messing with archive contents.

[brain dump commences]

[/brain dump]

olizilla commented 5 years ago

@lidel I here you. My main counter is I don't think the downloaded folder name is the right place to educate the users about CIDs, as it's outside out our realm, and we can't offer any support or guidance. It just looks like IPFS has dumped windows registry keys or UUIDs into my downloads folder.

I think we can iterate on this though and find a decent

When looking through onboarding lens, would archive.tar.gz be more meaningful than download?

Even if we don't want to keep entire CID in filename, we could take last n characters:

For a new user, I think the most useful thing is to associate it with the service that caused it to be there... shared-via-ipfs-<7 chars from the end of the cid>.tar.gz would be ok.

I'll have a think about the CID folder name stuff. We can leave that as is for now and revisit once we've got share.ipfs.io battle ready.

fsdiogo commented 5 years ago

Minor UX optimisation, If there is only one thing, The download all button should say "download" and should just download the actual thing, rather than a *.tar.gz.

👍 for downloading original unwrapped file if we download all from a share that has single item.

I'm not completely sold to that idea. OK with changing Download All > Download if it's a single item, but I like the fact that you always get a tar when clicking that button. If you want the original thing you can just click the individual download button.

fsdiogo commented 5 years ago

About the CIDs, I think @lidel made some pretty good points there for us not "inceptioning" around the wrapping of content.

Even if we don't want to keep entire CID in filename, we could take last n characters: Better than a date in safeguarding us against accidental duplication or ugly (2) (n) suffixes Can be really short (download_Tv5pq1.tar.gz) so we still have space for adding download date if we really want ;)

For a new user, I think the most useful thing is to associate it with the service that caused it to be there... shared-via-ipfs-<7 chars from the end of the cid>.tar.gz would be ok.

I like this. On it.

fsdiogo commented 5 years ago

I think this is ready to go.

I'd like to merge this ASAP so share.ipfs.io gets updated with the last changes. (PR #44 didn't get built because of the CI not passing, that has been fixed in this one.)