search

symfony / ux

Symfony UX initiative: a JavaScript ecosystem for Symfony
https://ux.symfony.com/
MIT License
820 stars 297 forks source link

[ux-icons] Allow stimulus functions to be parsed #1799

Closed Nayte91 closed 4 months ago

Nayte91 commented 4 months ago

Description

Hello all,

I'm playing with ux-icons, trying to replace my stimulus action to favorite/unfavorite an element, over a font awesome star icon; Something very classical.

As I can't use the function syntax {{ ux_icon('favorite') }} to embedd my stimulus data, I try to use the HTML syntax. I don't know if it's due to ux-icons or twig component project, but some behaviors can be changed to improve DX, around stimulus functions and title attribute management. Please see below,

Example

this is my original icon, stimuled:

<button
    type="button"
    class="transparent"
    {{ stimulus_action('replay-favorite', 'toggle') }}
    {{ stimulus_controller('replay-favorite', {
        'add-path': path('app_user_replay_favorite_add', {id}),
        'remove-path': path('app_user_replay_favorite_remove', {id})
    }) }}
>
    <i
        class="{{ isFavorite ? 'fa-solid' : 'fa-regular' }} fa-star"
        title="{{ isFavorite ? 'Remove from favorite' : 'Add to favorite' }}"
        {{ stimulus_target('replay-favorite', 'icon') }}
        data-state="{{ isFavorite ? 1 : null }}"
    ></i>
</button>

Then, here's what I try with twig syntax...

<twig:UX:Icon
    name="custom:unfavorite" 
    {{ stimulus_target('replay-favorite', 'icon') }}
    title="{{ isFavorite ? 'Remove from favorite' : 'Add to favorite' }}"
    data-state="{{ isFavorite ? 1 : null }}"
/>

...But it triggers a SyntaxError: Expected attribute name when parsing the "<twig:UX:Icon" syntax.

Then I tweak a bit my twig component to remove stimulus function:

<twig:UX:Icon
    name="custom:unfavorite"
    data-replay-favorite-target="icon"
    title="{{ isFavorite ? 'Remove from favorite' : 'Add to favorite' }}"
    data-state="{{ isFavorite ? 1 : null }}"
/>

But it triggers another error: Error rendering "UX:Icon" component: Invalid value type for attribute "data-state". Boolean or string allowed, "int" provided.

Ok, let's tweak to get a near final version:

<twig:UX:Icon
    name="{{ isFavorite ? 'custom:favorite' : 'custom:unfavorite' }}"
    data-replay-favorite-target="icon"
    title="{{ isFavorite ? 'Remove from favorite' : 'Add to favorite' }}"
    data-state="{{ isFavorite }}"
/>

This works and render; It works interestingly, with the stimulus function working well. Only the title attribute is rendered differently and doesn't work this way.

We see how title attribute is managed (or how Font awesome manages it?), a way that it works, where my current twig component implementation doesn't.

Kocal commented 4 months ago

Hi, in fact you probably don't want to use stimulus_* functions, AFAIK the preferred way is to use the Stimulus Jetbrains plugin, which can gave you a nice autocomplete in your templates and controllers

Otherwise, it's possible to use stimulus_* functions with the <twig:> syntax, you can spread them like this:

<twig:UX:Icon
    name="custom:unfavorite" 
-    {{ stimulus_target('replay-favorite', 'icon') }}
+    {{ ...stimulus_target('replay-favorite', 'icon') }}
    title="{{ isFavorite ? 'Remove from favorite' : 'Add to favorite' }}"
    data-state="{{ isFavorite ? 1 : null }}"
/>

EDIT: btw there is no real documentation, it has been implemented in https://github.com/symfony/webpack-encore-bundle/pull/178 then moved into the dedicated bundle which implements .toArray(), called by getIterator(), implicitly called by {{... }}

Nayte91 commented 4 months ago

Hello kocal,

Hi, in fact you probably don't want to use stimulus_* functions, AFAIK the preferred way is to use the Stimulus Jetbrains plugin, which can gave you a nice autocomplete in your templates and controllers

You're right, it's ultimately not a big deal, as stimulus functions are not that mandatory. PHPS can help.

Otherwise, it's possible to use stimulus_* functions with the <twig:> syntax, you can spread them like this:

<twig:UX:Icon
    name="custom:unfavorite" 
-    {{ stimulus_target('replay-favorite', 'icon') }}
+    {{ ...stimulus_target('replay-favorite', 'icon') }}
    title="{{ isFavorite ? 'Remove from favorite' : 'Add to favorite' }}"
    data-state="{{ isFavorite ? 1 : null }}"
/>

Tried, and it works! Nice one!

EDIT: btw there is no real documentation, it has been implemented in symfony/webpack-encore-bundle#178 then moved into the dedicated bundle which implements .toArray(), called by getIterator(), implicitly called by {{... }}

No documentation on the ux-icons page for this trick, but I also don't know if it's a thing to enrich a svg tag with this kind of html attributes (title, data-whatever, ..) or if it violates 39 rules and standards?

Still the data-state error and the title not working are here.

For data-state, it seems that there's 2 different behaviors:

I feel like there is something to align here.

Kocal commented 4 months ago

Hum, usually you would like to wrap your icon with a <button type="button">:

  1. it will allows you to use whatever attributes you want (e.g.: data-replay-favorite-target, data-state)
  2. it will improve your app accessibility

For <twig:UX:Icon>/ <svg>, IMO only some aria-* (and maybe title) attributes could be added, e.g. aria-label or aria-labelledby

CMH-Benny commented 4 months ago

There is another workaround possible, by using the "fake" spread operator, so if you want to keep using the helper, you could do it like this:

