IronIcon should extend Icon and allow alternate collections

[jflamy]

Enhancement request:

• IronIcon cannot be used where an Icon is, which is bad. Only IronIcon.create() would be needed, without the constants.
• IronIcon does not currently support using the other icon sets in iron-icons (suchs maps:, hardware:, av:, etc.) I have not (yet) found a way to do this in Java -- adding additional @HtmlImport for these collections seems to do nothing.

Version 12.0.5

[pleku]

Sorry for no response on this issue until now. I'm not sure, but I think this issue refers to the add-on: https://vaadin.com/directory/component/iron-icons

The Icon which is included in vaadin-icons-flow should be reusable with IronIcons. I see that this is not the case however. If there is something wrong with Icon which prevents from reusing it, then a specific issue about that should be made. I suspect that Icon could be moved to another artifact that comes together with Flow, but for now it should be fine if any custom icon implementations depend on (could even use provided scope) on vaadin-icons-flow.

But as I don't see that there is anything concrete currently that we should do, I'm going to close this issue but mention about this to the author of the add-on.

[javier-godoy]

I created an issue in the addon repository regarding our use of IronIcon instead of Icon (which I understand is correct), but I think this issue is actually about Flow.

In order to use an icon which is not from Vaadin Icons, a dependency to the iconset is needed (in this case iron-icons) because Flow has only a dependency with iron-icon (i.e. the web component).

<dependency>
    <groupId>org.webjars.bowergithub.polymerelements</groupId>
    <artifactId>iron-icons</artifactId>
    <version>2.1.1</version>
</dependency>

You'll need to import the actual iconset (for instance, maps) which is provided in the webjar, though as an independent HTML, and then you can use either the (String,String) constructor in Icon or the (String,String) constructor in IronIcon (The difference between them is that Icon imports the vaadin-icons iconset, while IronIcon doesn't.)

@HtmlImport("frontend://bower_components/iron-icons/maps-icons.html")
public class MainView extends VerticalLayout  {

    public MainView() {     
        add(new Icon("maps", "directions-bike"));
        add(new IronIcon("maps", "directions-bike"));
    }

}

[pleku]

Right, thanks @javier-godoy. I admit I had completely forgotten our IronIcon bundled in the vaadin-icons-flow.

The problem with the additional collections is that those should not be added always, but as opt-in when you actually need something from the collection. That forces the application developer to remember to add the iconset dependency to the application when used.

By minimum, this should be documented into https://vaadin.com/docs/v14/flow/components/tutorial-flow-icon.html unless we can figure out a nifty way to make the iconset loading happen out-of-the-box only when used. Reopening this issue and moving it to the component integration repository.

[javier-godoy]

unless we can figure out a nifty way to make the iconset loading happen out-of-the-box only when used.

The same approach of VaadinIcon/Icon can be applied to other iconsets: for each iconset create an enumeration of its icons, provided with a create method that returns a subclass of IronIcon, which is in turn annotated with the right @HtmlImport. That way, the import is only processed if you create() an icon from that iconset. (Of course, this requires updating the enumeration when the iconset changes). Whether this is or isn't a good idea for the Vaadin framework, is a different discussion.

Anyway, in order to avoid wrong usage, iron-icon classes that import an specific iconset have a one-argument constructor. See for instance here: you can create an instance of MapsIcons.Icon, but it will be necessarily prefixed with the collection name. In PR vaadin/vaadin-icons-flow#57 I'm proposing the same restriction for Flow's Icon, since there is already IronIcon for the general case.