NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

Allow TranslatableMessage object in form option 'help'#41717

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

Merged

fabpot merged 1 commit intosymfony:5.4fromplandolt:patch-2

Jul 4, 2021

Merged

Allow TranslatableMessage object in form option 'help'#41717

fabpot merged 1 commit intosymfony:5.4fromplandolt:patch-2

Jul 4, 2021

Conversation

Copy link

Contributor

plandolt commentedJun 15, 2021

Q	A
Branch?	5.4
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	n/a
License	MIT
Doc PR	-

This PR allows the use ofTranslatableMessage (ort(...)) for a help option the same way the label option already supports this.

Copy link

carsonbot commentedJun 15, 2021

Hey!

To help keep things organized, we don't allow "Draft" pull requests. Could you please click the "ready for review" button or close this PR and open a new one when you are done?

Note that a pull request does not have to be "perfect" or "ready for merge" when you first open it. We just want it to be ready for a first review.

Cheers!

Carsonbot

plandolt marked this pull request as ready for review

June 15, 2021 15:28

plandolt requested review fromxabbuh andyceruto ascode owners

June 15, 2021 15:28

carsonbot added Status: Needs Review Feature labels

Jun 15, 2021

OskarStark reviewed

Jun 15, 2021

View reviewed changes

src/Symfony/Component/Form/CHANGELOG.md OutdatedShow resolvedHide resolved

nicolas-grekas added this to the5.4 milestone

Jun 17, 2021

plandolt requested a review fromOskarStark

June 18, 2021 06:30

Copy link

Contributor

OskarStark commentedJun 18, 2021

Can you add a test case that this works?

xabbuh reviewed

Jun 18, 2021

View reviewed changes

src/Symfony/Component/Form/Extension/Core/Type/FormType.phpShow resolvedHide resolved

xabbuh reviewed

Jun 18, 2021

View reviewed changes

src/Symfony/Component/Form/Tests/AbstractLayoutTest.php OutdatedShow resolvedHide resolved

xabbuh reviewed

Jun 18, 2021

View reviewed changes

src/Symfony/Component/Form/Tests/AbstractLayoutTest.php OutdatedShow resolvedHide resolved

plandolt force-pushed thepatch-2 branch from721ffc8 to6d76a3fCompare

June 18, 2021 12:48

plandolt requested a review fromxabbuh

June 18, 2021 12:49

xabbuh approved these changes

Jun 18, 2021

View reviewed changes

Copy link

Member

xabbuh left a comment

Choose a reason for hiding this comment

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

with a minor comment

src/Symfony/Component/Form/CHANGELOG.md OutdatedShow resolvedHide resolved

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Jun 18, 2021

Allow to pass TranslatableMessage objects to the help option

44461f1

plandolt force-pushed thepatch-2 branch fromfb14864 to44461f1Compare

June 18, 2021 15:30

Copy link

ContributorAuthor

plandolt commentedJun 18, 2021

updated the changelog message according to@xabbuh his comment.

BTW: Is it better for you to leave all the commits or squash them regularly and force push this PR?

Copy link

Member

xabbuh commentedJun 21, 2021

@scuben Squashing and rebasing is fine, but when can also squash commits while merging as long the PR doesn't have conflicts with the target branch (otherwise it will need to be rebased).