Appuniversum icon improvements

[Windvis]

alternative to the symbolset setup [Released as experimental]

Read more

At the moment Appuniversum bundles a list of icons into a svg-symbols file which is also stored in the public folder and which is then referenced in AuIcon component.

This works, but has some downsides:

• no support for custom icons, which means icons need to be added here, even if they are very specific for a single project
• All the icons are bundled, even if only a few are used by a project
• If the app is embedded in a different domain, the browser might refuse to load the referenced symbolset, so something like svgxuse needs to be used as a workaround.

A potential solution for these issues is to use icon components, instead of the string based setup we have now. Each .svg file could be converted to a .gts file at build time, and imported like any other component.

The benefits:

• Since they're components, Embroidered apps will strip out the unused ones so only the used icons will be part of the build
• No reference to a symbolset file, so no issues with cross-domain requests

For this to work, each component that now accepts a string for the @icon argument, would also need to support a component (POC in #481 ).

We would also need to refactor our internal usage of the AuIcon component, and pass the components instead of the string.

TODO:

• Add support for passing components into the @icon arguments ([Experimental] Add support for using components as icons #481)
• Add a script that generates icon components from the .svg files ([Experimental] Add icon components #487)
• Use the icon components internally (Use the icon components internally #486)
• Release this work as an experimental feature, so apps can start experimenting with it (Released in v3.4.0)

Open questions:

• Do we want to deprecate the string based system? If we don't any usage of the symbolset will trigger the download resulting in all the icons being downloaded again)
• Do we want to expose these components in Ember's global component namespace, or do we only expose the export paths and require that consumers import where needed? (In non-gjs/gts files this might be cumbersome, but I think we're at a point where using .gjs is in a good place)

Webuniversum icon inconsistencies

Webuniversum has a lot of icons. Not all of them were added here, and we might also be using some of the legacy ones.

We should compare all the icons we bundle here with the Webuniversum list and document if it is a custom component, or a legacy one.

TODO:

• Sync our icons with the Webuniversum ones
• Release the icon component feature as a stable feature once the Webuniversum sync is complete

Custom icons

If an icon is not part of the Webuniversum list, we should check were it is being used. If it isn't used, or if a Webuniversum equivalent with a different name exists, we should deprecate it (or otherwise override it).

Legacy icons

We should deprecate the legacy icons and suggest the new equivalent.

Sync all the icons

We should also add all the icons that Webuniversum currently has so they can easily be used without new PRs. Assuming we only support the new icons in the component setup this shouldn't have an impact.

[piemonkey]

If the app is embedded in a different domain, the browser might refuse to load the referenced symbolset, so something like svgxuse needs to be used as a workaround.

In fact, while the svgxuse workaround does work if you're hosting the ember app that is embedded in another domain (for example in an iframe), as svgxuse is still able to request the symbolset. If the app is designed to be bundled within another project, then the symbolset is no longer available and this approach fails. This is the case for the embeddable-say-editor project which can now be installed through npm and in order to work requires overriding AuIcon with a custom version. This workaround only works for ember-appuniversum <=2.15.

[abeforgit]

@Windvis do you have an idea of when this could land/how much work it would be? Just to get an idea for our planning. @piemonkey what is your intuition on how much work this would be to do ourselves?

[Windvis]

@abeforgit The first part could be done fairly quickly, if I can't get time for it this sprint I can probably get time for it in the next one. @piemonkey did the groundwork for the component based icons, and I have a WIP PR that generates icon components from the .svg files. Once both are merged we can use these components internally.

I think we should release this as an experimental feature that apps can opt-into though. I don't feel confident enough about this setup to consider it stable, and I might want to change some things which might be breaking, without having to cut a major just for that.

The icon inconsistencies are not super high prio, so that can be done later since there is quite a bit of grunt work involved.

So current plan:

• land the component generation PR (optional, since the embeddable could still use svg-jar, but I think this setup is nicer)
• land the "pass icons components as args" pr (Done)
• use these components internally, instead of the strings
• roll out the feature as an experimental feature which might still receive breaking changes in the v3 major

The embeddable could then update to v3+ and test it out. I'll try to test it in one of the projects I work on as well. If all the kinks are resolved we can release it as a stable feature and deprecate the string setup.

[Windvis]

@piemonkey @abeforgit The icon component setup is released as an experimental feature in 3.4.0.

I don't foresee any huge breaking changes in the future. I think the API itself is fine. There might be some breaking changes once we sync the shipped icons with those from Webuniversum, but that will most likely be limited to the removal (in case of duplicates) / renaming of some icons, so should be easy to fix.

[Windvis]

First issue when trying some of the icon components:

Some icons have a fill="none" attribute which makes the icon invisible. For example the visible icon (the irony..).

The symbolset version doesn't have the same issue because the library we use strips out all the fill attributes (among others).

We should probably just fix the icons here, and maybe lint them in some way? I'm not the biggest fan of stripping them magically since it just hides the issue and there must be a reason why the attribute was added.

As a workaround you can inline the icon in a new component in your project, remove the fill and use that until the issue is resolved in the Appuniversum version.

This is something we have to keep in mind when we sync the new icons.

[piemonkey]

Ha, funny, I found one of these (link-broken) and was about to put in a PR with the fill removed from it.

I definitely think stripping the fill from the svg is the way to go. Not sure how best to go about linting them though.

[Windvis]

I definitely think stripping the fill from the svg is the way to go. Not sure how best to go about linting them though.

I think stripping it from the source is the way to go, but I don't like the 'let's do it during the build step' method, at least for now. I don't know enough about svg or why design programs output this attribute when exporting the svg to be sure we can simply strip it in all cases 😄. On the other hand, that's how we were already using the icons before, so it probably would be fine (for the icons we already have).

I still think updating the .svg is better though. Maybe we should just check the presence of fill="none" in the build script and throw an error?

[Windvis]

@piemonkey Should be fixed by #488. It solved the issue I noticed at least 👍.

[Windvis]

Small update, I'm not 100% convinced that our current "icons as components" implementation is the best way forward. Ideally we would still output a svg map which can be loaded separately and referenced so we don't bloat the js bundle with icons.

I'll need to investigate this some more.

[abeforgit]

Wouldn't only the used svgs get pulled in in a (bright future) embroider setup?