[Form] Fix the money form type render with Bootstrap3 #19426
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -21,8 +21,8 @@ | |||
|
|
|||
| {% block money_widget -%} | |||
| <div class="input-group"> | |||
| {% set prepend = '{{' == money_pattern[0:2] %} | |||
| {% if not prepend %} | |||
| {% set prepend = '{{' != money_pattern[0:2] %} | |||
There was a problem hiding this comment.
I love the readability of the starts with Twig operator. If you agree, you could change this code as follows:
{% block money_widget -%}
<div class="input-group">
{% set append = money_pattern starts with '{{' %}
{% if not append %}
<span class="input-group-addon">{{ money_pattern|replace({ '{{ widget }}':''}) }}</span>
{% endif %}
{{- block('form_widget_simple') -}}
{% if append %}
<span class="input-group-addon">{{ money_pattern|replace({ '{{ widget }}':''}) }}</span>
{% endif %}
</div>
{%- endblock money_widget %}There was a problem hiding this comment.
I love too, i didn't think about it.
@javiereguiluz: I've searched long time ago the not starts with feature but seems impossible right ?
There was a problem hiding this comment.
no it is not. It needs to be not money_pattern starts with '...'
There is a confusion between the variable naming, and the result expected.
When prepend variable is false, the currency symbol must be add after the widget.
When the money_patternstarts with {{, prepend variable must be false.
|
👍 Status: reviewed |
|
Thank you @Th3Mouk. |
nicolas-grekas
added a commit
that referenced
this pull request
Jul 25, 2016
…Mouk) This PR was merged into the 2.7 branch. Discussion ---------- [Form] Fix the money form type render with Bootstrap3 | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | Part fixing #19424 | License | MIT | Doc PR | none There is a confusion between the variable naming, and the result expected. When prepend variable is false, the currency symbol must be add after the widget. When the `money_pattern`starts with `{{`, `prepend` variable must be `false`. Commits ------- 637a441 Fix the money form type render with Bootstrap3
|
Hum looking at the issue and the time of submission, I may have merged this one to quickly? Is it OK or should we revert? |
|
@nicolas-grekas no it's OK, the other part of the problem "doesn't exist". The part of code that i was talking isn't called when |
This was referenced Jul 30, 2016
Merged
Merged
Merged
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There is a confusion between the variable naming, and the result expected.
When prepend variable is false, the currency symbol must be add after the widget.
When the
money_patternstarts with{{,prependvariable must befalse.