symfony/symfony-docsPublic

NotificationsYou must be signed in to change notification settings
Fork5.3k
Star2.3k

Changed option guessing to maxlength#7273

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Closed

caillioux wants to merge3 commits intosymfony:3.2fromcaillioux:patch-4

Closed

Changed option guessing to maxlength#7273

caillioux wants to merge3 commits intosymfony:3.2fromcaillioux:patch-4

Conversation

Copy link

Contributor

caillioux commentedDec 18, 2016

HTML attribute name is 'maxlength' instead of 'max_length'.

Changed option guessing to maxlength

1f4bc71

HTML attribute for max length is ``maxlength``.

carsonbot added the Status: Needs Review label

Dec 18, 2016

HeahDude suggested changes

Dec 18, 2016

View reviewed changes

Copy link

Contributor

HeahDude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This can be merged in 2.7 👍

forms.rst


		``max_length``
		If the field is some sort of text field, then the ``max_length`` option can be
		``maxlength``

Copy link

Contributor

HeahDudeDec 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks for opening this PR! This change is legit, but I think you should also replaceoption byhtml attribute for both this one andrequired above. since this does not use the options at all, but theattr option, btwmax_length option has been deprecated in 2.5.

caillioux added2 commits

December 18, 2016 21:20

Replaced "option" word with "html attribute"

b0113fc

Changed section title

52abda1

Copy link

ContributorAuthor

caillioux commentedDec 18, 2016

Thanks for the comment@HeahDude
It was quite unclear that the form optionmax_length before 2.5 became an html attributemaxlength after then.
required option still exists, but its scope seems limited to add arequired attribute to.
Then, calling the sectionForm Types Options Guessing does not make sens anymore to me, your opinion?

HeahDude reviewed

Dec 18, 2016

View reviewed changes

forms.rst

		single: Forms; Field type guessing

		FieldType Options Guessing
		FieldAttributes Guessing

Copy link

Contributor

HeahDudeDec 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I understand why you did it but this change is not needed since guessers are about to change type's options, actuallyrequired andmaxlength are define thanks to theattr option in the form factory but the registered doctrine and validation guessers are setting some options too, maybe it's worth documenting them though. I don't know what others think?