Fixing FileField, part... er... 8, or something? Anyways.
As you probably know by now, filefield 6.x does some funky AHAH replacements too, just like content.module's "add more" button, only a bit different. I have no problem duplicating code all over the place unless it gets ugly, which would happen if I want to properly implement two features:
1. Updating file upload requirements (say, file size left for the whole field) on multiple widgets at once, and sometime in the future,
2. Implementing the dreaded archive upload where it's necessary to expand one file (= widget) into multiple files (= widgets).
Maybe there are more use cases, maybe for filefield or maybe for other modules too. Those are the two that matter to me right now.
Now if I want to replace multiple upload requirement messages, in practice that means that I need to replace the whole field form instead of just my single filefield widget. Same with item expansion. And if I want to replace the whole field form, I need to implement all of content_add_more_js(), which is where it gets ugly because in filefield I really do not want to mess with '_weight' and other possible quirks that might appear there in the future.
So (you guessed it), here's a patch that makes it possible for filefield and other modules (including the "add more" functionality) to reuse CCK's JavaScript field form callback, and add their own spice by hooking into it with mostly one single hook, ${callback_base}_state_alter. You can just change the $form_state there (item count for "add more", the items themselves for filefield uploads/deletions, etc.) and get the finished form back to AHAH automatically. Validating the $field is also necessary, as "add more" did that before already (checking if the $field is really "Unlimited").
More ambitious natures want to add some spice like the AHAH class that marks a div as "new element in the form" and receives the fade effect for example. I moved that out of the JavaScript callback and into the form generation itself, because if you've got more complex needs than just knowing which element is the last in the list, then you need the context of the item, and with the deltas being swapped this is unnecessarily hard to get otherwise. So it should go where the context is available anyways, which is the form (or widget) callback.
And on the way, I also fixed a bug that appears when used with a demanding use case like filefield - $_POST must not be passed to the second form_builder(), because the form structure has already changed then and doesn't match the structure of $_POST values. Which causes all my form elements to use empty values instead of the default values that I had intended for them ("List" being checked by default, filename being used as default description, plus extensions providing more elements). So the $_POST needs to go into the first form_builder() where the form structure still matches.
I understand that this is yet a bit more code for your JS callback, but in turn I can get completely rid of my own copy (which was nearly as large), and get additional flexibility too. Please review and hopefully commit.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | cck_281548-make_js_callback_reusable.patch | 7.57 KB | sirkitree |
| #1 | filefield-depend-on-content-js.diff | 12.46 KB | jpetso |
| cck-reusable-js-callback.diff | 7.76 KB | jpetso |
Comments
Comment #1
jpetso commentedAnd for reference, here is how I intend to adapt filefield if the above patch gets applied. Pretty neat, eh?
Comment #2
dopry commentedbumping so I will remember to review tomorrow.
Comment #3
yched commentedI have nothing against the idea in general, but this part of the code is really non-trivial (as #273539: 3F #3: Value callbacks not executed on JS "add more" showed).
if we want to abstract it one step further, then the code needs to be extra documented and commented so that we limit support requests and make future maintenance easier.
Comment #4
sirkitree commentedHere is a re-roll against 6.2-2
I'm here testing this to try to solve the problem where if you form alter a nodereference field the AHAH response doesn't have those. Please let me know if I'm going down the wrong path here.
Comment #5
sirkitree commentedSo actually i was a bit out of touch here. I only needed to make sure that we're using drupal_alter() on the full form, that is passed through, not just the element. I've posted my findings here: #416074: drupal_alter() form instead of form_element in AHAH callback
Comment #6
mikeytown2 commentedsubscribe +1. Coming from http://drupal.org/project/imagefield_zip