Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[WebProfilerBundle] Add channel log filter#28934
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
ro0NL left a comment
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.
all good :D and a tiny step closer to sanity.
| --page-background: #f9f9f9; | ||
| --color-text: #222; | ||
| --color-muted: #777; | ||
| --color-muted: #999; |
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.
this var was overwritten below
| --table-header: #e0e0e0; | ||
| --shadow: 0px 0px 1px rgba(128, 128, 128, .2); | ||
| --border: 1px solid #e0e0e0; | ||
| --color-muted: #999; |
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.
that's here :)
| background-color: var(--base-5); | ||
| color: var(--base-2); | ||
| } | ||
| .tab-content { |
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.
not needed, now specified by[data-filters] (the container that requires it)
| display: block; | ||
| font-size: 12px; | ||
| } | ||
| table.logs tr td:last-child { width: 100%; } |
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.
already handled by.full-width on the message TH
fabpot left a comment
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.
javiereguiluz commentedOct 23, 2018
Great work Roland! I've tested this in the Symfony Demo app. I only have minor remarks. First: "Ghost filters": when loading the page, filters are displayed by default and they disappear when they shouldn't be displayed. This GIF shows a force reload in the page:
Maybe it's better to "hide by default" and display when appropriate? Second: "Moving filters": Right now, the multiple choice filters "move" under some circumstances. See hwen I deselect all of channels and icons disappear:
This kind of movements are distracting and user may click wrongly because of these element displacements. What if we replace the showing/hiding icon by a form checkbox? It doesn't have to be a real form, just a clickable checkbox. Stripe does something similar:
|
ro0NL commentedOct 23, 2018
I think i've addressed both issues.
|
fabpot commentedOct 24, 2018
Thank you@ro0NL. |
This PR was squashed before being merged into the 4.2-dev branch (closes#28934).Discussion----------[WebProfilerBundle] Add channel log filter| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no <!-- seehttps://symfony.com/bc -->| Deprecations? | no| Tests pass? | yes <!-- please add some, will be required by reviewers -->| Fixed tickets | #... <!-- #-prefixed issue number(s), if any -->| License | MIT| Doc PR | symfony/symfony-docs#... <!-- required for new features -->Continuation of#28906The JS is revised to be more generic;- support 2 filter types: `level` and `choice` (respectively `Log level` and `Log channel` here)- remove default filter value support (not used yet, but opportunity kept open) - it requires a bit more work to genericify it.- filters refines the resultset (e.g. show all logs in the app channel with priority higher than alert)Level filter (works the same as shown in#28906 )Choice filterWe forgot to update TwigBundle previously, that's still needed after review here.Commits-------e1bd82e [WebProfilerBundle] Add channel log filter
Uh oh!
There was an error while loading.Please reload this page.
Continuation of#28906
The JS is revised to be more generic;
levelandchoice(respectivelyLog levelandLog channelhere)Level filter (works the same as shown in#28906 )
Choice filter
We forgot to update TwigBundle previously, that's still needed after review here.