{% set icon_attr = {
  name: 'custom:unfavorite',
  ...stimulus_target('replay-favorite', 'icon'),
  title: isFavorite ? 'Remove from favorite' : 'Add to favorite',
  'data-state': isFavorite
} %}
<twig:UX:Icon {{ ...icon_attr }} />

This is afaik the only way to make the parser work, if you have some complex twig markup to determine the value for an attribute

E: Should have read further, that's not the real issue, I wonder how that works, since I had issues with parsing {{ }} in component html syntax except for the spread, but anyway - This is not providing help on the actual issue, so don't mind my post - Sorry :D

smnandre commented 4 months ago

My recommandation: you really should write "classic" HTML with the component or you will probably won't profit from cache / performance optimisation.

<twig:UX:Icon name="custom:unfavorite"  data-replay-favorite-target="icon" title="{{ isFavorite ? 'foo' : 'bar' }}"  />

If you need more attributes you should wrap a span/div around i think

Nayte91 commented 4 months ago

My recommandation: you really should write "classic" HTML with the component or you will probably won't profit from cache / performance optimisation.

<twig:UX:Icon name="custom:unfavorite"  data-replay-favorite-target="icon" title="{{ isFavorite ? 'foo' : 'bar' }}"  />

If you need more attributes you should wrap a span/div around i think

That's a smart point.

I shall remove a lot of attributes from the twig tag, and finish to a nice: 

<button
    type="button"
    class="transparent"
    title="{{ isFavorite ? 'Remove from favorite' : 'Add to favorite' }}"
    {{ stimulus_action('replay-favorite', 'toggle') }}
    {{ stimulus_controller('replay-favorite', {
        'add-path': path('app_user_replay_favorite_add', {id}),
        'remove-path': path('app_user_replay_favorite_remove', {id})
    }) }}
    data-state="{{ isFavorite }}"
>
    <twig:UX:Icon
        class="favorite__icon"
        name="{{ isFavorite ? 'custom:favorite' : 'custom:unfavorite' }}"
        data-replay-favorite-target="icon"
    />
</button>

But few points:

About the optimal cache/perf optimization, I have 3 possibilities:

{# first with twig component #}
<twig:UX:Icon
    class="favorite__icon"
    name="{{ isFavorite ? 'custom:favorite' : 'custom:unfavorite' }}"
    data-replay-favorite-target="icon"
/>

{# second with function #}
{{ ux_icon(isFavorite ? 'custom:favorite' : 'custom:unfavorite', {'data-replay-favorite-target': 'icon', class: 'favorite__icon'}) }}

{# third with condition from twig #}
{% if isFavorite %}
    <twig:UX:Icon
        class="favorite__icon"
        name="custom:favorite"
        data-replay-favorite-target="icon"
    />
{% else %}
    <twig:UX:Icon
        class="favorite__icon"
        name="custom:unfavorite"
        data-replay-favorite-target="icon"
    />
{% endif %}

{# fourth with cache optimization from documentation #}
{% set favoriteIcons = {"1": 'custom:favorite', "0": 'custom:unfavorite'} %}
{{ ux_icon(favoriteIcons[isFavorite], {'data-replay-favorite-target': 'icon', class: 'favorite__icon'}) }}

I suppose the 4th choice is the right one?

Also, with this system, I struggle to swap my icon from favorite to unfavorite (and the other way), because I actually swap a class with JS to change my icon, but with ux-icons, the image doesn't come from a class managed by browser, but instead is server rendered. So I'm wondering what should be the right tool for such a common case... I think about a live component, but WDYT?

smnandre commented 4 months ago

With this situation, they should be equivalent in term of performance, as the "twig component" is transformed back to a twig function after parsing 🥷

smnandre commented 4 months ago

Also, with this system, I struggle to swap my icon from favorite to unfavorite (and the other way), because I actually swap a class with JS to change my icon, but with ux-icons, the image doesn't come from a class managed by browser, but instead is server rendered.

@Nayte91 you can look how the dark/light mode switcher work on ux.symfony.com (source code is available in this repo), but i felt the safest way was to have both icons in HTML, and a parent div would play the "toggle" role.

Nayte91 commented 4 months ago

Thank you @smnandre for this suggestion, I looked for it and I understand your point of rendering both icons and choose after.

Still, the most I think about the more I'm sure this is a live component's use case. As I make an API call, then write a controller to handle the call, change my boolean on my database, send back a status code, and based on this status code I will modify my icon... This is in fact 75% back 25% front, so I will try to solve it with live component.

What made me doubt about it is there's no example on the live component page that fills this need, as it is a very common one: favorite/unfavorite, like/unlike, vote/unvote, follow/unfollow, retweet/not retweet, and basically any boolean switcher that need to hit the backend (not dark/light theme so!) may fit into it.

Guess I overlooked the Up & Down Voting example, that should be a simplier one to enroll more people into it... Because voting up/down is in fact a derivated scenario of a classical boolean backend switch, only with an additional up/down parameter.

Nayte91 commented 4 months ago

We spoke about title attribute but I didn't manage to make it work, and as I see how font awesome handles this point, I feel like it deserves a proper issue ticket. I will open one soon :)

edit: https://github.com/symfony/ux/issues/1807

smnandre commented 4 months ago

As I make an API call, then write a controller to handle the call, change my boolean on my database, send back a status code, and based on this status code I will modify my icon... This is in fact 75% back 25% front, so I will try to solve it with live component

Yep it seems a very good usecase :)

smnandre commented 4 months ago

I’m closing as sub-parsing will not be possible but if you have any other question / problem / suggestion feel free to open another one :)