What should we do with checkboxes?
Hi! There are two kinds of elements where you get an array as the value: multiple selects and checkboxes. Upon submit of a multiselect, you get a list of selected values. With checkboxes, however you get an associative array of something_that_has_the_information => 1 the information is in the *keys*. Note that #options have the same structure for multiselects and checkboxes: something_that_has_the_information => display_this. Currently, core has a problem: #default_value for checkboxes holds a list of something_that_has_the_information and it breaks serious havoc, because #value is either a list of these (when filled from #default_value) or it is an array of something_that_has_the_information => 1 when filled from submit. There is a special case: when you do not check any checkboxes on submit then it'll be copied from #default_value. Therefore the structure of #value changes per submit. This simply does not work, #value always needs to have the same structure. Either we change #default_value structure or we change #value. a) We could make #default_value an array of something_that_has_the_information => 1 pairs. This would make it more complicated to build #default_value and it's looking strange. b) we change #value instead to be a list of selected values, like in multiselect. The code change in form.inc for this is minimal: if ($form['type'] == 'checkboxes') { $edit = array_keys(array_filter($edit)); } I like this one much better. The only problem is that somewhat it differs from what you get from HTML -- but this is the information you actually need. I feel like this is something for 4.8. For 4.7, I suggest a whole another approach: we take #default_value as a list of checked things (that's what we have now) and change it in form.inc to be a). If this feels hackish, there are 16 #type => checkboxes in core and if do either a) or b) then we need to check at least 16 screens but most of these are settings, so we need to check whether they take effect. For example, we need to check whether vocabulary settings result in the correct node form. Happy holidays, Karoly Negyesi
Hi, Op zondag 25 december 2005 05:12, schreef Karoly Negyesi:
I feel like this is something for 4.8. For 4.7, I suggest a whole another approach: we take #default_value as a list of checked things (that's what we have now) and change it in form.inc to be a). If this feels hackish, there are 16 #type => checkboxes in core and if do either a) or b) then we need to check at least 16 screens but most of these are settings, so we need to check whether they take effect. For example, we need to check whether vocabulary settings result in the correct node form.
I seriously doubt anyone will object against this route. As yout also state, the big change should be for 4.8. Module conversion is not giong all that fast, as it is now (i suspect more people, like me, are waiting for the moment when 'it has all now really stabilized') . We should now move forward asap, imo. Even if that means having some 'hackish feelings'. Bèr -- PGP ber@webschuur.com http://www.webschuur.com/sites/webschuur.com/files/ber_webschuur.asc PGP berkessels@gmx.net http://www.webschuur.com/sites/webschuur.com/files/ber_gmx.asc
On 25 Dec 2005, at 11:16 AM, Bèr Kessels wrote:
I seriously doubt anyone will object against this route. As yout also state, the big change should be for 4.8. Module conversion is not giong all that fast, as it is now (i suspect more people, like me, are waiting for the We're at 4.8 already ? =)
Dries, could you supply us with a list of the top 10 or 20 most downloaded modules, so that we can check them off as they get converted ? -- Adrian Rossouw Drupal developer and Bryght Guy http://drupal.org | http://bryght.com
At 12:11 PM +0200 25/12/05, Adrian Rossouw wrote:
Dries, could you supply us with a list of the top 10 or 20 most downloaded modules, so that we can check them off as they get converted ?
At 4:49 PM +0200 19/10/05, Dries Buytaert wrote:
Hello world,
attached is a list of the top 15 downloaded contrib modules. It represents the modules we should update as soon as possible. Upgrading these modules is important in order to get Drupal 4.7 (release candidates) tested.
1. image module --> 3695 downloads/month 2. gallery module 3. flexinode module 4. tinymce 5. event module 6. fckeditor module 7. img_assist module 8. article module 9. ecommerce module 10. filemanager module 11. album module 12. front module 13. theme_editor module 14. banner module 15. gsitemap module
-- Dries Buytaert :: http://www.buytaert.net/
On 25 Dec 2005, at 6:12 AM, Karoly Negyesi wrote:
b) we change #value instead to be a list of selected values, like in multiselect. The code change in form.inc for this is minimal:
if ($form['type'] == 'checkboxes') { $edit = array_keys(array_filter($edit)); }
I like this one much better. The only problem is that somewhat it differs from what you get from HTML -- but this is the information you actually need. I like this approach too , but I don't really like the idea of adding type specific code into form_builder itself. The function is already too hard to comprehend as it is.
why not do : if (function_exist($form['#type'] . '_value')) { ${$form['#type'] . '_value'}($form); } else { $form['#value'] = $form['#default_value']; } ------------- function checkboxes_value(&$form) { $value = array(); foreach ($form['#default_value'] as $key) { $value[$key] = 1; } $form['#value'] = $value; } We never know if some other element might have a similar problem in the future. and at least now we would be able to fix it for just that element without having to resort to changing form_builder. -- Adrian Rossouw Drupal developer and Bryght Guy http://drupal.org | http://bryght.com
b) we change #value instead to be a list of selected values, like in multiselect. The code change in form.inc for this is minimal:
if ($form['type'] == 'checkboxes') { $edit = array_keys(array_filter($edit)); }
I like this one much better. The only problem is that somewhat it differs from what you get from HTML -- but this is the information you actually need.
I like this much better too. Drupal developers hardly remember how HTML works anyway :). I would not mind if we did this for 4.7.
On Sun, 25 Dec 2005 16:25:09 +0100, Moshe Weitzman <weitzman@tejasa.com> wrote:
b) we change #value instead to be a list of selected values, like in multiselect. The code change in form.inc for this is minimal: if ($form['type'] == 'checkboxes') { $edit = array_keys(array_filter($edit)); } I like this one much better. The only problem is that somewhat it differs from what you get from HTML -- but this is the information you actually need.
I like this much better too. Drupal developers hardly remember how HTML works anyway :). I would not mind if we did this for 4.7.
Oh I would not mind too. I'd be even happy to provide a patch. But who will review? :) Regards NK
Karoly Negyesi wrote:
Either we change #default_value structure or we change #value.
a) We could make #default_value an array of
something_that_has_the_information => 1
pairs. This would make it more complicated to build #default_value and it's looking strange.
b) we change #value instead to be a list of selected values, like in multiselect. The code change in form.inc for this is minimal:
if ($form['type'] == 'checkboxes') { $edit = array_keys(array_filter($edit)); }
I think (b) is uglier and less efficient than (a). (a) also makes more logical sense to my way of thinking. I don't find it strange looking; it actually rather resembles how HTML works. My 2 cents. ..chrisxj
participants (6)
-
Adrian Rossouw -
Bèr Kessels -
Chris Johnson -
Karoly Negyesi -
Moshe Weitzman -
Richard Archer