Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Icons] Icon aliases #2127

Merged
merged 1 commit into from
Sep 9, 2024
Merged

Conversation

smnandre
Copy link
Member

@smnandre smnandre commented Sep 3, 2024

Q A
Bug fix? no
New feature? yes
Issues Fix #
License MIT

This PR introduce a first implementation of this feature, asked in #1767

Icon aliases can now be defined in the bundle configuration, to improve the DX for most used icons in a project (logo, socials, ...)

# config/packages/ux_icons.yaml
ux_icons:
    aliases:
        dots: 'clarity:ellipsis-horizontal-line'

Now

    {{ ux_icon('dots') }}

    {# is the same as #}

    {{ ux_icon('clarity:ellipsis-horizontal-line') }}

Or... with the HTML syntax

    <twig:ux:icon name="dots" />

    {# is the same as #}

    <twig:ux:icon name="clarity:ellipsis-horizontal-line" />

@carsonbot carsonbot added Feature New Feature Icons Status: Needs Review Needs to be reviewed labels Sep 3, 2024
@smnandre smnandre added the DX label Sep 3, 2024
@smnandre smnandre requested a review from kbond September 3, 2024 19:57
@javiereguiluz
Copy link
Member

Just asking ... can you use : in the alias name? E.g. if someone wants to group all aliases under a common prefix (e.g. site:... or acme:...)

@smnandre
Copy link
Member Author

smnandre commented Sep 4, 2024

That's for sure the next step!

@Xbirdfr
Copy link

Xbirdfr commented Sep 5, 2024

Hello, thank you for this implementation!

To have thought a little about this functionality, I propose the following modification:

Put an @ before the name of the alias

This allows you to add an AliasNotFound error which would be easier to debug, and to only check the configuration file when calling an alias.
Also it increases readability: @dots directly gives us the information that the icon is an alias, and it also allows us to do things like
@user:login @user:logout @admin:login

@javiereguiluz
Copy link
Member

@Xbirdfr the problem with that is that it introduces a new convention that all users of UX Icon must learn, which causes friction.

In any case, is @ forbidden in icon aliases? If it's not, what you are asking it's already possible to do. If it's forbidden, let's double check if we should really forbid it.

@Xbirdfr
Copy link

Xbirdfr commented Sep 5, 2024

@javiereguiluz I understand that, but as Icones Alias will be a new feature, I will cause less friction

Actually, if @dots is not referenced in the config, an IconNotFound error will be thrown, and this is not the good error message

With an @ we clearly know if we want an alias or not while using ux icon, and allow a better readability and debugging.

But it's maybe too much and I understand this, but I really think this will be a good way to handle alias

@smnandre
Copy link
Member Author

smnandre commented Sep 5, 2024

oblem with that is that it introduces a new convention that all users of UX Icon must learn, which causes friction.

@javiereguiluz yep absolutely!

And we decided to avoid it in the TwigComponent shared by bundles, so it would be a bit strange to introduce it here.

@smnandre
Copy link
Member Author

smnandre commented Sep 5, 2024

@user:login @user:logout @admin:login

@Xbirdfr This is precisely what i did not want to handle for now. We have the : to identify a namespace/ icon set.

Full disclosure: i will open a PR after this one to implement the long-waiting "configuration per icon sets/directorys" ... so let's talk about namespaces aliases afterwards 😅

I promised the simple aliases long time ago, and we have been poked again recently ... so this is more the context of this PR :)

Personally, I will use the aliases in several projects, to have simple "one-word-domain-icon-names" for the 10/12 most used icons (which come from different sets -- excluding here any partial mapping). Something like this:

# config/packages/ux_icons.yaml
ux_icons:
    aliases:
    
        # Logos
        github: 'simple-icons:github' 
        twitter: ' pajamas:x'
        php: 'bxl:php'

       # Actions
       delete: 'tdesign:delete'
       add: 'material-symbols:add'
       
       # ...

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Sep 6, 2024
@javiereguiluz javiereguiluz merged commit 38e685d into symfony:2.x Sep 9, 2024
41 of 42 checks passed
@javiereguiluz
Copy link
Member

Simon, thanks for yet another nice feature contribution!

@Kocal Kocal deleted the feat/icons-aliases branch September 12, 2024 06:14
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
DX 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.

6 participants