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.

[Holt59]

I created a icon-helper branch, it'll be easier for me to accept PR to this branch and then merge it to master when everything is working as intended.

[ypnos-web]

So this sounds like a version of preg_replace that supports named back references. Do a preg_replace_callback whereas the callback will fill-in all named captures as provided?

So in summary:

1. We have several templates and a default template selection as configurable by the user. we can provide common templates (for glyphicon and fontawesome) as defaults and glyphicon would be the default template selection
2. The user can configure the matching pattern, and the <template> capture group is used to determine the template from above, or if not present, default is used.
3. The user can also use/reference to other capture groups in both matching pattern and template strings at will which will be generically filled. The default pattern and templates would include a <shape> (or <name>?) capture group for the icon name
4. There is a injectIcon option that is fished from '_original' or maybe '_parent' option array to disable functionality when false
5. We move over icon() from HtmlHelper and add a template option to it for convenience. Any other variables that the user defined in their templates they can provide with templateVars option.

Outside IconHelper:

• formatAttribute() needs to strip/ignore injectIcon in custom template class
• all helpers use IconHelper for injecting icons, passing $options as ['_parent' => $options]
• HtmlHelper wraps icon() method by IconHelper

If you agree with this I will start working on the code.

Just as a remark I think the possibility of translating regex captures to template variables might be a bit of an obscure feature for this specific context.

[Holt59]

So this sounds like a version of preg_replace that supports named back references. Do a preg_replace_callback whereas the callback will fill-in all named captures as provided?

I think preg_replace_callback handles named pattern since PHP 5.3, so it should be ok.

1. We have several templates and a default template selection as configurable by the user. we can provide common templates (for glyphicon and fontawesome) as defaults and glyphicon would be the default template selection

Yes, and templates would be in the IconHelper class.

2. The user can configure the matching pattern, and the <template> capture group is used to determine the template from above, or if not present, default is used.

You mean have <template> named groupe and use it like i:fa-icon-far:home?

3. The user can also use/reference to other capture groups in both matching pattern and template strings at will which will be generically filled. The default pattern and templates would include a (or ?) capture group for the icon name

Yes, let's start with something matching shape and type (name to be confirmed) and replace it in a icon{{shape}} pattern for template.

4. There is a injectIcon option that is fished from '_original' or maybe '_parent' option array to disable functionality when false

Yes, I don't know if injectIcon or inject-icon would be the better, I think I have used the latter convention for some other options...

5. We move over icon() from HtmlHelper and add a template option to it for convenience. Any other variables that the user defined in their templates they can provide with templateVars option.

Yes, HtmlHelper::icon simply forwards everything to IconHelper::icon (or some other method).

• formatAttribute() needs to strip/ignore injectIcon in custom template class

Yes, this can be easily done by calling parent::formatAttribute(..., ['injectIcon']);.

• all helpers use IconHelper for injecting icons, passing $options as ['_parent' => $options]

Yes.

• HtmlHelper wraps icon() method by IconHelper

Yes.

Just as a remark I think the possibility of translating regex captures to template variables might be a bit of an obscure feature for this specific context.

Maybe, but you'd be surprised by the number of obscure feature in this plugin 😀 I don't think that much people are even aware of the easy icon feature. As long as this does not disturb the basic usage of this feature, I'm fine with adding this.

[ypnos-web]

Ok, so I will be working on this.

[Holt59]

@ypnos-web After closing the above-mentioned PR, I was thinking about a nice feature that we could implement using __invoke and __call. Below is a (non-working) example:

class IconHelper {

    public function icon($type, $options = [], /* Extra? */) {
        $options += [
            'template' => $this->getConfig('defaultTemplate') // Something like this?
        ];
        // Whatever... 
    }
  
    public function __invoke() {
        return call_user_func_array([$this, 'icon'], func_get_args());
    }
    
    public function __call($name, $arguments) {
        // Function name -> Template
        $result = $this->nameToTemplate($name);
        // Update arguments... 
        $arguments[1]['template'] = $result;
        return call_user_func_array([$this, 'icon'], $arguments);
    }
};

Assuming $this->Icon is the icon helper, we could have stuff like this:

$this->Icon->icon('home'); // Default icon
$this->Icon('home'); // Same as above, less verbose.
$this->Icon->fa('home'); // Use icon-fa template?
$this->Icon->faS('home');; // Use icon-fa-s template?

At first, I thought about using faIcon as before, but since the helper is dedicated to icon, this seems redundant, and $this->Icon->fa('home') seems better for me.