-
-
Notifications
You must be signed in to change notification settings - Fork 355
[Icons] ux icons website should template size #2669
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
Comments
It is up to you to decide the size... here are only listed the stricti minimul amount of code for everyone to customize. |
its tricky. not mentioning that a size is needed is a bit strange. in https://symfony-devs.slack.com/archives/C01FN4EQNLX/p1743336193049989?thread_ts=1743280429.656589&cid=C01FN4EQNLX @kbond said that icon set websites would normally include a size, so i thought it would make sense to do it here too. that said, i have set a size in the default attributes now, as i want my icons to always be the same size (and can still overwrite when i need to deviate) feel free to close as won't fix if you think it would be wrong to have the size here. |
Its tricky because how you size them depends on the css framework (or lack of) you use. Heroicons shows tailwind sizing, bootstrap icons shows bootstrap sizing |
hm, indeed tricky. we could add some footnote under the 3 copy-paste fields mentioning that you should add size as per your css framework, linking to the doc section. wdyt? |
I'd be fine with adding I'm not sure the other @symfony/ux team members would agree though. |
I'd be fine adding a row for Tailwind users. |
But the main goal is not to for users to add this on every icon, instead we should promote the usage of settings to do so. |
I don't know if this is good idea. Most icon repos including iconify show setting the size on each svg. That makes sense as the icon size can vary depending on the context. |
What i was saying is that these values are fixed per icon sets, and so should/could be set once instead of copy-pasting things everywhere (and we should not ask users to open source code and find these values them-selves) But i'm 100% serious, let's add a full Tailwind example, this is not a bad idea ! Regarding attributes, best practice would be to define height and width ... with unit-less values Look for instance.... on this Github page ![]() (oh and... by the way.. Same thing on the 3 first i tried:
So yeah, we could add these attributes automatically (depending on a config/option) during render.. and/or on some examples (especially if we add a new part in the documentation regarding styling)
Like for HTML image, width and height are not simply about the rendered size, but viewport / ratio information for browsers to determine space and for the svg engine to determine what's "in" or "out" the inner viewport. |
A quick experiment shows me the unitless values don't work like I expect. ux_icons:
default_icon_attributes:
fill: currentColor
height: 16
width: 16 {{ ux_icon('bi:person-fill', {class: 'w-full'}) }} The icon is full width but only 16 (something) high. I expected the height to be proportional to the width. To me, the way it works now is exactly like I'd expect: {{ ux_icon('bi:person-fill') }} {# fills the entire width of the containing html, proportions are correct because of the view box #}
{{ ux_icon('bi:person-fill', {class: 'size-2'}) }}
{{ ux_icon('bi:person-fill', {class: 'size-4'}) }}
{{ ux_icon('bi:person-fill', {class: 'size-8'}) }}
{{ ux_icon('bi:person-fill', {class: 'size-12'}) }}
{{ ux_icon('bi:person-fill', {class: 'w-full'}) }} For the most part, I think you don't want the icon to fill the entire width, so adding a sensible default class in the snippet makes sense. |
The UX icons finder shows the render / twig how to show the found icon, but that template is without size. As per slack discussion, the
<svg>
element must have a size specified to work properly inside HTML.It would make sense to add a sensible default size as style attribute, maybe
1em
?The text was updated successfully, but these errors were encountered: