[Forms] Bootstrap Theme: Explain that `form_errors()` is harmful

[ThomasLandauer]

Since form_label() now also renders the validation error messages, calling form_errors() results in having the error messages twice. Should I come up with a PR for https://symfony.com/doc/current/form/bootstrap4.html#labels-and-errors explaining that calling form_errors() isn't a good idea at all?

My only question: Is this true for all fields or are there any special cases to consider?

Second (minor) question, while I'm at it: This sentence only applies to checkbox and radio, right? (it's a little ambiguous - I'd make it clearer):

Due to Bootstrap internals, the label is already rendered by form_widget().

Ping @Nyholm & @mpiot

[mpiot]

@ThomasLandauer I've check to be sure, in the Bootstrp 4 theme only apparently:

• The form_errors() for each field is called in form_label(), then if you render fields manually like:

  form_label(form.field)
  form_widget(form.field)
  form_help(form.field)
  form_errors(form.field)

  The form_errors is called twice: in the label, and after the label. Because the call of form_errors() is in the form_label(), it's right for all.

• The checkbox and radio inputs, have this HTML:

  <div class="form-group form-check">
  <input type="checkbox" class="form-check-input" id="exampleCheck1">
  <label class="form-check-label" for="exampleCheck1">Check me out</label>
  </div>

To render it, the form_widget() render the form_label(), because this widget is the checkbox and its label. In that case: form_widget() call form_label that call form_errors(). This only takes place fo checkboxes and radio.

Maybe wait a little an answer from @Nyholm to double check.

[Nyholm]

Is this true for all fields or are there any special cases to consider?

I dont think there is any special case. Sure the choice are different with their legend

This sentence only applies to checkbox and radio, right?

Yes, I think so.

The checkbox and radio inputs, have this HTML:

@mpiot Are you sure about this?

I have not checked, But I was convinced it was more like:

  <div class="form-group form-check">
        <label class="form-check-label" for="exampleCheck1">
             <input type="checkbox" class="form-check-input" id="exampleCheck1">
             <span>Check me out</span>
      </label>
  </div>

[mpiot]

@Nyholm I've check in one of my app, I've. I think this is the previous render (Bootstrap 3).

From my app:

<div class="custom-control custom-checkbox">
  <input type="checkbox" id="user_edit_roles_2" name="user_edit[roles][]" class="custom-control-input" value="ROLE_USER" checked="checked">
  <label class="custom-control-label" for="user_edit_roles_2">Utilisateur</label>
</div>

Bootstrap 3 doc:

  <div class="checkbox">
    <label>
      <input type="checkbox"> Check me out
    </label>
  </div>

In bootstrap 3, we were forced to render the label with the input inside. It's not always the case, but a checkbox item without label is not really revelant, that's why it's kept like it I think.

And check in the code: https://github.com/symfony/symfony/blame/master/src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_4_layout.html.twig#L288 That's it

[ThomasLandauer]

Thanks, guys. To me, this is the most important fact to explain. Since: When switching from another theme to Bootstrap, you might have some form_errors() sitting around in your templates, and you need to manually remove them. So I'd suggest another heading before this: https://symfony.com/doc/4.4/form/bootstrap4.html#accessibility - plus a short hint and link somewhere at https://symfony.com/doc/4.4/form/form_customization.html

Or wait a minute. Is there maybe a way to "resolve" this problem in the code? Since form_label() already includes form_errors(), is there a way to "disable" form_errors() completely? (i.e. make it output nothing). Have you thought about this?

[mpiot]

Or wait a minute. Is there maybe a way to "resolve" this problem in the code? Since form_label() already includes form_errors(), is there a way to "disable" form_errors() completely? (i.e. make it output nothing). Have you thought about this?

In my case I'm not for it, just because in some case you can want to display it in an other place too. Not really common, but it can be usefull, then you call many time the form_errors().

[ThomasLandauer]

Yeah, sure! But switching form themes should just affect the design, shouldn't it? Or is this just my illusion, and there's always some manual tweaking necessary? Do you know if the other form themes have "twists" like this?

The current behavior breaks https://symfony.com/doc/4.4/form/form_customization.html#form-label-form-view-label-variables so it should be explained there.

[mpiot]

But switching form themes should just affect the design, shouldn't it?

If you use form_row(form.field) or form_widget(form), you normally have no trouble. When using a theme you just want render the row eventually to place it in a column for example. But you don't split the rendering: label, widget, error, help message. Sometimes I do that because I want to have a custom rendering, but if I switch the theme, I'm forced to edit it (no same class, div arround).

Do you know if the other form themes have "twists" like this?

I'm not sure, but if I remember correctly it's just Bootstrap 4 that render the errors in label

The current behavior breaks https://symfony.com/doc/4.4/form/form_customization.html#form-label-form-view-label-variables so it should be explained there.

Sorry, I don't understand :/ When call this, the rendering of the label is ok, you also can pass the content if you want override, and can add attributes too.

[ThomasLandauer]

Yeah, that's a really good point! Whenever you move away from form_row(), you cannot switch themes easily anyway ;-)

About https://symfony.com/doc/4.4/form/form_customization.html#form-label-form-view-label-variables : I'll just add a note that it doesn't render the label alone (as the title suggests). See https://github.com/symfony/symfony-docs/pull/13650

[carsonbot]

Thank you for this issue. There has not been a lot of activity here for a while. Has this been resolved?

[carsonbot]

Friendly reminder that this issue exists. If I don't hear anything I'll close this.

[carsonbot]

Hey,

I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen!