Skip to content

[Icons] improve DX when symfony/http-client is not installed #2678

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

Merged
merged 1 commit into from
Apr 14, 2025

Conversation

kbond
Copy link
Member

@kbond kbond commented Apr 5, 2025

Q A
Bug fix? no
New feature? yes
Docs? yes
Issues Alternative to #2667
License MIT

Exception when rendering a template w/o http-client:
Screenshot 2025-04-05 at 1 13 18 PM

Exception when running ux:icon:* command w/o http-client:
Screenshot 2025-04-05 at 1 14 50 PM

Output of ux:icons:warm -v when there are "potential" invalid icons:
Screenshot 2025-04-05 at 1 38 06 PM

TODO

@carsonbot carsonbot added Feature New Feature Icons Status: Needs Review Needs to be reviewed labels Apr 5, 2025
@kbond kbond force-pushed the icons/no-http-client branch 3 times, most recently from 8bc80c3 to 2c18a5f Compare April 5, 2025 17:41
Copy link
Contributor

@dbu dbu left a comment

Choose a reason for hiding this comment

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

thanks, great!

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Apr 7, 2025

.. caution::

The process to find icons to cache in your Twig templates is imperfect. It
Copy link
Member

Choose a reason for hiding this comment

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

Maybe advise Lock command here? It shows details (-vv) on wether the IconSet does not exist (99% of the false positive extracted from templates) or if there is a real problem with an icon.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added this caution admonition to the lock command section too.

Copy link
Member

@smnandre smnandre left a comment

Choose a reason for hiding this comment

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

Nice! 👏

- throw exception if http-client is not available
- always enable iconify, iconify commands and on-demand registry
- display "potential" missing icons to `ux:icons:warm-cache`
@kbond kbond force-pushed the icons/no-http-client branch from 2c18a5f to c14e89b Compare April 7, 2025 13:54
Copy link
Member

@Kocal Kocal left a comment

Choose a reason for hiding this comment

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

😍

@kbond kbond merged commit 67f5544 into symfony:2.x Apr 14, 2025
77 of 80 checks passed
@kbond kbond deleted the icons/no-http-client branch April 14, 2025 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New Feature Icons Status: Reviewed Has been reviewed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants