Skip to content
This repository was archived by the owner on Sep 20, 2023. It is now read-only.

Fancy image uploads #44

Closed
wanderingstan opened this issue Jan 7, 2018 · 10 comments
Closed

Fancy image uploads #44

wanderingstan opened this issue Jan 7, 2018 · 10 comments
Assignees
Labels

Comments

@wanderingstan
Copy link
Contributor

Aure's design calls for images to be uploaded via a slick UI with thumbnails:

image

We're using react-json-schema-form, which does only old-school style file uploads. (Click a button to open native OS file selector.)

So there are multiple ways to go:

  1. Break image uploading into its own step in listing-create.js. (One idea would be to use the same react-json-schema-form, but hide the image part when entering details, and hide everything BUT the image step when uploading photos.)
  2. Perhaps use a natve FileReader() to do the upload, and inject the results into the react-json-schema-form. (Docs)
  3. Use HTML5 image uploader to show preview of uploaded images, and possibly even resize them.
@nikkwong
Copy link

nikkwong commented Jan 26, 2018

@wanderingstan I'll take a stab at this.

@nikkwong
Copy link

@wanderingstan Are Aure's designs publicly available, so I can get a better idea of how these flows work together?

@wanderingstan
Copy link
Contributor Author

@nikkwong doesn't look like they are, and I can't publish them. FWIW, the link is here:
https://app.zeplin.io/project/59fa2311bac7acbc8d953da9?seid=5a361ba792b402caa8e2de28

BUT, I can just upload the image:
demo app - create listing 3

The idea is that this would be a step in between entering the details and just before preview.

For how it would work, I think we can just propegate the jsonschema-form to the next component, and hide everything. (Or store the form data in state, and add images to it there.) Not totally sure.

@wanderingstan
Copy link
Contributor Author

@nikkwong Oh crap, just realized that image was already in this issue. I'll get Aure's entire flow to post here.

@nikkwong
Copy link

nikkwong commented Jan 27, 2018

@wanderingstan got it. Yeah, it starts getting a wee-bit hacky because you don't want to remove the pictures field from the json schemas as it breaks the contract between the json and the jsonschema-form component.

I think we should do a small bit of refactoring to make the ListingCreate component more robust and able to handle potential future extensions. First, I think we should remove the submit handler from the continue button here:

<button type="submit" className="float-right btn btn-primary">Continue</button>

Instead, we should have a handler which can read from the state and decide where to go next based on the state, so, ex:

<button type='button' onclick='goToNextStep()' className="float-right btn btn-primary">Continue</button>
...
goToNextStep() {
   // Pseudocode example checking various things, for example ..
   // If the current step is details and the selected schema requires photo uploads
   if (step === 'DETAILS' && this.state.selectedSchema.photos) {
      this.setState({step: this.STEP.UPLOAD_PHOTOS})
   }
   // If there are no more steps
   this.onDetailsEntered() // submit form
}

This would thereby enable us to in the future add more field types not supported by jsonschema-form if need be.

The STEP.UPLOAD_PHOTOS step could then just use html5 apis for photo handling and once completed, write the file blob back to the formData. Then, clicking continue would call goToNextStep and then onDetailsEntered.

A couple things to note, though:

  1. Probably would want to change the type of pictures in the json to input[type=hidden] rather than input[type=file], since we'll just be assigning the string result from the upload photos step back to the formData.
  2. Would have to manually run validation on each step, probably at the top of the goToNextStep method.
  3. It's worth keeping in mind that onDetailsEntered currently checks the state of the entire formData object, which is the result of multiple steps. Presenting validation errors that are the cumulated result of multiple steps on a single step (the step the user is currently presented) may be bad UI. For example, imagine the photos the user uploaded are too big. If the STEP.UPLOAD_PHOTOS step came chronologically before the STEP.DETAILS step, and the user filled out the data for both steps, and was currently on the STEP.DETAILS step, and tried to submit the form, they will get the error alert("Your listing is too large. Consider using fewer or smaller photos."), which would force them to go to a previous step, the STEP.UPLOAD_PHOTOS step to correct. That's kind of wonky. This does not currently present an issue but if the forms are ever to get more complicated, it could be a problem.

If you guys like this proposal, I'll go ahead and implement!

@wanderingstan
Copy link
Contributor Author

wanderingstan commented Feb 2, 2018

Thanks Nikk! Again, sorry for the delay--this came in midst of a ton of pull requests and just as the website repo was needed a ton of attention. Feel free to ping me on slack too.

we should have a handler which can read from the state and decide where to go next based on the state...

I feel a goToNextStep() abstraction could be premature optimization for such a small app, but go for it if you like.

The STEP.UPLOAD_PHOTOS step could then just use html5 apis for photo handling and once completed, write the file blob back to the formData

Yes, this seems exactly the way to do it!

  1. Probably would want to change the type of pictures in the json to input[type=hidden] rather than input[type=file], since we'll just be assigning the string result from the upload photos step back to the formData.

The schema json needs to best represent the data being stored, and shouldn't bend to UI considerations. My early thought on this was to use css to hide the image field on STEP.DETAILS, or see if react-jsonschema-form has some way to selectively hide fields. Other ideas?

EDIT: I think we might actually be saying the same thing -- leave the schema json alone, but hide the input.

  1. It's worth keeping in mind that onDetailsEntered currently checks the state of the entire formData object...

onDetailsEntered() should be paired with STEP.DETAILS as it's completion handler and validation. Then STEP.UPLOAD_PHOTOS would have its own, e.g. onPhotosUploaded(), which is where the photo size check should happen. I.e. each step of process should do it's own validation.

I'll also get you access to the full Zeplin graphic of Aure's flow design.

@wanderingstan wanderingstan added the good first issue Good for newcomers label Apr 18, 2018
@jordajm
Copy link
Contributor

jordajm commented May 12, 2018

@nikkwong thanks for the thorough research / game plan on this one. Are you still planning to implement it?

@micahalcorn
Copy link
Member

This should handle adding AND removing one image at a time in both the listing creation form and the messaging form.

@micahalcorn
Copy link
Member

micahalcorn commented Oct 26, 2018

This should also handle re-ordering the images, probably via drag-and-drop or something. The natural ordering (without incremental additions) is determined by the current sort setting of the native file select dialog, which may or may not be "by name".

@jordajm
Copy link
Contributor

jordajm commented Nov 1, 2018

This is mostly resolved by #731, but we would need to pull images out into its own step to make it work like Aure's designs. I'm thinking we should close this ticket and create a new one if/when we decide to do that...

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

No branches or pull requests

5 participants