Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for wrapping quick uploads in a directory. #349

Closed
falsechicken opened this issue Dec 30, 2017 · 3 comments
Closed

Add support for wrapping quick uploads in a directory. #349

falsechicken opened this issue Dec 30, 2017 · 3 comments
Assignees
Labels
exp/intermediate Prior experience is likely helpful kind/enhancement A net-new feature or improvement to an existing feature status/in-progress In progress
Milestone

Comments

@falsechicken
Copy link

Maybe in the form of a checkbox or something in the quick upload UI. Would be nice for my uses because if I share a file to another user using a public gateway the filename is not preserved when they try and save the file. This is a bigger problem on Windows because it relies so much on file extensions.

@lidel lidel added exp/intermediate Prior experience is likely helpful kind/enhancement A net-new feature or improvement to an existing feature good first issue Good issue for new contributors help wanted Seeking public contribution on this issue UX labels Dec 30, 2017
@lidel lidel added this to the 2018-Q1 milestone Dec 30, 2017
@nunofmn
Copy link
Member

nunofmn commented Jan 18, 2018

Ok, so I want to give a go at this one 👍

So I will try to summarize the current status:

  • The Files.add function doesn't implement the wrap-with-directoryoption.
  • In go-ipfs this functionality is implemented in the HTTP API and CLI.
  • In js-ipfs this functionality is only implemented in CLI. (missing HTTP API and Core).

So the following steps are needed:

I have a working implementation of this option in js-ipfs HTTP API, and I'm working on adding it to js-ipfs Core also.

@lidel from an UX perspective do you think the checkbox field that @falsechicken mentioned is a good option, or could this functionality be activated by default?

@lidel lidel self-assigned this Jan 18, 2018
@lidel
Copy link
Member

lidel commented Jan 18, 2018

Plan A

@nunofmn sounds good to me 👍 the summary and list of steps are very helpful, thanks!
If you are able to add it to the API, that would be beneficial to other projects using js-ipfs-api.
The task is yours – go for it! 🚀

Plan B

Just for the record, here is a backup plan, if extending API turns out to be not feasible:

upload file the old way + assemble wrapping directory manually via object API:

  • file add<file_cid>
  • object new unixfs-dir<dir_cid>
  • object path <dir_cid> add-link <filename> <file_cid><dir_with_file_cid>
  • pin <dir_with_file_cid>

Checkbox for wrapping file with directory

I think we should have an explicit checkbox on the bottom of the upload form.
And it should be checked by default because most of users will want to preserve file name.

Reasons for preserving file name by default:

  • better UX for receiver: file type can be infered from file name or extension, looks less scary
  • various systems will not load remote content unless URL ends with specific extension
  • solves false-positives of mime-type detection when opened in older browsers (eg. SVG being rendered as XML tree etc)

@lidel lidel removed good first issue Good issue for new contributors help wanted Seeking public contribution on this issue labels Jan 18, 2018
@lidel lidel removed their assignment Mar 22, 2018
@lidel lidel self-assigned this May 14, 2018
@lidel lidel added the status/in-progress In progress label May 14, 2018
lidel added a commit that referenced this issue May 16, 2018
Implements #349 (for now only for external node)
lidel added a commit that referenced this issue May 16, 2018
Implements #349 (for now only for external node)
@lidel
Copy link
Member

lidel commented May 19, 2018

FYI this feature just landed in beta channel: v2.2.2.9320. Implementation details in PR #479.
Big thank you to everyone who contributed necessary changes to upstream libs and specs.

Embedded node does not support it yet, but that will be tracked in #482.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/intermediate Prior experience is likely helpful kind/enhancement A net-new feature or improvement to an existing feature status/in-progress In progress
Projects
None yet
Development

No branches or pull requests

3 participants