Skip to content

Conversation

@ThomasLandauer
Copy link
Contributor

@ThomasLandauer ThomasLandauer commented May 28, 2025

Q A
Bug fix? no
New feature? no
Docs? yes
Issues Closes #2782
License MIT

Page: https://symfony.com/bundles/ux-icons/current/index.html#performance

Info is taken from #2782 (comment)

Comment on lines +459 to +462
When including the same icon multiple times on one page, the same string (SVG content)
is inserted repeatedly in the HTML source. However, since most icons are small (just a few
hundred bytes), this overhead is negligible in most cases (especiall wthen proper
transfer encoding like e.g. gzip is enabled).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The UX Icons component is a PHP one, that does generates HTML.

Do you think on twig documentation, you'd write "every time you include a partial, it is included" ?

:)

@smnandre
Copy link
Member

I'm personally voting against this change (for the reasons explained in the original issue and here)

  • this is not something that solves a real need to me
  • .... and instead gives importance to something that has none for 99.9% of users

Let's see if other have a different opinion

@kbond
Copy link
Member

kbond commented May 29, 2025

I'm against it also, I'm concerned this could scare users off. With modern browsers/compression/http2, I'm not convinced this is a real issue. We originally (pre-release) had a "deferred" feature to fix this "issue" but I saw numbers that showed it was nothing but a micro-optimization.

@ThomasLandauer
Copy link
Contributor Author

I see that you have strong opinions.

So just take what you think is useful, and close/delete the rest.

@smnandre
Copy link
Member

smnandre commented Jun 2, 2025

We're always open to improvements

Recent suggestions felt a bit mixed --some very valid points, others less aligned with concrete user needs or going against points we've already covered.

Thanks again for your time and input!

@smnandre smnandre closed this Jun 2, 2025
@ThomasLandauer ThomasLandauer deleted the patch-4 branch June 2, 2025 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Icons Status: Needs Review Needs to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Icons] Explain client side performance

4 participants