backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 40 forks source link

[UX] Support "multiple" HTML5 attribute on File elements #440

Open quicksketch opened 9 years ago

quicksketch commented 9 years ago

The HTML5 "multiple" attribute allows multiple files to be selected and uploaded at once. In Backdrop, we should integrate this into the "managed_file" element and the File module field.

Sub-issue of https://github.com/backdrop/backdrop-issues/issues/200

This should be relatively portable from Drupal 8.

Commit: http://drupalcode.org/project/drupal.git/commit/8fd2b4779ec83c3e44a8caac65300b96e3a52ab8 Issue: http://drupal.org/node/625958


PR by @docwilmot: https://github.com/backdrop/backdrop/pull/2430 PR by @quicksketch (based on @docwilmot's): https://github.com/backdrop/backdrop/pull/3030 PR by @docwilmot: https://github.com/backdrop/backdrop/pull/3755

quicksketch commented 9 years ago

I'd love to get this in the next release, as we don't have time for this in 1.1.0.

docwilmot commented 9 years ago

This has been languishing for a year now. I've had a look at whats involved and its a bit above me. Can a person who is comfortable with File module and fields have a stab at this to move it along?

Just shepherding 1.3.0 issues.

jenlampton commented 8 years ago

Not sure this is going to make 1.3, but I'd also love to see it get in.

klonos commented 8 years ago

I have forgotten about this issue here. So forgive me if #1380 is a dupe. I had more in mind the D8 feature parity meta issue rather than the HTML5 meta.

serundeputy commented 8 years ago

Moving to 1.5; still a good idea!

klonos commented 8 years ago

I think that this one is pretty much what was left from #200. Do we have time to get this done for 1.5.0??

olafgrabienski commented 8 years ago

Getting it in 1.5 would be great! (In my case an important feature to be able to build a portfolio website for a photographer.)

jenlampton commented 8 years ago

We're less than one week away from release and there's no PR here yet. I'd say no, this likely won't make it. Officially moving to 1.x-future.

olafgrabienski commented 7 years ago

Unfortunately, I can't help with the code, but let us not forget this issue. (Just came back to it because I'm currently building a site where I could need it multiple image upload for a gallery.)

olafgrabienski commented 7 years ago

Added "[UX]" to the issue title, hope that's okay!

laryn commented 7 years ago

Everybody may be aware of this module already but I'll add it here since it hasn't been mentioned yet. Not sure if any code from this module can make this task easier:

https://backdropcms.org/project/html5_upload

klonos commented 7 years ago

These too:

https://www.drupal.org/project/multiupload_filefield_widget https://www.drupal.org/project/multiupload_imagefield_widget

docwilmot commented 5 years ago

@klonos you mentioned multiple uploads recently. Want to give this a whirl? https://github.com/backdrop/backdrop/pull/2430

docwilmot commented 5 years ago

So I wanted to do this because it came up recently, and because @quicksketch said it "should be relatively portable from Drupal 8". I realise now that his "relatively" may have a different meaning. FieldAPI is such a morass of hooks and forms, I cant fix image defaults, which should arrays with this PR. Obviously this isn't going to make it to 1.12, so putting it back to rest till next time.

I think it may be close, the multiple-ness of fields is working, just need someone with FieldAPI skills to finish.

quicksketch commented 4 years ago

Hey @docwilmot, it's been a year since this work but I have to say: this is great! I tried out your PR and it actually nearly works! Very cool!

In the Drupal 8 approach, the file_save_upload() function was reworked to always return an array instead of a single file. From an API-compability perspective, I don't think that's an option for us. Similar changes to the managed_file element (where $element['fid'] is changed to $element['fids']) are also going to cause problems for contrib modules.

I took a stab at trying to maintain compatibility while also adding multiple file support. It's coming along but it's not fully working yet. In-progress PR at https://github.com/backdrop/backdrop/pull/3030

klonos commented 4 years ago

I have done a quick CR (mainly suggestions on inline code comments, and user-facing text), and I have tested on the PR sandbox. Looks very promising! ❤️ ...thanks @docwilmot and @quicksketch 👍

quicksketch commented 4 years ago

Not sure how this got in 1.15.1. It should be backwards compatible when finished, but it's a pretty big change (and not a bug fix). I'd be happy to have it in 1.16.0 though. :smile:

quicksketch commented 3 years ago

@docwilmot refreshed his PR at https://github.com/backdrop/backdrop/pull/3755. I gave it a code review and it's looking great. Still lots of test failures related to File module.

docwilmot commented 3 years ago

Thanks for the review. I am finding field API to be now more annoying than JQuery TBH, and chipping away at this PR one line at a time. Dont know when I'll figure it out enough to be able to finish. 🙂

olafgrabienski commented 3 years ago

In the PR's sandbox website multiple files (e.g. images for Posts) are however already working nicely!

klonos commented 3 years ago

Perhaps rebasing the PR branch and letting the new GHA tests run would be a good start to revive this? @docwilmot ?

docwilmot commented 2 years ago

Shout out to @hosef who I threatened to pass this on to. He didnt disagree, as far as I recall.

ghost commented 2 years ago

I've had a look at whats involved and its a bit above me.

That's disconcerting... If @docwilmot is struggling with this, how can mere mortals hope to contribute to this issue? 🙂

docwilmot commented 2 years ago

I did attempt to pass this on to the Eternal @hosef but he did not heed my supplication. Perhaps I should sacrifice a goat. 🤔

klonos commented 2 years ago

In the Drupal 8 approach, the file_save_upload() function was reworked to always return an array instead of a single file. From an API-compability perspective, I don't think that's an option for us.

I understand how changing the return type of file_save_upload() would be a BC-breaking API change, but could/should we introduce a new file_save_upload_multiple() function, which always returns either an array of File objects, or FALSE? (when an error occurred) - if no file was uploaded, the array would be empty (instead of NULL that file_save_upload() returns in that case).

quicksketch commented 2 years ago

but could/should we introduce a new file_save_upload_multiple() function.

Yes, I think that may be a great approach and limit the complexity inside of file_save_upload(). We could potentially extract the similarities into a 3rd function and then put only the differences in file_save_upload() vs file_save_upload_multiple().

jenlampton commented 1 year ago

Would (or could) this change also allow multiple files at a time for image fields?

olafgrabienski commented 1 year ago

Would (or could) this change also allow multiple files at a time for image fields?

I think so, see my sandbox testing from October 2021:

In the PR's sandbox website multiple files (e.g. images for Posts) are however already working nicely!

stpaultim commented 1 year ago

Just noting that this issue is still top of our forum wishlist. https://forum.backdropcms.org/feature-requests

I see that there is a PR, but re-reading the discussion it's not clear to me exactly how close to completion this issue is or if there have been any new developments which might effect it's feasibility.

Back in Oct 2021, @quicksketch wrote "I gave it a code review and it's looking great. Still lots of test failures related to File module."