-
-
Notifications
You must be signed in to change notification settings - Fork 756
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
changed email fields with the email type and added required parameter on... #1180
Conversation
->add('name', 'text') | ||
->add('email', 'text') | ||
->add('name', 'text', array('required' => true)) | ||
->add('email', 'email') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The required
isn't needed because, since the form is mapped to the User entity, it will apply annotation from the entity to the form. In that case, it will add these ones for the email:
* @Assert\Email()
* @Assert\NotBlank()
So, it will be required in some way 😃
And about the name, why should it be required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The required isn't needed because, since the form is mapped to the User entity, it will apply annotation from the entity to the form.
It's just more pratical to use with mobile keyboards when entering your email (@ shows up). Nothing to do with server checking. Right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And about the name, why should it be required?
My bad.
As far as I can remember, I used |
@@ -20,6 +20,7 @@ public function buildForm(FormBuilderInterface $builder, array $options) | |||
{ | |||
$builder | |||
->add('email', 'email', array( | |||
'required' => true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already declare constraints as new Constraints\NotBlank(),
so this is not required to extra add the required
attribute.
I was wondering if you got any problems while testing the v2 and then feel the need to add this requirement. |
I just like my browser starting to tell me the informations I entered aren't correct before sending the page. Of course it doesn't replace server-side checking which stays mandatory but I appreciate it. |
... some stuff
Just tiny changes.