Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Form][WebProfiler] Empty form names fix#12267
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
phramz commentedOct 20, 2014
won't a |
kix commentedOct 20, 2014
It sure could, but personally I think that it would be less understandable. This fix improves usability a bit, too, IMO. |
javiereguiluz commentedOct 20, 2014
@kix thanks for this PR! I personally love this kind of small fixes that improve the overall quality of the framework. This will lead to less confusion for developers and that's very important. I also agree that showing some explicit message, such as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
you should remove the spaces around the| according to the Twig coding standards.
And I even suggest using{% if name is not empty %} here. It will make the intent easier to understand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@stof 👍
76411e1 to88c5910Comparekix commentedOct 27, 2014
@webmozart Done. Is it better now? |
webmozart commentedNov 3, 2014
Yes! :) Could you add the same fix to the details view on the right? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Why not simply:
{{ name|default('(no name)') }}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
👍
88c5910 to3800402CompareWhen a Form had no name, the markup was broken in the profiler,making the form tree ugly.
3800402 toc121deaComparekix commentedNov 4, 2014
Got it. Right-side detail view is updated in the same manner, too. |
fabpot commentedNov 21, 2014
Thank you@kix. |
This PR was submitted for the master branch but it was merged into the 2.5 branch instead (closes#12267).Discussion----------[Form][WebProfiler] Empty form names fix| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | none| License | MIT| Doc PR | noneWhen a Form had no name, the markup was broken in the profiler, making the form tree ugly. This pull request changes the output so that `(no name)` string is displayed when no form name was available.Before:After:Commits-------6e9642a [Form][WebProfiler] Empty form names fix
When a Form had no name, the markup was broken in the profiler, making the form tree ugly. This pull request changes the output so that
(no name)string is displayed when no form name was available.Before:
After:
