symfony2admingenerator / AvocodeFormExtensionsBundle

(old-legacy) Symfony2 form extensions for Admingenerator project (also working standalone!)
Other
48 stars 31 forks source link

[WIP] Fix issue #59 #80

Closed sescandell closed 10 years ago

sescandell commented 10 years ago

This PR fix issue #59 in a "simple way".

On commit df27da06cab85bb04ceb41a479c7d9660ba06880 we allow to click multiple times on the add button making the file list growing automatically (and on save, all files are correctly uploaded). On a click on the main cancel button, all "wating for upload" files are removed. This works fine. Avantages: we can select multiple files from multiple directories in one simple upload Drawbacks: if we click only on cancel button of one file from the list of "waiting for upload", entry in the table presentation will effectively be removed. But the file will still be uploaded on form submission. This is because FileList associated to the input could not be modified in order to remove an item (files property is a readonly one).

So I made the commit 04254075f1d06253e0b7cf8f2bd3f7a69fdd885c in order to implement what @satiricon proposed in #28344375 : for each click on the "add files" button with new files selected, we simply remove all previously "waiting for upload" files. Advantages: no "fantom" upload Drawbacks: we can only add files from the same directory at a time. "Fantom" upload too...

All of this is only concern synchronous upload.

To prevent "fantom upload" I see three possibilities:

@loostro , @satiricon what do you think? For you, what's the best behavior?

satiricon commented 10 years ago

When the form is synchronous we may just leave the cancel button in the upper bar of the file list and hide the cancel buttons in the individual files. When the user clicks in this general cancel button the list is reseted and the input type="file" inside the add files button replaced by an empty, JavaScript created one (that way you/we avoid the files uploading if the button Cancel is pressed).

Also when the form is synchronous we can replace the "Add files" with "Select files" and as soon as new files are selected we reset the selected files list before the new ones are added and shown (this time we don't replace the input as the files selected are effectively the ones we want to upload).

I think that behavior would be fairly acceptable. Just a wrapper of the input type="file" that looks nice and show a little preview before uploading when the type is configured as synchronous.

Cheers!

sescandell commented 10 years ago

Hi @satiricon ,

I'll remove cancel button in presentation table, but I will not change the button name "add files" to "select files" because in an edit context, we are not affectively selecting files, but adding some...

We still have some drawbacks with this solution: we can't select files from different folders. I will merge the PR as it, and, later, we'll change that to allow multiple directory with no "fantom upload".

satiricon commented 10 years ago

@sescandell Just tested and it's working great!

I would love to see what kind of solution you come up allow multiple folder selection. I thought, like you said, in doing some kind of configuration field in witch you can say the server which fields to save and which not but that may get to be quite the problem with larger files. Another possibility is to use flash instead of HTML5 to select the files, not that hard, just a flash movie instead of the input in the "add files" button. But flash is a dying technology and maybe we should leave it rest in peace.

Maybe the best strategy is to use the async file upload... I still haven't tried to use it but I probably will one of these days.

Thanks for your work!

sescandell commented 10 years ago

Hi @satiricon

I thought, like you said, in doing some kind of configuration field in witch you can say the server which fields to save and which not but that may get to be quite the problem with larger files.

It's exactly something like that. Adding hidden input for each "real selected file". Thanks to that, on the server side we can only handle "matching" files... (but I'm not totaly sure if this is possible).

Another possibility is to use flash instead of HTML5

True... and GenemuFormBundle already provide this kind of FormType... But... as you said... I'm not a fan of using Flash "just for that"... Asynchronous upload is, in my opinion, a better idea (given that the solution provided by the "flash" version from GenemuFormBundle also works asynchronously... but with flash :) ).

Maybe the best strategy is to use the async file upload

I think this is the best way to proceed too... but since these last changes, I think we have to "test it" one more time (and probably fix it)...

ioleo commented 10 years ago

@sescandell @satiricon I've read the discussion and I'd like to add a few comments:

I think that the asynchronous file upload should be the default for this widget. I believe we should write some upload handler class that recieves files (from POST) and saves them in a temporary location. E.g.

After the upload is complete, our widget recieves (through AJAX) the unique ID for our file. When we "submit" the form -> only the unique ID is submitted (in a hidden field).

On server side, if the form (and files) are valid -> we move the file out of temporary location to where it belongs, create associated entity, save, flush, and in the end -> remove the temporary directory (/30d4094921ae3) that is no longer needed.

We know which file we want, becouse we have the unique ID submitted.

Pros:

Cons:

sescandell commented 10 years ago

@loostro

I believe we should write some upload handler class that recieves files (from POST) and saves them in a temporary location.

This is something I also thinked about... but just don't have time to implement it for now (I talked about that in the comment of PR #76 ). Maybe I can work on that in coming days... (I have to implement this "handler" for a personnal project... and so, by the way, test the asynchronous behavior).

Waiting for this PR, I think synchronous behavior is a good alternative.