Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Display a better error design when the toolbar cannot be displayed#23274
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
stof commentedJun 23, 2017
shouldn't it use the |
| .sfErrorToolbar .sf-toolbar-icon { float: left; padding: 5px 0; margin-right: 10px; }\ | ||
| </style>\ | ||
| <div class="sfErrorToolbar">\ | ||
| <div class="sf-toolbar-icon"><svg width="26" height="28" xmlns="http://www.w3.org/2000/svg" version="1.1" x="0px" y="0px" viewBox="0 0 26 28" enable-background="new 0 0 26 28" xml:space="preserve"><path fill="#FFFFFF" d="M13 0C5.8 0 0 5.8 0 13c0 7.2 5.8 13 13 13c7.2 0 13-5.8 13-13C26 5.8 20.2 0 13 0z M20 7.5 c-0.6 0-1-0.3-1-0.9c0-0.2 0-0.4 0.2-0.6c0.1-0.3 0.2-0.3 0.2-0.4c0-0.3-0.5-0.4-0.7-0.4c-2 0.1-2.5 2.7-2.9 4.8l-0.2 1.1 c1.1 0.2 1.9 0 2.4-0.3c0.6-0.4-0.2-0.8-0.1-1.3C18 9.2 18.4 9 18.7 8.9c0.5 0 0.8 0.5 0.8 1c0 0.8-1.1 2-3.3 1.9 c-0.3 0-0.5 0-0.7-0.1L15 14.1c-0.4 1.7-0.9 4.1-2.6 6.2c-1.5 1.8-3.1 2.1-3.8 2.1c-1.3 0-2.1-0.6-2.2-1.6c0-0.9 0.8-1.4 1.3-1.4 c0.7 0 1.2 0.5 1.2 1.1c0 0.5-0.2 0.6-0.4 0.7c-0.1 0.1-0.3 0.2-0.3 0.4c0 0.1 0.1 0.3 0.4 0.3c0.5 0 0.9-0.3 1.2-0.5 c1.3-1 1.7-2.9 2.4-6.2l0.1-0.8c0.2-1.1 0.5-2.3 0.8-3.5c-0.9-0.7-1.4-1.5-2.6-1.8c-0.8-0.2-1.3 0-1.7 0.4C8.4 10 8.6 10.7 9 11.1 l0.7 0.7c0.8 0.9 1.3 1.7 1.1 2.7c-0.3 1.6-2.1 2.8-4.3 2.1c-1.9-0.6-2.2-1.9-2-2.7c0.2-0.6 0.7-0.8 1.2-0.6 c0.5 0.2 0.7 0.8 0.6 1.3c0 0.1 0 0.1-0.1 0.3C6 15 5.9 15.2 5.9 15.3c-0.1 0.4 0.4 0.7 0.8 0.8c0.8 0.3 1.7-0.2 1.9-0.9 c0.2-0.6-0.2-1.1-0.4-1.2l-0.8-0.9c-0.4-0.4-1.2-1.5-0.8-2.8c0.2-0.5 0.5-1 0.9-1.4c1-0.7 2-0.8 3-0.6c1.3 0.4 1.9 1.2 2.8 1.9 c0.5-1.3 1.1-2.6 2-3.8c0.9-1 2-1.7 3.3-1.8C20 4.8 21 5.4 21 6.3C21 6.7 20.8 7.5 20 7.5z"/></svg></div>\ |
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.
could the SVG icon be maintained separately, like other icons we have ?
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.
Well, I thought that but in 2.7 branch there isn't icons maintained separately yet, andin 2.8 there is already one, so I guess this could be refactored when this is merged in 2.8?
yceruto commentedJun 23, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I think makes sense use the current elements to avoid empty div, respect the configured toolbar |
yceruto commentedJun 23, 2017
Fixed position when the toolbar is configured to the Ready! |
| } | ||
| /***** Error Toolbar *****/ | ||
| .sfErrorToolbar { |
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.
can't you apply thesf-toolbarreset class on your error toolbar too ? This would avoid duplicating many things (your error toolbar is missing the resetting of box-sizing for instance, and a few other things).
This would also handle the positioning automatically.
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.
btw, not having thesf-toolbar class means you also need to handle the case of the error toolbar separately in the print version.
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.
Indeed!
yceruto commentedJun 23, 2017
Well, after several adjustments the result remains the same (tested until version 3.4) :) @stof thanks! any other suggestions? |
yceruto commentedJun 23, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
/***** Media query print: Do not print the Toolbar. *****/@media print { .sf-toolbar {display: none;visibility: hidden; }} Btw,this style is present in 2.7 butwas removed since 2.8 so the toolbar is displayed in the print version, maybe@javiereguiluz could validate those changes? |
javiereguiluz commentedJun 25, 2017
@yceruto I don't remember this change ... but thinking about this now, in my opinion is OK to print the debug toolbar ... if you want to print your web app, you probably display it in prod. Being able to print the debug toolbar can be interesting in some scenarios (think about reports, audits, etc. to show the performance, number of database queries, etc.) |
yceruto commentedJun 26, 2017
@javiereguiluz I agree with you. We're fine then, this PR is ready. |
ogizanagi commentedJun 26, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I don't agree about printing the debug toolbar. In most cases it does not make sense to me, and for the few case you'll need it, you can still create a new css rule or remove it using browser inspections. I think it was simply forgottenwhen redesigning the profiler |
yceruto commentedJun 26, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@ogizanagi I agree with you too, I've opened an issue to discuss about it#23303 as this issue starts since 2.8 (no related to this PR) ;) |
fabpot commentedJul 3, 2017
Thank you@yceruto. |
…isplayed (yceruto)This PR was squashed before being merged into the 2.7 branch (closes#23274).Discussion----------Display a better error design when the toolbar cannot be displayed| Q | A| ------------- | ---| Branch? | 2.7| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes (failure unrelated)| Fixed tickets |#23266| License | MIT| Doc PR | n/aFixing the left position of the bar (tested in app without style) and escaping the literal newline (ES5) as some IDEs fails with previous syntax (and Github diff too). Btw, I have added the Symfony icon to make clear that this message comes from Symfony ;)**Before:****After:**Commits-------ed3e403 Display a better error design when the toolbar cannot be displayed
Uh oh!
There was an error while loading.Please reload this page.
Fixing the left position of the bar (tested in app without style) and escaping the literal newline (ES5) as some IDEs fails with previous syntax (and Github diff too). Btw, I have added the Symfony icon to make clear that this message comes from Symfony ;)
Before:
After:
