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][TwigBridge] Add help_html #29861

Merged
merged 1 commit into from Jan 25, 2019

Conversation

Projects
None yet
6 participants
@mpiot
Copy link
Contributor

mpiot commented Jan 12, 2019

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets no
License MIT
Doc PR symfony/symfony-docs#10978

Sometimes, when we use the form help option, we want to display it as HTML (add bold, italic, a span with a specific class, ...). For security reasons, we escape the help content.

In this PR, I've added an help_html option, seted to false per default. When it set on true, the help content is no longer escaped.

@nicolas-grekas nicolas-grekas added this to the next milestone Jan 24, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Jan 24, 2019

Thanks @mpiot, could you please rebase to fix the conflicts? They must be related to the move to short arrays.

@nicolas-grekas nicolas-grekas force-pushed the mpiot:help-html branch from cb692b6 to 33f5f85 Jan 25, 2019

@nicolas-grekas
Copy link
Member

nicolas-grekas left a comment

(I just rebased for you)

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Jan 25, 2019

Thank you @mpiot.

@nicolas-grekas nicolas-grekas merged commit 33f5f85 into symfony:master Jan 25, 2019

0 of 3 checks passed

fabbot.io Some changes should be done to comply with our standards.
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

nicolas-grekas added a commit that referenced this pull request Jan 25, 2019

feature #29861 [Form][TwigBridge] Add help_html (mpiot)
This PR was merged into the 4.3-dev branch.

Discussion
----------

[Form][TwigBridge] Add help_html

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | no
| License       | MIT
| Doc PR        | symfony/symfony-docs#...

Sometimes, when we use the form `help` option, we want to display it as HTML (add bold, italic, a span with a specific class, ...). For security reasons, we escape the `help` content.

In this PR, I've added an `help_html` option, seted to false per default. When it set on true, the `help` content is no longer escaped.

Commits
-------

33f5f85 [Form][TwigBridge] Add help_html option
@mpiot

This comment has been minimized.

Copy link
Contributor Author

mpiot commented Jan 25, 2019

Thanks a lot :-) I was thinking about doing this tonight, it's faster like this :D

@mpiot mpiot deleted the mpiot:help-html branch Jan 25, 2019

javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request Feb 11, 2019

minor #10978 [FormType] Add help_html documentation (mpiot)
This PR was merged into the master branch.

Discussion
----------

[FormType] Add help_html documentation

Add documentation about the new feature symfony/symfony#29861

Commits
-------

cc6a5f3 Add help_html
@scuben

This comment has been minimized.

Copy link
Contributor

scuben commented Feb 20, 2019

I wonder if we should also add a boolean option label_html with a similar functionality like the newly help_html? Would it be appreciated if one would create a PR for that? An old issue (#8733) suggested that but was closed due to lack of traction.

@mpiot

This comment has been minimized.

Copy link
Contributor Author

mpiot commented Feb 20, 2019

@scuben I can do that, (I've hesitate to do that when I've done this PR), but is it really usefull ?
In which case we need to implement HTML in a label ? Often the label is stilized by it class and we don't really need to use HTML tags.

@scuben

This comment has been minimized.

Copy link
Contributor

scuben commented Feb 20, 2019

@mpiot I just had a use case to add a TOS link to the label of a checkbox and would have loved this feature 👍

@mpiot

This comment has been minimized.

Copy link
Contributor Author

mpiot commented Feb 20, 2019

@scuben Okay, I see, then it should be implement in CheckboxType, RadioType, and ChoiceType only ?

@scuben

This comment has been minimized.

Copy link
Contributor

scuben commented Feb 20, 2019

I would implement it in any type which would reduce complexity as you know you can relay on that it's available in any label.

@mpiot

This comment has been minimized.

Copy link
Contributor Author

mpiot commented Feb 20, 2019

It's comprehensive, then it's easier to implement too. I start it, when I've some times :-)

@xabbuh

This comment has been minimized.

Copy link
Member

xabbuh commented Feb 20, 2019

I think it would be better to discuss this in a new issue before investing time in something that might not be merged. Personally, I don't see this as a common use case and would rather suggest to use a custom form theme for cases when you really need this instead.

@mpiot

This comment has been minimized.

Copy link
Contributor Author

mpiot commented Feb 20, 2019

Okay, then @scuben, can you create an Issue ? You can refer it and ping me, if there are a real interest from users, I'll do it.

For the moment, I've must do that one times, and I've used a custom form theme thath extends which theme I use, with a custom type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.