-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Limit the max height/width of icons in the profiler menu #17330
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
javiereguiluz
commented
Jan 11, 2016
Q | A |
---|---|
Bug fix? | yes |
New feature? | no |
BC breaks? | no |
Deprecations? | no |
Tests pass? | yes |
Fixed tickets | #17329 |
License | MIT |
Doc PR | - |
Thank you @javiereguiluz You are awesome. I can confirm that this resolves the issue on the profiler page. This does not however address the issue in the toolbar. The "q." is outside the of the box. See attached screen shots |
Regarding the pending bug, is your HTML code similar to this? <div class="sf-toolbar-block">
<a href="...">
<div class="sf-toolbar-icon">
<svg ...></svg>
<span class="sf-toolbar-value">1</span>
<span class="sf-toolbar-label">req</span>
</div>
</a>
<div class="sf-toolbar-info">
...
</div>
</div> Because I cannot reproduce the bug: |
Yes, my HTML was similar to that. With HTML copied from the documentation: {% extends '@WebProfiler/Profiler/layout.html.twig' %}
{% import _self as macro %}
{% block toolbar %}
{% if collector.totalRequests > 0 %}
{% set icon %}
{{ include('@Httplug/Icon/httplug.svg') }}
<span class="sf-toolbar-status">Request</span>
{% endset %}
{% include 'WebProfilerBundle:Profiler:toolbar_item.html.twig' with { link: false } %}
{% endif %}
{% endblock %} |
I think I made some progress. I removed my bundle from the equation and started with the twig icon. The twig.svg file looks like this: <svg version="1.1" xmlns="http://www.w3.org/2000/svg" x="0px" y="0px" height="24" viewBox="0 0 24 24" enable-background="new 0 0 24 24" xml:space="preserve">
<path fill="#AAAAAA" d="M20.1,1H3.9C2.3,1,1,2.3,1,3.9v16.3C1,21.7,2.3,23,3.9,23h16.3c1.6,0,2.9-1.3,2.9-2.9V3.9
C23,2.3,21.7,1,20.1,1z M21,20.1c0,0.5-0.4,0.9-0.9,0.9H3.9C3.4,21,3,20.6,3,20.1V3.9C3,3.4,3.4,3,3.9,3h16.3C20.6,3,21,3.4,21,3.9
V20.1z M5,5h14v3H5V5z M5,10h3v9H5V10z M10,10h9v9h-9V10z"/>
</svg> And renders as we expect. If I edit the twig.svg by removing <svg version="1.1" xmlns="http://www.w3.org/2000/svg" x="0px" y="0px" viewBox="0 0 24 24" enable-background="new 0 0 24 24" xml:space="preserve">
<path fill="#AAAAAA" d="M20.1,1H3.9C2.3,1,1,2.3,1,3.9v16.3C1,21.7,2.3,23,3.9,23h16.3c1.6,0,2.9-1.3,2.9-2.9V3.9
C23,2.3,21.7,1,20.1,1z M21,20.1c0,0.5-0.4,0.9-0.9,0.9H3.9C3.4,21,3,20.6,3,20.1V3.9C3,3.4,3.4,3,3.9,3h16.3C20.6,3,21,3.4,21,3.9
V20.1z M5,5h14v3H5V5z M5,10h3v9H5V10z M10,10h9v9h-9V10z"/>
</svg> Then we will see the bug. |
This will solve the issue: .sf-toolbar-block .sf-toolbar-icon img,
.sf-toolbar-block .sf-toolbar-icon svg {
height: 20px;
} But I'm not sure we should use a fix height. |
Sorry for the delay. After several tests, I think @Nyholm's solution is the best one. Thanks for sharing it! I did these changes: .sf-toolbarreset svg,
.sf-toolbarreset img {
- max-height: 20px;
- max-width: 20px;
+ height: 20px;
}
#menu-profiler .label .icon img,
#menu-profiler .label .icon svg {
- max-height: 24px;
+ height: 24px;
max-width: 24px;
} |
Thank you for getting back. You all probably know this but I just want to highlight this. The sizes of the icons in the toolbar has changed from 2.7 to 2.8. Which means that this fix should not be merged "as is" to 2.7. Symfony 2.7 uses 28px toolbar icons if i remember correctly. |
@Nyholm this PR does not target Symfony 2.7 at all currently |
Yeah, I know. My point was to highlight the fact that this PR should not be merged to 2.7 |
Thank you @javiereguiluz. |
…javiereguiluz) This PR was squashed before being merged into the 2.8 branch (closes #17330). Discussion ---------- Limit the max height/width of icons in the profiler menu | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #17329 | License | MIT | Doc PR | - Commits ------- 1f5f81c Limit the max height/width of icons in the profiler menu
… menu (javiereguiluz) This PR was squashed before being merged into the 2.8 branch (closes symfony#17330). Discussion ---------- Limit the max height/width of icons in the profiler menu | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#17329 | License | MIT | Doc PR | - Commits ------- 1f5f81c Limit the max height/width of icons in the profiler menu