[WebProfilerBundle] Improve cache panel #22129
Conversation
There was a problem hiding this comment.
👍 trying to improve this page was on my TODO list for 3.3 too. @ro0NL thanks for taking care of this.
I agree that this page has too much information. Looking at your screenshot, I'd like to suggest these additional changes:
-
Use the
<span class="unit">ms</span>code to display the "ms" value, as we do in other profiler panels such as "Performance" (and maybe apply it to the "%" too in "hit/misses" ??) -
Rearrange the row of information as follows:
- Total calls
- Total time
<div class="metric-divider"></div>- Total reads
- Total writes
- Total deletes
<div class="metric-divider"></div>- Total Hits
- Total Misses
- Hits/reads
|
Improved the metric handling, moving things to the view layer. Totally worth it :) (screenshot updated). |
| <tr> | ||
| <th>Results</th> | ||
| <td> | ||
| {% if call.result != false %} |
There was a problem hiding this comment.
@Nyholm not sure about a) this check and b) loose comparison. As you can see in the screenshots null was hidden as wel. Does false need to be excluded, strictly?
There was a problem hiding this comment.
Thank you for this PR.
I'm not sure about your questions. Call.result could be false if there was an error (if I remember correctly). I think that is the reason for this check.
Why it not is a strict comparison... that is probably a typo.
There was a problem hiding this comment.
I'll check it out tonight. I'd prefer something like displaying n/a in that case, opposed to just an empty cell.
There was a problem hiding this comment.
@Nyholm figured it out; this is (AFAIK) the direct result value from the method call passed by the traceableadapter.
Now given https://github.com/php-fig/cache/blob/master/src/CacheItemInterface.php#L42 i dont think we should maky any assumptions here.
Besides, this hides false as a result from a hasItem call ;-)
|
Did some overall improvements to the table structure.. i kinda like it. The whole thing looks really calm now and we save a lot on spacing overall. Screenshots updated. Should be reviewed with |
|
Thank you @ro0NL. |
This PR was squashed before being merged into the 3.3-dev branch (closes #22129). Discussion ---------- [WebProfilerBundle] Improve cache panel | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #... <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#... <!--highly recommended for new features--> An attempt to improve the page a bit. Personally i think all elements on a single page is too much info. Before   After   Commits ------- 3592d0d [WebProfilerBundle] Improve cache panel
An attempt to improve the page a bit. Personally i think all elements on a single page is too much info.
Before
After