Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Form] Add label_html attribute#31375

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

Merged

Conversation

przemyslaw-bogusz
Copy link
Contributor

@przemyslaw-boguszprzemyslaw-bogusz commentedMay 3, 2019•
edited
Loading

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
LicenseMIT
Doc PRsymfony/symfony-docs#13316

I propose to add a new attribute toBaseType class so it is easy to include html tags in labels for, both, buttons and other elements that inherit fromFormType class. This gives you an ability to add, e.g. a glyphicon to a button, or a link to a checkbox, simply inside theFormBuilder, which means you can just do

{{ form(form) }}

inside a template.

Sidenotes

  1. I have started working on this two days ago and it the meantime@alexander-schranz made a similar proposition in[Form] Add label_raw attribute to form theme #31358. If necessary, I can close my PR and@alexander-schranz can include my suggestions inside his PR.
  2. I have just read in[Form][TwigBridge] Add help_html #29861 that@mpiot wanted to include this idea in his PR. With respect to@xabbuh's comments from that PR, I hope that my PR will be at least a good place to discuss, if the proposed feature is a good solution.

alexander-schranz, AndreasA, DavidBadura, FabianSchmick, and creiner reacted with thumbs up emoji
@alexander-schranz
Copy link
Contributor

alexander-schranz commentedMay 9, 2019•
edited
Loading

As described in#31358 I think it totally make sense to have label_html attribute in it as we also have help_html attribute. 👍 for this.

@alexander-schranz
Copy link
Contributor

@przemyslaw-bogusz the label is also used inside the button example and could contain html there also see:https://github.com/symfony/symfony/pull/31358/files

@AndreasA
Copy link
Contributor

AndreasA commentedMay 28, 2019•
edited
Loading

Hi, just wondering will this make it into Symfony 4.3?

@przemyslaw-bogusz
Copy link
ContributorAuthor

@AndreasA I've been busy preparing for SymfonyLive in Warsaw, but that's on the list of my priorities, so I will fix the tests and complete it as soon as I can. However, it doesn't mean it will be automatically approved. If you like the idea, give it a 'thumbs up' or leave a positive comment.

@AndreasA
Copy link
Contributor

AndreasA commentedMay 28, 2019•
edited
Loading

I think this would be really helpful. Especially in combination with thelabel_translation_parameters it would allow to create terms and condition checkboxes with links in them way easier than it is now.

@alexander-schranz
Copy link
Contributor

@przemyslaw-bogusz the tests seems to fail because $label_html is not define in the form_label.html.php not sure how this happens maybe you can try to debug the tests and try to fix it?

@przemyslaw-bogusz
Copy link
ContributorAuthor

I've tried a different approach by implementing this feature inFormType instead ofBaseType, but in either case tests keep failing with the same result - variable "label_html" does not exist. Nevertheless, it works in real application with different combinations of field types and themes. Maybe@xabbuh or@chr-hertel could shed some light?

EvKoh reacted with thumbs up emoji

@przemyslaw-bogusz
Copy link
ContributorAuthor

@xabbuh I have started working on this some time ago, but I had problem with a single space, that which caused the tests to fail. I have fixed this, but there is still some issue with Travis on PHP 7.4. I could really use some help to solve this out, because there are a few people, who like this proposition -#35598.

@xabbuh
Copy link
Member

@przemyslaw-bogusz I think making sure that the Twig bridge always gets the Form component in version 5.1 or higher should solve it. We would need to make some changes to the Twid bridge'scomposer.json file.

require-dev:

"symfony/form": "^5.1"

conflict:

"symfony/form": "<5.1"

@przemyslaw-bogusz
Copy link
ContributorAuthor

@xabbuh Thanks a lot. That solved the issue. However, now there is a single test in Cache Component failing.

@fabpot
Copy link
Member

It looks like this PR contains not related commits. Can you rebase and force push?

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

(I rebased + squashed)

Copy link
Contributor

@HeahDudeHeahDude left a comment

Choose a reason for hiding this comment

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

Thanks! We also need a note in the form component CHANGELOG.

@przemyslaw-bogusz
Copy link
ContributorAuthor

I have updated the form component changelog. Sorry for the mess with the commits and thanks for the cleanup.

@przemyslaw-boguszprzemyslaw-bogusz changed the title[Form] Add label_html attribute[Form] Add label_html attribute testFeb 18, 2020
@przemyslaw-boguszprzemyslaw-bogusz changed the title[Form] Add label_html attribute test[Form] Add label_html attributeFeb 18, 2020
@przemyslaw-bogusz
Copy link
ContributorAuthor

I removed the CHANGELOG update, and without AppVeyor CI is green again.

Copy link
Member

@javiereguiluzjaviereguiluz left a comment

Choose a reason for hiding this comment

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

Nice and useful feature!

@nicolas-grekas
Copy link
Member

Thank you@przemyslaw-bogusz.

@nicolas-grekasnicolas-grekas merged commit49e5d57 intosymfony:masterMar 12, 2020
@alexander-schranz
Copy link
Contributor

@przemyslaw-bogusz good work nice to see this merged!

@przemyslaw-bogusz
Copy link
ContributorAuthor

@alexander-schranz Thanks! I wish you all the best in future PRs.

@przemyslaw-boguszprzemyslaw-bogusz deleted the label_html branchMarch 13, 2020 09:04
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull requestMar 14, 2020
…bogusz)This PR was squashed before being merged into the master branch (closes#13316).Discussion----------[Form] Add description of label_html option| Q             | A| ------------- | ---| Feature PR      |symfony/symfony#31375| PR author(s)      |@przemyslaw-bogusz| Merged in  | WCMCommits-------7a74bb3 [Form] Add description of label_html option
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull requestMar 24, 2020
…rzemyslaw-bogusz)This PR was squashed before being merged into the master branch (closes#13409).Discussion----------[Validator] Add description for AtLeastOne constraint| Q             | A| ------------- | ---| Feature PR      |symfony/symfony#31375| PR author(s)      |@przemyslaw-bogusz| Merged in  | 5.1-dev| Fixes |#13355Commits-------5ba5224 [Validator] Add description for AtLeastOne constraint
@nicolas-grekasnicolas-grekas modified the milestones:next,5.1May 4, 2020
@fabpotfabpot mentioned this pull requestMay 5, 2020
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@javiereguiluzjaviereguiluzjaviereguiluz approved these changes

@HeahDudeHeahDudeHeahDude approved these changes

@xabbuhxabbuhAwaiting requested review from xabbuh

Assignees
No one assigned
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.

9 participants
@przemyslaw-bogusz@alexander-schranz@AndreasA@xabbuh@fabpot@nicolas-grekas@javiereguiluz@HeahDude@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp