Multiple Icon Templates

[ypnos-web]

This is a (multiple allowed):

• bug
• enhancement
• question

Since Fontawesome 5 got released, I am faced with the distinction between icon shapes 'solid', 'regular', 'light', and 'brand'. These are selected through a supplemental class 'fas', 'far', 'fab' instead of simply 'fa' (which falls back to solid). While this is mostly a Fontawesome Pro feature, it also affects free users as some regular icons are free to replace the previous '-o' icons, and then there are the brand icons.

To be able to selectively use a specific shape we would need several icon templates, not only a default one. E.g.:

'icon' => [
    '_default' => '<i class="far fa-{{type}}{{attrs.class}}"{{attrs}}></i>',
    's' => '<i class="fas fa-{{type}}{{attrs.class}}"{{attrs}}></i>',
    'b' => '<i class="fab fa-{{type}}{{attrs.class}}"{{attrs}}></i>',

Then we could enhance the magic icon syntax:

i:<iconname> // render icon['_default']
i:s:<iconname> // render icon['s']

Well the first problem with this idea is that templates reside in a flat array so my nesting would not work.
But just for the general idea.

What do you think about it and what design considerations would you have?
I would be happy to prepare a pull request for this feature.

[Holt59]

That's an interesting idea, maybe a better idea for the templates would be to append something to 'icon', e.g.:

'icon' => '<i class="fa-{{type}}{{attrs.class}}"{{attrs}}></i>',
'icon-s' => '<i class="fas fa-{{type}}{{attrs.class}}"{{attrs}}></i>',
'icon-b' => '<i class="fab fa-{{type}}{{attrs.class}}"{{attrs}}></i>'

And when parsing the easy-icon, you get i:s:<iconame> and you render using icon-s. This way, users can easily add the icon templates they want and have them working with easy icons.

The following regex should work for parsing the icon:

(^|\s+)i:([a-zA-Z0-9\\-_]+:)?([a-zA-Z0-9\\-_]+)(\s+|$)

Something could be achieved by backing the icon template prior to the regex parsing, but maybe it is a better idea to add a custom option to the icon method in the HTML helper, like adding a 'template' option and defaulting it to 'icon'? Or maybe if you have some ideas on how the easy-icon trait can be detached from the HTML helper, I'll be glad to hear them, since it's something I am not really happy with...

[ypnos-web]

I agree that the entry point to this functionality would be HtmlHelper's icon() method that could take a custom option for it. I will also have a closer look at the easy icon trait to see if I find a way to improve how it interfaces with the rest.

[ypnos-web]

Options I considered:

1. Make icon() a functionality provided by EasyIconTrait and use the trait in HtmlHelper to expose to the user. All functionality is in one place. Drawback: Configuring the template does not work as expected (the template would need to be configured for all helpers exhibiting the trait individually).
2. Move the whole functionality of EasyIconTrait to HtmlHelper. All functionality, again, is in one place. Drawback: _makeIcon() and _easyIcon() need to be public methods and their hacky interface will be exposed.
3. Make an IconHelper for this. Drawbacks: Overkill, same drawback as option 2.

Have you thought about reversing how easyIcon works to make the interface less ugly? Consider replacing the call:

$this->_easyIcon('parent::link', 0, 2, [$title, $url, $options]);

with:

$this->Html->injectIcon($title, parent::link($title, $url, $options));

Now what insertIcon does is regex match $title within the already formatted HTML output of parent::link() and replace the icon at this point. No option mangling is needed.

Signature:

public function injectIcon($title, $text = null, $options = []);

Advantages:

1. No silent escape suppression. The text is still escaped as expected
2. Transparent in what it does, an interface that we can expose through the helper
3. Tag options can be supplied specifically for the icon (e.g., in my use case, where I wrote a custom button helper and add 'fa-fw' for menu buttons using _makeIcon())

What do you think about this idea?

[Holt59]

This sounds interesting, I'd rather have this functionality in a IconHelper to separate it from HtmlHelper. HtmlHelper::icon could then falls back to IconHelper::icon (or something similar) for simplicity/compatibility.

You'd need to set templates for IconHelper instead of HtmlHelper but I think it's an improvement rather than anything else (HtmlHelper already contains two much templates... ).

I think we would need something to disable easy icon if needed, maybe:

$this->Icon->inject = false; // another name maybe

But it would be quite difficult to separate "automatic" injection from manual one... Maybe something else than a simple switch? I don't mind if this is not as simple as switching a variable to true/false since this is probably not used that often, I'd rather have something clean for this (if we make the icons clean, why not make everything clean ? 😃).

The regex for the pattern would need to be changed since it actually only matches beginning of string / pattern between spaces. At first glance this does not look too complex (would simply need to match > followed by spaces), but this would require more extensive testing than what is actually available.

[ypnos-web]

Regarding the regex, my idea was to apply it on $title and then do a str_replace of $title within $text (if $title had matched and was actually altered). This is why it takes $title as a separate parameter, to make it safer against false replacements outside the title. We could harden it further by enforcing sth like '>{$title}<'.

[Holt59]

I am not fan of this idea because:

• You would need to str_replace against the possibly escaped $title, this means more work for the caller (need to pass $options) and the method (need to check), and it makes the interface not so user-friendly (people have to know that they should pass the escape if necessary).
• ...or you would need to escape $title before sending it to injectIcon, but this would require more work from the caller (in particular for all the automatically injected icon, this would add lots of boilerplate code... ).
• Regarding the interface, this duplicates the place where you need to put the icon, e.g.:

$this->Icon->injectIcon('i:home', $this->SomeHelper->someMethod('i:home'));

I'd rather remove the $title option and parse the final $text, I don't think there would be much problem with this, especially if we provide something to disable it when needed, or maybe better, allow user to customize the matching pattern (and eventually disable it).

[ypnos-web]

So the two issues you raise are argument duplication and how to deal with escaping.

1. For me the argument duplication is not a big deal as I only see calls for this method from other helpers or custom widget classes like one I wrote for myself. The usual developer will just call these or use icon() directly. So only few places in library/plugin code call injectIcon.

2. Escaping issue is a valid point, however I believe we would get away with simply trying to replace both escaped and unescaped title (try unescaped first, if it fails, escape it and try again). So it works transparently from the outside and in almost all cases, the first test will be the end of it.

My counter argument for parsing the whole output is that people might have weird tag options where an 'i:something' slips in (e.g. data attributes). A test for '>i:something ' || ' i:something<' would certainly migitate this problem but would fail to work with a (stupid) button template like '<button>%nbsp;{{title}}&nbsp;</button>'.

I am just putting out my thoughts here, I am happy to implement either approach, just name it.

Regarding an easy way to disable it, obviously we can have enableInjection() / disableInjection() methods. I would like an easy way to do it case-by-case, e.g.

$this->Form->button('i:1 j:2', ['icon' => false]);

but this would lead to quite some extra work for the FormHelper (take icon option from option array, unset it in option array, pass it to injectIcon…), so it's probably not what we want after all.

[Holt59]

I am answering the last part of the comment first — I agree it would be great to have a way to disable the injection case-by-case, even though I think icon is too generic here, maybe inject-icon 😀

I think there is a simple way to disable some options from being parsed as an attribute: we need to override formatAttribute inside the template class, but since helpers are already using a custom template class, this should not be a lot of work. We could then simply forward all options from the FormHelper to the IconHelper, e.g.:

$this->Icon->injectIcon($title, parent::link($title, $url, $options), 
    ['_original' => $options]);

Using a custom (documented) option name so that other options (e.g. class or id) are not used for the actual icon.

This _original option could also be used for the escape if we choose to apply the pattern only on $title, even if this is less easy to do since the default escape is not the same for all methods...

Regarding the parsing now, I agree that this may raise some issue, but as I said, I think we could have a "standard" rule, e.g. very generic, and then user would disable the injection when needed. Something very generic could be:

(^|[^\p{L}\p{N}]+)i:([a-zA-Z0-9\\-_]+:)?([a-zA-Z0-9\\-_]+)([^\p{L}\p{N}]+|$)

We could also allow user to set a specific pattern if they want (documenting it correcty), so they could do:

// Current one:
$this->Icon->setMatchingPattern('(^|[\w\W\d]+)i:([a-zA-Z0-9\\-_]+:)?([a-zA-Z0-9\\-_]+)([\w\W\d]+|$)');
// New one?
$this->Icon->setMatchingPattern('(^|[^\p{L}\p{N}]+)i:([a-zA-Z0-9\\-_]+:)?([a-zA-Z0-9\\-_]+)([^\p{L}p{N}]+|$)');
// Disable?
$this->Icon->setMatchingPattern(false);
// Enable? Falls back to the default one?
$this->Icon->setMatchingPattern(true);

[ypnos-web]

Great idea of allowing matching pattern customization, I would add that I think for the user it is cleaner to have a setMatchingPattern() / enableMatching() / disableMatching() interface, as it is self-explanatory.

[Holt59]

Yes, it would probably be better, I have to check CakePHP's convention for this if there are any (maybe setMatchingEnable(true / false);.

[Holt59]

Just checked, it is enableMatching, disableMatching.

[Holt59]

I think we could use named capture group in the regex so that user can customize how template are chosen depending on the pattern, e.g.:

// The <type> group is the special one used to select the actual icon.
$pattern = (^|[^\p{L}\p{N}]+)i:(?<type>[a-zA-Z0-9\\-_]+)([^\p{L}\p{N}]+|$);
$templatePattern = 'icon';

// New pattern, {{shape}} will be replaced by the matching:
$pattern = (^|[^\p{L}\p{N}]+)i:((?<shape>[srb]):)?(?<type>[a-zA-Z0-9\\-_]+)([^\p{L}\p{N}]+|$);
$templatePattern = 'icon-{{shape}}';

The method setMatchingPattern would take an optional argument modifying the template pattern (that would be 'icon' or 'icon-{{shape}}' by default).

This way, this is extremely customizable, I could even use something like:

$pattern = (^|[^\p{L}\p{N}]+)i((?<toolkit>[srb]):)?:((?<shape>[srb]):)?(?<type>[a-zA-Z0-9\\-_]+)([^\p{L}\p{N}]+|$);
$templatePattern = 'icon-{{toolkit}}-{{shape}}';

To switch between FontAwesome and e.g. Glyphicon simply by doing i:fa:home or i:gl:home.

Maybe we would need to find a way to find the template pattern when one of the matching group is not found, but that's something to consider I think. One idea would be to force a leading hyphen -, e.g. the template pattern would be. icon{{toolkit}}{{shape}} and {{toolkit}} would be replaced by -$toolkit only if tookit is found by the matching.