Skip to content

[Form] Fixed ChoiceType trim option #9598

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

Merged
merged 5 commits into from
May 8, 2018
Merged

Conversation

HeahDude
Copy link
Contributor

@HeahDude HeahDude changed the title Fixed ChoiceType trim option [Form] Fixed ChoiceType trim option Apr 15, 2018
fabpot added a commit to symfony/symfony that referenced this pull request Apr 16, 2018
This PR was merged into the 2.7 branch.

Discussion
----------

[Form] Fixed trimming choice values

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #24247, #24712
| License       | MIT
| Doc PR        | symfony/symfony-docs#9598

Follows #24712 discussion.

Commits
-------

00cdf5e [Form] Fixed trimming choice values
@HeahDude HeahDude removed the On hold label Apr 16, 2018
Copy link
Member

@javiereguiluz javiereguiluz left a comment

Choose a reason for hiding this comment

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

Nice! Thanks Jules.

@HeahDude
Copy link
Contributor Author

HeahDude commented Apr 18, 2018

Thank you for the rewording 👍

@@ -323,6 +324,14 @@ error_bubbling
Set that error on this field must be attached to the field instead of
the parent field (the form in most cases).

trim
~~~~
Copy link
Member

Choose a reason for hiding this comment

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

I think we should move the description to its own file to be able to reuse it in other form types. This is currently missing in some documents (see the failing build).

@wouterj
Copy link
Member

wouterj commented May 5, 2018

Ping @HeahDude can you please fix this PR according @xabbuh's comment?

@@ -30,6 +30,7 @@ To use this field, you must specify *either* ``choices`` or ``choice_loader`` op
| Overridden | - `compound`_ |
| options | - `empty_data`_ |
| | - `error_bubbling`_ |
| | - `choice_type_trim`_ |
Copy link
Member

Choose a reason for hiding this comment

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

here we would still have to use trim

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was not sure ^^. Fixed, thanks!

**type**: ``boolean`` **default**: ``false``

Trimming is disabled by default because the selected value or values must match
the given choice values exactly (and they could contain white spaces).
Copy link
Member

Choose a reason for hiding this comment

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

"whitespaces" (without the space)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, fixed. Thanks again!

@wouterj
Copy link
Member

wouterj commented May 8, 2018

Thanks Jules.

@wouterj wouterj merged commit 2dd6c56 into symfony:2.7 May 8, 2018
wouterj added a commit that referenced this pull request May 8, 2018
…uiluz)

This PR was merged into the 2.7 branch.

Discussion
----------

[Form] Fixed ChoiceType trim option

Ref symfony/symfony#26932.

Commits
-------

2dd6c56 fixup typo
360066d fixup option links
6631665 fixup @xabbuh's comment proper choice type trim file
8bfa059 Reword
bbca3b0 [Form] Fixed ChoiceType trim option
@HeahDude HeahDude deleted the choice-trim branch May 15, 2018 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants