Skip to content

feat(icons): add ux:icons:lock command to "mass import" used icons #1660

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 1, 2024

Conversation

kbond
Copy link
Member

@kbond kbond commented Mar 25, 2024

Q A
Bug fix? no
New feature? yes
Issues n/a
License MIT

There has been valid concern over using the on-demand system in production. Icons could change/be removed or the iconify service could be down. This new command searches your twig files and imports valid iconify.design icons in your twig file and mass imports them locally. By default, they aren't overwritten but using the --force option enables this.

I think, the official best practice should be:

  • on-demand enabled in development only
  • part of your workflow includes running ux:icons:lock (could be automated)
  • (future) ux:icons:lint to be used in your CI to ensure all icons are available locally

This could/should be reflected in the official recipe (TODO).

@carsonbot carsonbot added the Status: Needs Review Needs to be reviewed label Mar 25, 2024
@smnandre
Copy link
Member

So if i understand:

  • during dev, you use ux_icon(foo), foo is magically downloaded from iconify and stored in cache
  • if you call the lock method, it's saved in assets/icons (and can be committed)
  • if you call another time lock with --force you can override the local one

This seem really sensible!

(+1 for the lint command idea)

@kbond kbond force-pushed the icons/import-all branch from 2b63fb7 to 23dc48a Compare April 1, 2024 12:44
@kbond kbond merged commit e9faf78 into symfony:2.x Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs Review Needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants