Add disabled attribute to select options
cdale - July 20, 2008 - 10:40
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | forms system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
Description
Recently had a need for this in one of my own projects and didn't really like that the only way to achieve this was to override the theme function for the select box in question.
This patch adds a new FAPI type '#disabled_values' which would only be valid for select options. An example of using it would be as follows:
$form['test'] = array(
'#type' => 'select',
'#title' => 'TEST',
'#options' => array('test' => 'Test', 'test1' => 'Test 1', 'test2' => 'Test 2', 'test3' => 'Test 3'),
'#disabled_values' => array('test1', 'test2'),
'#default_value' => 'test2'
);In the above example code, 'test2' is the default value, as well as one of the disabled options, in this case, 'test2' will actually be enabled as it needs to be a selectable option to maintain integrity of submitted data. I'm sure there are other ways to handle this, but this seems the cleanest.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| form.inc_.patch | 887 bytes | Idle | Failed: Failed to apply patch. | View details | Re-test |
#1
Here is a patch against 6.3.
#2
And checkboxes and radiobuttons, ideally; basically anything with #options.
Edit: Please make a patch against 7.x-dev, seeing as it is already radically different in multiple places.
#3
Seems a bit cleaner to allow the options value to be an array and set disabled there. just like how table cells can be an array. if we do a separate property, i suggest putting options in the name.
#4
Is 7.x-dev the same as HEAD? If so, the first patch, 'form.inc_1.patch' is against that. If it's not HEAD, which CVS tag do I checkout to make the patch?
The second is against 6.3.
#5
@cdale - D7 is HEAD, yes.
At first glance, I think I agree with Moshe's comments in #3.
#6
I thought about this too, but when looking at the code, I saw that when the options value is an array, it actually creates an optgroup with the label of the group being the key, and contained options being the array of values. It might be possible to use an object and change a little bit of the logic, as there is also an is_object check, but it doesn't seem to do anything special with it. It just felt cleaner to add a new property. I'm open to suggestions though.
I agree on the options in the name part. I've re-roll the patch using #disabled_options instead of values. I had actually considered using the #disabled property, but was unsure if setting this to be an array would break other code where it expected the value to be a boolean.
#7
Too bad IE6 and 7 don't support disabled options for select fields. :)
#8
@ #4: Yes, HEAD = 7.x. I didn't check your first patch; I just saw that the second was for 6.3 and made assumptions. ;)
Also, we don't rely on client-side validation anyway. If the browser sucks, about the worst thing that can happen is that the user submits the form and gets a message about an invalid selection.
#9
As far as I can tell, #disabled is unused. An entire field is disabled by setting ['#attributes']['disabled']. However, #disabled would be easily mistaken for a boolean option to disable the entire field, so we shouldn't use it for this.
---
Here's a new patch that adds this functionality for checkboxes and radios, also adding it as a system default to system_elements.
There's also a code-style fix in the checkboxes function, where I split a big single line array into multiple lines.
#10
Great. Patch works for me with Radios, Checkboxes, and Selects each with multiple different options and configurations. :)
#11
Having thought about it, Should the #default_value for radios always be enabled? Even if it is in the #disabled_options? That way, an accidental click can be corrected, rather than having a potentially wrong value submitted that, even on validation error could most likely not be corrected?
#12
The problem does make sense.
A disabled select option will never be selected by default, and a disabled checkbox may well be desired to be checked by (unalterable) default. So this affects only radio buttons.
I'm not sure how to resolve precedence, but I prefer your suggestion of ignoring commands to disable the default. The alternative of discarding the default when it is disabled seems rather counter-intuitive to me; if a module really wishes an option to be disabled, it should just avoid setting it as a default.
#13
Updated patch that does not disable the radio option if it is the default value.
#14
That works well. Code style seems to be okay too.
I'm not quite sure if the modification I made to system_elements is correct. There are some properties that elements support, but that are not mentioned in system_elements - therefore not sure if disabled_options should be explicitly mentioned. A Form API guru should sign off on that.
#15
subscribing
#16
Patch broken. Fortunately this is merely because the aforementioned code-style fix has been implemented on its own by now. Hence, rerolling without this fix.
#17
The last submitted patch failed testing.
#18
Being able to disable options per element is a very common requirement, and have been supported by HTML for ages. Promoting to a bug.