Description: ------------ The default blank label used in select isn't good for radio buttons. The following patch creates a new option: radioAddEmptyLabel Test script: --------------- Index: FormBuilder.php =================================================================== RCS file: /repository/pear/DB_DataObject_FormBuilder/FormBuilder.php,v retrieving revision 1.228 diff -u -U 2 -r1.228 FormBuilder.php --- FormBuilder.php 7 Jun 2006 15:38:58 -0000 1.228 +++ FormBuilder.php 16 Jun 2006 08:17:56 -0000 @@ -478,9 +478,14 @@ /** - * An string to put in the "empty option" added to select fields + * A string to put in the "empty option" added to select fields */ var $selectAddEmptyLabel = ''; /** + * A string to put in the "empty option" added to radio fields + */ + var $radioAddEmptyLabel = '--None--'; + + /** * By default, hidden fields are generated for the primary key of a * DataObject. This behaviour can be deactivated by setting this option to @@ -1464,4 +1469,6 @@ $formValues[$key] = $this->_do->$key; if (!isset($element)) { + $isRadio = isset($this->linkElementTypes[$key]) + && $this->linkElementTypes[$key] == 'radio'; if (isset($this->enumOptions[$key])) { $options = $this->enumOptions[$key]; @@ -1485,5 +1492,7 @@ if (in_array($key, $this->selectAddEmpty) || !($type & DB_DATAOBJECT_NOTNULL)) { - $options = array('' => $this->selectAddEmptyLabel) + $options; + $options = array('' => $isRadio ? + $this->radioAddEmptyLabel : + $this->selectAddEmptyLabel) + $options; } if (!$options) { @@ -1491,5 +1500,5 @@ } $element = array(); - if (isset($this->linkElementTypes[$key]) && $this->linkElementTypes[$key] == 'radio') { + if ($isRadio) { $element =& $this->_form->_createRadioButtons($key, $options); } else {

Additional explanation: Up to now, it is possible to have an empty label in addition to existing field values in a select element. If a blank value is perfect for select, the result is quite misleading when used with radio buttons: a radio button with no text. The workaround is to modify $selectAddEmptyLabel to contain the radio text but IMO it is better to define 2 separate strings. This allows to define once for all the layout for select and radio buttons. And then each DO can choose to display as it prefers without having to care about the rendering.

Link to a colored, nicely presented version of the patch http://www.phpfi.com/146840

First of all, thanks for taking the time to make a patch. It's always best to link to a patch as pasting it into the bug makes it basically impossible to use directly. Thanks also for fixing some docblocks and bringing my attention to some invalid code but these would have been better in their own bug/patch. (Note: it's options, not portions. ;-) These are fixed in CVS now. However, I don't really see the utility in this feature. I do understand that an empty radio label isn't very useful but I don't understand how allowing an empty option for radios makes sense in the first place. An empty option for selects is meant to be set as the default option for a select box for a new record so that a real option is not set automatically as a default. It is meant to force the user to select an option manually. This doesn't make sense for radios since radio buttons don't have a default value selected for a new record. This is a difference between the nature of selects and radio groups. Let me explain it another way. selectAddEmptyOption injects an empty value into the top of a required select field so that the default value selected when creating a new record is not a valid option, forcing the user to select a real value. Radio buttons don't have a default value and so this is not needed. If what you're looking for is allowing an empty option to be really selected for a link then I would suggest adding a record to your linked table with the "empty" value you want.

You're right. I forgot to mention that I extended FB to create empty forms used to filter selected rows to display in a datagrid. You can look at http://www.phpfi.com/147073?lang=php to have a idea. So I agree it isn't needed by the default form. But there are other cases where it still might be useful. For instance if you want to extend the form to allow the user to leave a field as "unchanged", maybe when doing changes to several records at a time. Of course, it can be put in the extension class but it won't be so easy :-( (I knew potions wasn't correct! And I should have guessed it was more likely to be swapped letters...)

This bug has been fixed in CVS. If this was a documentation problem, the fix will appear on pear.php.net by the end of next Sunday (CET). If this was a problem with the pear.php.net website, the change should be live shortly. Otherwise, the fix will appear in the package's next release. Thank you for the report and for helping us make PEAR better. Ok, I can see that as a useful use case for this. I've applied your patch with some changes. 1) radioAddEmptyLabel is an empty string. I realize that the point of this fix is to *not* have an empty string as this label but I can't think of a label that would work for all cases. Keeping it empty also doesn't break BC quite as bad. 2) Your patch addressed only ENUMs. This is odd since this option is normally used for links. I've applied the same basic logic to links.