Skip to content
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

[Form] Add label_html attribute #31375

Open
wants to merge 3 commits into
base: master
from

Conversation

@przemyslaw-bogusz
Copy link
Contributor

przemyslaw-bogusz commented May 3, 2019

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
License MIT

I propose to add a new attribute to BaseType class so it is easy to include html tags in labels for, both, buttons and other elements that inherit from FormType class. This gives you an ability to add, e.g. a glyphicon to a button, or a link to a checkbox, simply inside the FormBuilder, 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 #31358. If necessary, I can close my PR and @alexander-schranz can include my suggestions inside his PR.
  2. I have just read in #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

This comment has been minimized.

Copy link
Contributor

alexander-schranz commented May 9, 2019

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

This comment has been minimized.

Copy link
Contributor

alexander-schranz commented May 13, 2019

@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

This comment has been minimized.

Copy link

AndreasA commented May 28, 2019

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

@przemyslaw-bogusz

This comment has been minimized.

Copy link
Contributor Author

przemyslaw-bogusz commented May 28, 2019

@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

This comment has been minimized.

Copy link

AndreasA commented May 28, 2019

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

@przemyslaw-bogusz przemyslaw-bogusz force-pushed the przemyslaw-bogusz:label_html branch from d92e2a8 to 05a1c39 May 29, 2019
@alexander-schranz

This comment has been minimized.

Copy link
Contributor

alexander-schranz commented Jun 12, 2019

@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 przemyslaw-bogusz force-pushed the przemyslaw-bogusz:label_html branch from 05a1c39 to 5a50ee8 Jun 16, 2019
@przemyslaw-bogusz

This comment has been minimized.

Copy link
Contributor Author

przemyslaw-bogusz commented Jun 17, 2019

I've tried a different approach by implementing this feature in FormType instead of BaseType, 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?

@przemyslaw-bogusz przemyslaw-bogusz changed the base branch from master to 4.4 Jun 24, 2019
@przemyslaw-bogusz przemyslaw-bogusz changed the base branch from 4.4 to master Jun 25, 2019
@przemyslaw-bogusz przemyslaw-bogusz force-pushed the przemyslaw-bogusz:label_html branch from a23d9eb to fc0ee8f Jun 25, 2019
@przemyslaw-bogusz przemyslaw-bogusz requested a review from xabbuh as a code owner Jun 25, 2019
@przemyslaw-bogusz przemyslaw-bogusz force-pushed the przemyslaw-bogusz:label_html branch from fc0ee8f to 46f625f Jun 25, 2019
@przemyslaw-bogusz przemyslaw-bogusz force-pushed the przemyslaw-bogusz:label_html branch from 2617c46 to b4c0924 Feb 9, 2020
@przemyslaw-bogusz

This comment has been minimized.

Copy link
Contributor Author

przemyslaw-bogusz commented Feb 10, 2020

@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

This comment has been minimized.

Copy link
Member

xabbuh commented Feb 14, 2020

@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's composer.json file.

require-dev:

"symfony/form": "^5.1"

conflict:

"symfony/form": "<5.1"
@przemyslaw-bogusz

This comment has been minimized.

Copy link
Contributor Author

przemyslaw-bogusz commented Feb 14, 2020

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

@fabpot

This comment has been minimized.

Copy link
Member

fabpot commented Feb 15, 2020

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

@nicolas-grekas nicolas-grekas force-pushed the przemyslaw-bogusz:label_html branch from 3fb25c0 to 7037723 Feb 15, 2020
Copy link
Member

nicolas-grekas left a comment

(I rebased + squashed)

Copy link
Member

HeahDude left a comment

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

@przemyslaw-bogusz

This comment has been minimized.

Copy link
Contributor Author

przemyslaw-bogusz commented Feb 15, 2020

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

@@ -11,7 +11,8 @@ CHANGELOG
is deprecated. The method will be added to the interface in 6.0.
* Implementing the `FormConfigBuilderInterface` without implementing the `setIsEmptyCallback()` method
is deprecated. The method will be added to the interface in 6.0.

* Added `label_html` option to display the `label` text as HTML.

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Feb 15, 2020

Member

extra space on this line :)

This comment has been minimized.

Copy link
@przemyslaw-bogusz

przemyslaw-bogusz Feb 16, 2020

Author Contributor

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants
You can’t perform that action at this time.