marinosoftware / active_storage_drag_and_drop

Provides a form helper to make it easy to make drag-and-drop file upload fields that work with Rails' Active Storage.
MIT License
92 stars 14 forks source link

Persist on form rerender #3

Closed cseelus closed 5 years ago

cseelus commented 5 years ago

Is the file_field supposed to persist on a form rerender, for example when any validation for the form object fails and the form gets redisplayed?

IanGrantMarino commented 5 years ago

Hi Chris,

The file fields should persist when re-rendering the form when they have not been successfully attached to the parent object (i.e. the ActiveStorage::Attachment has not been persisted). This would be the case when a validation is failed. Persisted attachments should not be displayed.

The reason it's currently handled this way is that files don't have to be uploaded again in the case of an unrelated validation error. This should hopefully save your users time and bandwidth in the case that they are uploading a large file and there is a validation error elsewhere in the form. If you would prefer not to have this behavior we can look into allowing an option to be passed to disable it.

If you're experiencing behavior different from what I've described please let me know.

cseelus commented 5 years ago

Hi Ian, thanks for the fast reply.

The approach you describe sounds reasonable, thats what I was expecting and I think most Users would expect: When the form redisplays because the validation of an (unrelated) attribute of the form object fails, the attachment should be persisted.

You write that persisted attachments are not displayed though. So the User has no way of knowing that the file(s) persisted. Thats also what I observed, when testing your Gem and what got me thinking they aren't persisted.

IanGrantMarino commented 5 years ago

If you need to display the persisted attachments to show the user which ones have already been attached you can access them from the attachment on the record e.g. user.images. Then select the persisted ones: user.images.select(&:persisted?)

If you want them shown inside of the drag and drop zone you can pass it to the helper in a block:

= form.drag_and_drop_file_field :images do
  Drag and Drop images here
  %ul.saved-images
    user.images.select(&:persisted?).each do |image|
      %li= image.filename

We could look at optionally including a default listing of persisted attachments but there are design changes coming in 1.0.0 so it would be after that.

cseelus commented 5 years ago

Yes, I did something similar to your example to show them inside the drag and drop area.

In the long run, having a default listing of persisted attachments like you proposed, would be a sensible default I reckon.