Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Form] Separate errors and display errors #9994

Closed
webmozart opened this issue Jan 10, 2014 · 19 comments
Closed

[Form] Separate errors and display errors #9994

webmozart opened this issue Jan 10, 2014 · 19 comments

Comments

@webmozart
Copy link
Contributor

I would like to separate errors and "display errors" in the form component. Therefore I would like to add two methods to FormInterface:

public function getDisplayErrors($deep = false, $flatten = true);

public function addDisplayError(FormError $error);

The idea is that getDisplayErrors() should return the errors that should be displayed on a given form, while getErrors() should only return the errors that were actually caused by the field. In this way, it will be possible to distinguish really erroneous fields from those that only display errors of other fields (#6001).

To make that work, I would like to deprecate the "error_bubbling" option and introduce an equivalent option for the addDisplayError() method. The option should control the following:

  • either errors are displayed at the field they are created on (the default)
  • or they are displayed on the containing form

What should we call this option? "display_errors"?

$builder->add('field', 'type', array(
    // show errors on the field
    'display_errors' => true,
    // show errors on the form
    'display_errors' => false,
));

Unfortuantely we cannot continue to use "error_bubbling", because then we need to break BC.

@willdurand
Copy link
Contributor

"errors_level":

$builder->add('field', 'type', array(
    // show errors on the field
    'errors_level' => 'field',
    // show errors on the form
    'errors_level' => 'form',
));

"inline_errors":

$builder->add('field', 'type', array(
    // show errors on the field
    'inline_errors' => true,
    // show errors on the form
    'inline_errors' => false,
));

@stof
Copy link
Member

stof commented Jan 10, 2014

field vs form is weird IMO, as everything is a Form in Symfony. It should be root to talk about the root form to be consistent

@webmozart
Copy link
Contributor Author

Which leads me to an alternative proposal:

$builder->add('field', 'type', array(
    // show errors on the field
    'display_errors' => 'self',
    // show errors on the parent form
    'display_errors' => 'parent',
    // show errors on the root form
    'display_errors' => 'root',
));

@willdurand
Copy link
Contributor

display_errors makes me think about the PHP setting, and so I would except to set either true or false. It is more a matter of "level" IMHO.

@stof
Copy link
Member

stof commented Jan 10, 2014

maybe error_location or error_position ?

@willdurand
Copy link
Contributor

@stof what's the difference between your two proposals?

@stof
Copy link
Member

stof commented Jan 10, 2014

sorry, edited it to actually give the 2 proposal I thought about

@nicolopignatelli
Copy link

what about display_errors_on or just errors_on with self, parent and root as options as proposed by @bschussek ?

Alternatively, error_location sounds clear enough for me, as well.

@Tobion
Copy link
Member

Tobion commented Jan 10, 2014

So when you would want to show errors two levels higher in hierarchy, you would set error_location of the field to parent and in the parent form you set the option to parent as well. Right?

I think the new option with parent|self|root makes more sense than the old error_bubbling because it is more user-centric whereas error_bubbling was just technical.

@webda2l
Copy link

webda2l commented Jan 10, 2014

+1 for second Bernhard proposition #9994 (comment)
And errors_scope as other possible alternative

@stanlemon
Copy link

This would be a great change! I like error_scope too fwiw.

@emgiezet
Copy link

👍 for the parent|self|root

@raziel057
Copy link
Contributor

+1 for errors_scope or display_errors_scope with possible values self | parent | root.

@mdavis1982
Copy link
Contributor

@webmozart Have there been any updates on this?

@SofHad
Copy link
Contributor

SofHad commented Jan 31, 2015

👍 for error_location with self | parent | root

@mfrieswyk
Copy link

@webmozart status bump? Is this issue still relevant or stale?

@ThomasLandauer
Copy link
Contributor

If the value would be an array, you could choose multiple error locations:

error_location => ['self','parent']

See #36729

@carsonbot
Copy link

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

@xabbuh
Copy link
Member

xabbuh commented Feb 21, 2021

Let's close this old issue as nobody cared enough to implement this feature.

@xabbuh xabbuh closed this as completed Feb 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests