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

Create a file upload component #2

Open
JonatanSalas opened this issue Jul 22, 2016 · 17 comments
Open

Create a file upload component #2

JonatanSalas opened this issue Jul 22, 2016 · 17 comments
Assignees

Comments

@JonatanSalas
Copy link
Member

JonatanSalas commented Jul 22, 2016

Add support for drag and drop file upload component

@andrerpena
Copy link
Member

I'll come back to this as soon as this is complete: redux-autoform/redux-autoform#45

Having the features we have now working, tested and DOCUMENTED is far more important than adding new features.

@JonatanSalas
Copy link
Member Author

100% agree with you @andrerpena! I will try to add test for components with enzyme.

@andrerpena
Copy link
Member

+1, really.. Enzyme seems really great

@JonatanSalas
Copy link
Member Author

@andrerpena the guys from airbnb are really good with js!

JonatanSalas pushed a commit that referenced this issue Aug 9, 2016
@JonatanSalas JonatanSalas self-assigned this Aug 9, 2016
@JonatanSalas JonatanSalas removed this from the v1.0.2 milestone Aug 9, 2016
JonatanSalas pushed a commit that referenced this issue Aug 10, 2016
JonatanSalas pushed a commit that referenced this issue Aug 10, 2016
JonatanSalas pushed a commit that referenced this issue Aug 10, 2016
JonatanSalas pushed a commit that referenced this issue Aug 10, 2016
JonatanSalas pushed a commit that referenced this issue Aug 10, 2016
JonatanSalas pushed a commit that referenced this issue Aug 10, 2016
danigomez pushed a commit that referenced this issue Aug 10, 2016
JonatanSalas pushed a commit that referenced this issue Aug 10, 2016
danigomez pushed a commit that referenced this issue Aug 10, 2016
@danigomez
Copy link
Contributor

danigomez commented Aug 10, 2016

This comment is to remember how to configure a FileUpload!! 😄

In order to obtain the file data sent by a multipart/form-data content type, we should use the multer middleware (https://github.com/expressjs/multer).
This allows you to define a diskStorage where you set where the files will be uploaded and which pattern will be used to select the filename of a uploaded file.

import multer from 'multer';

const storage = multer.diskStorage({
    destination: function(request, file, callback) {
        callback(null, "./demo/uploads");
    },
    filename: function(request, file, callback) {
        callback(null, file.originalname + '-' + Date.now())
    }
});

After that you create an instance of multer with the configured storage:

const upload = multer({storage});

And finally you should use that multer instance in each route that will receive the multipart data:

router.post("/upload", upload.array("fileData"), (request, response) => {
/* Some awesome stuff */
}

IMPORTANT The data you append to the FormData class in a FileUpload should have the same name you use when you use the upload.array/single/files functions, in our case, we are sendind the fileUpload data as fileData

JonatanSalas pushed a commit that referenced this issue Aug 10, 2016
danigomez pushed a commit that referenced this issue Aug 10, 2016
JonatanSalas pushed a commit that referenced this issue Aug 10, 2016
@JonatanSalas
Copy link
Member Author

JonatanSalas commented Aug 11, 2016

Things TODO:

  • Support inline and stacked mode
  • Avoid duplicated files
  • Modal to confirm delete on file
  • Provide a better way to customize styles
  • Extract styles from JS or use somethings like CSS in JS option
  • Consume status from service and show an alert or success message when uploading files
  • Auto upload behaviour (This will delete the upload button that we use now, this could be another component too)
  • Sync file related fields with redux-form store

Actually the FileUpload looks like this:

captura de pantalla de 2016-08-11 13-29-20

@JonatanSalas JonatanSalas mentioned this issue Aug 11, 2016
6 tasks
danigomez pushed a commit that referenced this issue Aug 11, 2016
JonatanSalas pushed a commit that referenced this issue Aug 11, 2016
danigomez pushed a commit that referenced this issue Aug 11, 2016
danigomez pushed a commit that referenced this issue Aug 11, 2016
@JonatanSalas
Copy link
Member Author

JonatanSalas commented Aug 11, 2016

Added modal to confirm delete!

captura de pantalla de 2016-08-11 15-34-32

I have merged this now because we need to use @andrerpena, I will work on fix the TabGroup minor bugs and test all the components, because I have make the coverage decrease 😞 but I promise the coverage will increase!! 😄

@JonatanSalas
Copy link
Member Author

I will keep this issue open since there are so much things to do to get it as we want

@andrerpena
Copy link
Member

@JonatanSalas and @danigomez . I've been away for a while but I'm back now and trying to get up to speed again.

First, you've been doing a fantastic job with this component. I got a few questions:

  1. I can see the component is basically composed of the files FileUpload.js and DropZone.js. If you've created this component and not leveraged an existing one, I assume that you didn't find one. If you didn't find one, it might be a good oportunity to extract this component out of RAF-BS. In my vision, RAF-BS shouldn't contain it's own components (left alone complex ones like this) but be a "package" of existing ones.
  2. I've seen multer is being used for handling the multi-part upload. How are we dealing with the published demo version that doesn't have Node/Express support?

Another note, I added support for stacked vs inline in @JonatanSalas list.

Again.. Amazing job.

@JonatanSalas
Copy link
Member Author

@andrerpena here you have the explanations:

  1. The component I have used was react-dropzone. But I was having a bug with it, so I extracted the code.. make some modifications and used by myself, that's why this component is in bootstrap ui.
  2. I want to make a small backend in node and deploy it through openshift. So we can have it uploaded to internet and the demo will consume from there.

What's your opinion?

@andrerpena
Copy link
Member

Thanks @JonatanSalas

  1. I understand you just used the source inside RAF because you had to deliver it. Totally comprehensible 😄 . But now, do you think we can extract it away from it? And create a file upload component that works? This would be interesting because the community could use it even outside RAF. This should drawn contributors too.
  2. If you want to create a full-blown server to deploy the RAF's demo, it's ok for me as long as it's free. But don't you think it's a little overkill? We could just deploy the demo prod in such a way that the "upload" button doesn't work.

@JonatanSalas
Copy link
Member Author

  1. Do you mean create a FileUpload project? 😮
  2. I refer to create a simple api that expose the simple resources that the demo needs (For example to fill the select/lookup from api, and FileUpload with it's upload service).
    The demo will be deployed through github-pages also.

@danigomez
Copy link
Contributor

I like the idea for a FileUpload project!!! That way we can reuse it in the other UIs!! 😄

PS: 🎉 🍰 🍰 🎉 😄

@JonatanSalas
Copy link
Member Author

@danigomez the only case this will be different it's with material.. so we have to think on how to style it or make something that mix well with any UI

@andrerpena
Copy link
Member

  1. Yes. If the one you found doesn't work we should have our own. This is just my opinion, but RAF shouldn't have components. Also, as @danigomez said, it would be reusable.
  2. I don't find it a priority but feel absolutely free to create it if you want 😄 . In my opinion this is what we should do: If the demo can behave differently online and locally according to a build configuration, then do it. Otherwise, we should think carefully. Example: For the autocomplete, we can load the options from a Promise (works online) or through a URL. This case, we can use a public URL, or don't have this option on the demo at all. Just in the documentation. But again, if you want to spin up a server, go for it. Open source is about doing what you are motivated to do ❤️

@andrerpena
Copy link
Member

@JonatanSalas I agree that styling is a problem. Maybe we could have a base headless component and than one component for each UI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants