Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Form\Element\MultiCheckbox Set multi_checkbox as default attribute type #2272

Merged
merged 1 commit into from Aug 30, 2012

Conversation

Projects
None yet
4 participants
Contributor

kpieters commented Aug 30, 2012

Removed 'multicheckbox' as a type.
Added unit test.

It's also possible to also use 'multicheckbox', then it also need to be added to Form/View/Helper/FormElement.php so that the right plugin is used

Form\Element\MultiCheckbox removed 'multicheckbox' as a type. Set mul…
…ti_checbox as default attribute type. Added unit test

This pull request passes (merged 10a7274 into 89eaf41).

@cgmartin cgmartin commented on the diff Aug 30, 2012

library/Zend/Form/View/Helper/FormRow.php
@@ -82,7 +82,7 @@ public function render(ElementInterface $element)
// Multicheckbox elements have to be handled differently as the HTML standard does not allow nested
// labels. The semantic way is to group them inside a fieldset
$type = $element->getAttribute('type');
- if ($type === 'multi_checkbox' || $type === 'multicheckbox' || $type === 'radio') {
+ if ($type === 'multi_checkbox' || $type === 'radio') {
@cgmartin

cgmartin Aug 30, 2012

Contributor

We might to leave this line with 'multicheckbox' just in case...so there's no breaking changes for users with existing forms that use this type string?
This probably has a low probability of causing problems (and easy to fix), but just wanted to note it.

@cgmartin

cgmartin Aug 30, 2012

Contributor

@kpieters , I just saw your note about the FormElement helper which uses 'multi_checkbox'... I see what you mean. Doesn't seem like any value to keeping 'multicheckbox' in since it doesn't currently work anyway with FormElement. Carry on... ;) Looks good

weierophinney added a commit that referenced this pull request Aug 30, 2012

[#2272][#2269][ZF2-505] Updated README
- Noted fix for multi-checkbox rendering

@weierophinney weierophinney merged commit 10a7274 into zendframework:master Aug 30, 2012

Owner

weierophinney commented Aug 30, 2012

Merged to 2.0.0, 2.0.1, and master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment