form validation css support

Create issue
Issue #30 resolved
Manuel created an issue

I've just noticed that only the Horizontal form is supporting style for inline/ajax validation We should add it to vertical and the others

The problem is that this code is missing in BootInput Vertical/Inline, why?

{{{ #!php

/* * Runs the widget. / public function run() { echo CHtml::openTag('div', array('class'=>'control-group '.$this->getContainerCssClass())); parent::run(); echo '</div>'; } }}}

and this div wrapping is missing too:

{{{ #!php

echo '<div class="controls">'; ... echo '</div>'; }}}

should I make a PR?

Comments (16)

  1. Christoffer Niska repo owner

    Well Bootstrap vertical forms doesn't have control-groups or controls since that would break, e.g. inline forms. I'd suggest using errorSummary instead of displaying errors for each field.

  2. Christophe

    Bootstrap doc says that Vertical form should be "lighter and smarter", with "no extra markup"... I find that a bit restrictive. I totally agree for inline forms, which are designed to be compact, and therefore, an errorSummary displayed on submit is sufficient.

    But for Vertical Forms, my opinion is that we should have a surrounding tag (control-group ?) at least to allow to style errors like in Horizontal forms. The drawback is that .control-group class has a default margin-bottom set to 9px (@baseLineHeight/2), but, if needed, can be easily fixed in own style sheet.

    To sum up, I would vote to add this piece of code in BootInputVertical :

    public function run()
    	echo CHtml::openTag('div', array('class'=>'control-group '.$this->getContainerCssClass()));
    	echo '</div>';

    And leave BootInputInline as is (i.e. no ajax validation, only on submit with errorSummary)

  3. Christophe

    I don't think wrapping controls is useful for Vertical forms (except maybe for radio/checkbox). If you look at bootstrap css (or at forms.less), you'll see that .controls class is only used in .form-horizontal.

  4. Manuel reporter

    I was thinking that too before seeing this and from the source-code of there's this vertical-form:

            <div class="control-group">
              <label class="control-label" for="inputIcon">Email address</label>
              <div class="controls">
                <div class="input-prepend">
                  <span class="add-on"><i class="icon-envelope"></i></span>
                  <input class="span2" id="inputIcon" type="text">

    The result with or without the .controls div look the same, so we can leave it.

    Consider that BootInputInline extend from BootInputVertical so we need a check before adding .control-group

    So I would add this code to BootInputVertical

    	public function run($inline=false)
    		if (!$inline) echo CHtml::openTag('div', array('class'=>'control-group '.$this->getContainerCssClass()));
    		if (!$inline) echo '</div>';

    And this to BootInputInline

    	public function run($inline=true)
  5. Christophe

    You're right for BootInputInline, but I wouldn't change CWidget::run() signature in derived classes :)

    Just put this in BootInputInline :

    public function run ()

    But i'm thinking of another approach, more generic :

    In BootInput base class:

    public function run ()
      switch ($this->type)
        // current code
    protected function beginControlGroup ()
      echo CHtml::openTag ('div', array('class'=>'control-group '.$this->getContainerCssClass()));
    protected function endControlGroup ()
      echo "</div>";

    and override the begin/endControlGroup method in BootInputInline.

    but let's wait Chris opinion on this. It's his extension ! :)

  6. Christoffer Niska repo owner

    Lets say that we'd add control-groups for vertical forms, in that case I would also wrap the controls in an own div to stay consistent. I've been thinking about always making the success/warning/error inline in horizontal forms (because that's how it is in their docs), this would make everything more simple. Then on the other hand we could use success/warning/error blocks in vertical forms. That would add a bit more flexibility to the forms imho.

    However, I wouldn't change the run signature. Instead I wouldn't extend the inline form from the vertical one and change the code in the run methods to generate the correct code.

    BTW, what do you two think about the current form implementation, is it good enough? I think there is still room for improvement but overall it works quite nicely and it's quite easy to change the code without breaking BC.

    I'm also planning to write a CSS file that would sort of bridge Bootstrap and Yii by adding the missing CSS such as .required, grid styles, etc. What do you think about that?

    Maybe we should start a google group instead of discussing the development here in the issues? :)

  7. Former user Account Deleted

    I understand this is a problem of original bootstrap's intention and bootstrap css and yii-bootstrap is only facilitating this. However, ideally the entire decision of displaying errors in horizontal/inline/vertical forms is left to the developer. Lets say there's error styles in place for all 3 form types - control-group for horizontal and vertical; and something else that only does the focus of the form element with no additional help-inline for inline forms. Then we could add another parameter to BootsFormAction named "displayErrors" where the default value is true for horizontal and false for inline and vertical. Just an idea... cheers, yiidf

  8. Log in to comment