Let labels in CHtml optionally wrap input elements #373

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
8 participants
@mikehaertl

This change allows all <label> tags to optionally wrap an input element. The change affects all checkbox and radiobutton derivates.

  • CHtml::$labelWrapInputs. If turned on, all radioButtonLists and checkBoxLists will render with lables wrapping the input element. Off by default, so this change is backwards compatible.
  • CHtml::label and CHtml::activeLabel have 2 new htmlOptions: wrapInput (wraps input element, which can be specified in $for) and template which defaults to '{input} {label}'
<?php echo Chtml::label('test','<input ... />,'
    array('wrapInput'=>true)
) ?>

Gives <label><input .../> test</label>.

  • Because almost every checkbox or radio button need a label, this is was now included into CHtml::checkBox and CHtml::radioButton. Both have new htmlOptions label and labelOptions. If set, a label will automatically be rendered. A template can be specified in labelOptions (see CHtml::label). This change was neccessary to play nice with hidden inputs. Inside a label tag must only be 1 input element. So the hidden element must be rendered outside.
<?php echo Chtml::radioButton('test',false,array(
    'label'=>'Some label',
    'labelOptions'=>array('template'=>'{label} - {input}'),
)); ?>

Gives <label><input .../> - Some label</label>.

  • New htmlOptions for radioButtonList, checkBoxList and their active derivates: wrapInput. Defaults to CHtml::labelWrapInputs.
    <?php CHtml::$labelWrapsInput=true; ?>
    <?php echo Chtml::checkBoxList('test5',1,array(
        1=>'Value 1',
        2=>'Value 2',
    )); ?>
@phpnode

This comment has been minimized.

Show comment Hide comment
@phpnode

phpnode Feb 19, 2012

Contributor

I like the idea but not so much the implementation, why not just make all form elements take an optional label parameter?
E.g. this would wrap the element in a label with the value "Blah"

CHtml::textField("blah", "value",array("label" => true));

This would perform the same as functions as CHtml::activeLabel(), (check the return value of attributeLabels() and generate a default label)

CHtml::activeTextField($model, "blah", array("label" => true));

This would override the default label with some custom content

CHtml::activeTextField($model, "blah", array("label" => "A custom label goes here, can contain html"));
Contributor

phpnode commented Feb 19, 2012

I like the idea but not so much the implementation, why not just make all form elements take an optional label parameter?
E.g. this would wrap the element in a label with the value "Blah"

CHtml::textField("blah", "value",array("label" => true));

This would perform the same as functions as CHtml::activeLabel(), (check the return value of attributeLabels() and generate a default label)

CHtml::activeTextField($model, "blah", array("label" => true));

This would override the default label with some custom content

CHtml::activeTextField($model, "blah", array("label" => "A custom label goes here, can contain html"));
@qiangxue

This comment has been minimized.

Show comment Hide comment
@qiangxue

qiangxue Feb 19, 2012

Member

Please pay attention to CActiveForm to make sure form validation works when inputs are wrapped within labels.
I'd also like to see some alternative namings to $labelWrapsInputs.

Member

qiangxue commented Feb 19, 2012

Please pay attention to CActiveForm to make sure form validation works when inputs are wrapped within labels.
I'd also like to see some alternative namings to $labelWrapsInputs.

@mikehaertl

This comment has been minimized.

Show comment Hide comment
@mikehaertl

mikehaertl Feb 20, 2012

@qiangxue I already checked jquery.yiiactiveform.js but could not find anything specific to the label. But i'll have another look. Anything in particular where you know, my changes will break things? I'm also happy to rename the option - but what could be a better name? $enclosingLabels? $listLabels=(standalone|wrapping)? $enableListLabelWrapping?

@phpnode I agree with you that your proposal would give max flexibility. But from a pragmatic perspective i think radiobuttons and checkboxes are really the only element which will ever use it (at least that's my impression if i look at different CSS frameworks). So i tried to keep the impact small. Maybe your suggestion is better suited for 2.0?

@qiangxue I already checked jquery.yiiactiveform.js but could not find anything specific to the label. But i'll have another look. Anything in particular where you know, my changes will break things? I'm also happy to rename the option - but what could be a better name? $enclosingLabels? $listLabels=(standalone|wrapping)? $enableListLabelWrapping?

@phpnode I agree with you that your proposal would give max flexibility. But from a pragmatic perspective i think radiobuttons and checkboxes are really the only element which will ever use it (at least that's my impression if i look at different CSS frameworks). So i tried to keep the impact small. Maybe your suggestion is better suited for 2.0?

@phpnode

This comment has been minimized.

Show comment Hide comment
@phpnode

phpnode Feb 20, 2012

Contributor

@mikehaertl If you put this logic in activeInputField instead of activeCheckboxList and activeRadioButtonList then it can be used everywhere and i don't think it will have any negative implications. Your current approach of adding a parameter to CHtml::activeLabel() can still be used, this is just an alternative (imho slightly more convenient) way to accomplish the same thing. I don't think we need to wait for 2.0

Contributor

phpnode commented Feb 20, 2012

@mikehaertl If you put this logic in activeInputField instead of activeCheckboxList and activeRadioButtonList then it can be used everywhere and i don't think it will have any negative implications. Your current approach of adding a parameter to CHtml::activeLabel() can still be used, this is just an alternative (imho slightly more convenient) way to accomplish the same thing. I don't think we need to wait for 2.0

@qiangxue

This comment has been minimized.

Show comment Hide comment
@qiangxue

qiangxue Feb 20, 2012

Member

@mikehaertl Did you try activeform with your code? does it work as expected (such as error highlighting)?

Member

qiangxue commented Feb 20, 2012

@mikehaertl Did you try activeform with your code? does it work as expected (such as error highlighting)?

@ghost ghost assigned qiangxue Feb 21, 2012

@mikehaertl

This comment has been minimized.

Show comment Hide comment
@mikehaertl

mikehaertl Feb 22, 2012

@qiang Yes i did. The error logic was not changed - but even in current code it differs for clientside and serverside validation.

Elements which receive the error class on validation:

  • Clientside: container div
  • Serverside: label and input

Both work like before.

@phpnode I really focused on checkboxes and radiobuttons. If you look at the code you'll see that activeCheckBoxList and activeRadioButtonList don't use activeInput. But i can have another look and move the logic to activeInputField and inputField. @qiang What do you think about this?

@qiang Yes i did. The error logic was not changed - but even in current code it differs for clientside and serverside validation.

Elements which receive the error class on validation:

  • Clientside: container div
  • Serverside: label and input

Both work like before.

@phpnode I really focused on checkboxes and radiobuttons. If you look at the code you'll see that activeCheckBoxList and activeRadioButtonList don't use activeInput. But i can have another look and move the logic to activeInputField and inputField. @qiang What do you think about this?

@mikehaertl

This comment has been minimized.

Show comment Hide comment
@mikehaertl

mikehaertl Mar 21, 2012

Any news on this?

Any news on this?

@ghost ghost assigned cebe Sep 8, 2012

@whuhacker

This comment has been minimized.

Show comment Hide comment
@whuhacker

whuhacker Dec 6, 2012

+1 support.
When could this feature be included in the official release?

+1 support.
When could this feature be included in the official release?

@DrDeath72

This comment has been minimized.

Show comment Hide comment
@DrDeath72

DrDeath72 Mar 18, 2013

Make it by replace this string "{{input}label}" :)

Make it by replace this string "{{input}label}" :)

@magefad magefad referenced this pull request in clevertech/YiiBooster Mar 29, 2013

Closed

Chekboxrow, Label remove #411

@creocoder

This comment has been minimized.

Show comment Hide comment
@creocoder

creocoder Apr 17, 2013

Contributor

Vote to make this feature through CHtml::beginLabel(), CHtml::endLabel() and CHtml::labelTitle(). Also need to add placeholders {beginLabel}, {labelTitle} and {endLabel} to template option for CHtml::radioButtonList() and CHtml::checkBoxList().

Contributor

creocoder commented Apr 17, 2013

Vote to make this feature through CHtml::beginLabel(), CHtml::endLabel() and CHtml::labelTitle(). Also need to add placeholders {beginLabel}, {labelTitle} and {endLabel} to template option for CHtml::radioButtonList() and CHtml::checkBoxList().

@mikehaertl

This comment has been minimized.

Show comment Hide comment
@mikehaertl

mikehaertl Apr 18, 2013

To be honest i have not much hope that this is ever gonna be fixed anyway. This PR was opened more than a year ago and nobody seems to care anymore - which is a pitty. You can't really use Bootstrap with the default Yii form helpers.

To be honest i have not much hope that this is ever gonna be fixed anyway. This PR was opened more than a year ago and nobody seems to care anymore - which is a pitty. You can't really use Bootstrap with the default Yii form helpers.

@creocoder

This comment has been minimized.

Show comment Hide comment
@creocoder

creocoder Apr 18, 2013

Contributor

Solved by another (more Yii way) approach: #2370 . Tested with Twitter Bootstrap. All nice.

Contributor

creocoder commented Apr 18, 2013

Solved by another (more Yii way) approach: #2370 . Tested with Twitter Bootstrap. All nice.

@mikehaertl

This comment has been minimized.

Show comment Hide comment
@mikehaertl

mikehaertl Apr 18, 2013

Not sure if i agree that this is more Yii style than my proposed solution. :) Both give you the same level of freedom. But i don't mind as long as this finally gets fixed somehow at least.

Not sure if i agree that this is more Yii style than my proposed solution. :) Both give you the same level of freedom. But i don't mind as long as this finally gets fixed somehow at least.

@creocoder

This comment has been minimized.

Show comment Hide comment
@creocoder

creocoder Apr 18, 2013

Contributor

@mikehaertl Your solution is slightly overcoded and contains controversial things. By fixing only template option for CHtml::radioButtonList() and CHtml::checkBoxList() we get same result and 99% chance PR will be approved (i hope).

Contributor

creocoder commented Apr 18, 2013

@mikehaertl Your solution is slightly overcoded and contains controversial things. By fixing only template option for CHtml::radioButtonList() and CHtml::checkBoxList() we get same result and 99% chance PR will be approved (i hope).

@resurtm

This comment has been minimized.

Show comment Hide comment
@resurtm

resurtm Apr 19, 2013

Contributor

Fixed by merging #2370.

Contributor

resurtm commented Apr 19, 2013

Fixed by merging #2370.

@resurtm resurtm closed this Apr 19, 2013

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