[WebProfilerBundle] UI tweaks for the command profiler #52138
Merged
+164
−26
Conversation
This file contains hidden or 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
stof
reviewed
Oct 18, 2023
src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/profiler.css.twig
Show resolved
Hide resolved
src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/profiler.css.twig
Outdated
Show resolved
Hide resolved
src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/profiler.css.twig
Outdated
Show resolved
Hide resolved
HeahDude
reviewed
Oct 18, 2023
| {% if status_code == 0 or interrupted %} | ||
| <span class="status-response-status-code">{{ interrupted is null ? 'Success' : 'Interrupted by signal: ' ~ interrupted }}</span> | ||
| {% if status_code == 0 %} | ||
| {% if interrupted %} |
There was a problem hiding this comment.
Command can be interrupted with an exit code != 0.
There was a problem hiding this comment.
I've updated the logic. I hope I got it right this time. Thanks.
There was a problem hiding this comment.
It may be worth adding the exit code when interrupted. WDYT?
There was a problem hiding this comment.
Yes, let's do it. This is how it looks:
src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/header.html.twig
Show resolved
Hide resolved
HeahDude
approved these changes
Oct 18, 2023
fabpot
approved these changes
Oct 19, 2023
|
Thank you @javiereguiluz. |
fabpot
force-pushed
the
update_cli_profiler
branch
from
3ad1e42 to
041480f
Compare
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.
This PR proposes some minor UI tweaks for the amazing feature contributed by @HeahDude in #47416.
First change
I propose to not display the
HTTP / Commandstoggle in the header of all pages. I don't think this should be an option with quick permanent access from all profiler pages.My proposal is to move the toggle to the search sidebar:
And here in action:
Second change
In my opinion, the current header of command profiles looks too similar to HTTP profiles:
Before / Light
Before / Dark
I propose to differentiate them a bit more and use the well-known "fake terminals" used on Symfony website and docs:
After / Light
After / Dark
The "fake terminals" look different depending on your OS. See this image (from top to bottom: macOS, Windows, Linux)