Skip to content
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

first draft for ux icons support #195

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

first draft for ux icons support #195

wants to merge 4 commits into from

Conversation

kevinpapst
Copy link
Owner

Description

This is a first draft for UX icons integration.
It supports both Fontawesome webfont and UX icons.

It ships a command that can convert current webfont configuration and download all icons configured at tabler.icons, and it even supports converting old FA5 to FA6 names (the webfont supported both versions, iconify only the new names).

The tabler_icon() Twig functions works as before and supports both implementations.
The |tabler_icon Twig filter will still work with webfonts, but not with UX icons.

The biggest issues I see:

  • It increases the page size by 20kB for each request
  • And many icons cannot be cached e.g. the entire menu is based on runtime events and those icons won't be cached (according to the docs). That means for every page load PHP will read all these SVGs from disk? If that is true (I need to investigate further) this is a blocker for me as of now.

Closes #194

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I updated the documentation (see here)
  • I agree that this code will be published under the MIT license

@kevinpapst kevinpapst added the Status: Needs Review Not under investigation label Apr 26, 2024
@cavasinf
Copy link
Collaborator

Big 👍 for splitting RuntimeExtension in IconExtension.

Comment on lines 45 to +51
icons:
# used for the action_collapsebutton components
collapse: fas fa-chevron-down
collapse: "fa6-solid:chevron-down"
# used for the password-reset template
mail: far fa-envelope
mail: "fa6-regular:envelope"
# used for the 404/500/... error page
back: fas fa-long-arrow-alt-left
back: "fa6-solid:left-long"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we still keep the icons config part in the future, or just during the temporary "deprecated" period?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Keep it during the deprecation period. I want to try to make this PR completely BC safe.

Copy link
Owner Author

Choose a reason for hiding this comment

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

On the other hand, this is a tiny layer on top of the UX icons later, that helps managing and downloading them.
Not sure yet ...

return $this->iconRenderer->renderIcon($icon, ['class' => 'icon']);
}

@trigger_error('Support for webfonts is deprecated. Switch to UX icons. Using twig function tabler_icon("' . $name . '") is deprecated.', \E_USER_DEPRECATED);
Copy link
Owner Author

Choose a reason for hiding this comment

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

Suggested change
@trigger_error('Support for webfonts is deprecated. Switch to UX icons. Using twig function tabler_icon("' . $name . '") is deprecated.', \E_USER_DEPRECATED);
@trigger_error('Using twig function tabler_icon("' . $name . '") without an iconify identifier is deprecated, see https://github.com/kevinpapst/TablerBundle/pull/195.', \E_USER_DEPRECATED);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs Review Not under investigation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Icon] Bye bye tabler_icon ➡️ Welcome Symfony UX Icon 🎉
2 participants