8000 [TwigBridge] do not translate null placeholders or titles by xabbuh · Pull Request #38595 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[TwigBridge] do not translate null placeholders or titles #38595

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 21, 2020
Merged

[TwigBridge] do not translate null placeholders or titles #38595

merged 1 commit into from
Oct 21, 2020

Conversation

xabbuh
Copy link
Member
@xabbuh xabbuh commented Oct 16, 2020
Q A
Branch? 3.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #37573
License MIT
Doc PR

@xabbuh xabbuh added this to the 3.4 milestone Oct 16, 2020
@xabbuh xabbuh mentioned this pull request Oct 16, 2020
Closed
@Nyholm
Copy link
Member
Nyholm commented Oct 16, 2020

Thank you.

Im thinking of the label. Currently, if the label attribute is null, we are trying to create/invent a label looking at the ID. It feels weird that the behavior for placeholder and title is not the same.

But I also agree that we should translate null values, because that will cause an error =)

I am 👍 but I want to hear your thoughts on this ^^

@xabbuh
Copy link
Member Author
xabbuh commented Oct 16, 2020

Most of the time we can derive useful labels from the field names if it isn't configured. For placeholders and titles that not really possible. I mainly think that using null for both of them does not make much sense but I can also see the use case described in #37573 and I agree with the reasoning made there that we should not fail hard over it.

@fabpot
Copy link
Member
fabpot commented Oct 21, 2020

Thank you @xabbuh.

@fabpot fabpot merged commit 1eead3f into symfony:3.4 Oct 21, 2020
@xabbuh xabbuh deleted the issue-37573 branch October 21, 2020 10:44
This was referenced Oct 28, 2020
Merged
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

Assignees
No one assigned
Labels
Bug Status: Reviewed TwigBridge
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.
5 participants
@xabbuh @Nyholm @fabpot @nicolas-grekas @carsonbot
2A62
0