[4.0] Add FA svg sprite set(s) & helperclass

[N6REJ]

Pull Request for Issue # .

Summary of Changes

adds fontawesome's svg sprite sets to core.
provides a htmlhelper class to handle creating icons from sprites.

Testing Instructions

apply pr
run npm ci
check that joomla logo in admin hasn't changed looked.
inspect code notice it's now an inline svg, will fill support

Actual result BEFORE applying this Pull Request

svg images are not available in /media/vendor/fontawesome-free

Expected result AFTER applying this Pull Request

Documentation Changes Required

svg sprites are used by calling the htmlhelper "sprite" class.
HTMLHelper::_('sprite', 'icon-joomla', 'brands', '', 'Joomla version', 'Icon showing Joomla\'s current version')

This class takes 5 arguments.
string $icon, string $brand = 'solid', string $class = '', string $title = '', string $description = ''

• icon name. ( can be any name but if icon- is included then it is stripped )
• sprite set name ( brands, regular or solid ) ( 'solid' by default )
• class ( 'icon' + icon name by default )(optional)
• title ( used for tooltip and a11y ) ( optional )
• description ( used by a11y ) (optional)

It returns a string consisting of the html to display the svg as mentioned here

default svg icon styling is set in 'administrator/templates/atum/scss/blocks/_icons.scss' by

svg.icon {
  max-height: 2rem;
  max-width: 2rem;
}

then individual icons are styled like this..

// Joomla
svg.icon-joomla {
  fill: $white;
  width: 1.5rem;
  height: 1.5rem;
}

[brianteeman]

do we really want to add an additional 2000 files to the core. ftp users will love that

[HLeithner]

based on the proposal by dimitries we only need to ship the sprites files and extract the icon at runtime and include it in the body

[sandewt]

I tested successfully, but how to change te color of the star.svg ? Should a new file, for example star_color.svg, be created?

.content_rating .star {
    display: inline-block;
    width: 1em;
    height: 1em;
    content: "";
    background-image: url(star.svg);
    background-size: 110%;

[infograf768]

color:
https://stackoverflow.com/questions/22252472/how-to-change-the-color-of-an-svg-element

[dgrammatiko]

@N6REJ please do not add the sprites. PHP is a capable language to concatenate some selected icons (that's what's a sprite)...

Edit: This PR shouldn't be merged unless there is an API endpoint for using the SVGs, something like #28351 . The files alone are useless without can official way to use them...

[HLeithner]

@N6REJ please do not add the sprites. PHP is a capable language to concatenate some selected icons (that's what's a sprite)...

Edit: This PR shouldn't be merged unless there is an API endpoint for using the SVGs, something like #28351 . The files alone are useless without can official way to use them...

extracting the icons we need from the sprites on the fly (and maybe cached) is better then ship 1000 additional files.

[dgrammatiko]

extracting the icons we need from the sprites on the fly (and maybe cached) is better then ship 1000 additional files.

Providing sprites means you're loosing (or to be more accurate making it way harder) the ability to simply override any icon by placing the responsive icon in the relative template folder (eg vendors/fontawsome/solid/star.svg). Pre caching can and should happen in the template...

[HLeithner]

if you do it right you can still override this icons, why should the extract process not look for the image first?

[dgrammatiko]

why should the extract process not look for the image first?

Why over engineer something that should be straight forward? The point is to work with the established workflows instead of creating a new one, especially for something so simple 😉

[HLeithner]

You care about performance on client side, also care about performance on server and isp side....

Even if filesystems should handle many files good (because it's there job) they don't like many files, especially in one folder.
Also filetransfer protocols like brian mentioned don't like many files.

That's one of the reasons why some games created there own file "compression" format (actually without compression) because file access (read list close metadata) is a expensive.

[dgrammatiko]

You care about performance on client side, also care about performance on server and isp side

I'm not arguing with the server side caching, FWIW this was a public argument (twitter) over my proposal on the lazyloading (eg cache the attributes width and height instead of reading the images attributes in runtime, so it's kinda unfair to accuse me that I don't care of server side perf).
All I said was that the caching mechanism could (and probably should be) a concern of another part (eg Document or the template). I mean if you cache something you probably want (in the Joomla context) a GUI way to invalidate this cache at some point...

[HLeithner]

I think we talk about different things here, shipping additional 2000 files for so isn't a good option.
Anyway the PR on it's own is not useful.

[brianteeman]

To be clear about what I meant and to avoid confusion.
If you have to upload joomla to a server using ftp then adding an additional 2000 files has a massive impact on the time it takes as ftp transfers single files at a time. So there are 2000 open connections, 2000 transfers and 2000 close connections.

If you are unfortunate enough to be using windows locally then there is also a massive impact when you unzip as each file will be scanned on extraction.

Finally many hosts restrict their accounts not by disk size but by inodes. with lots of tiny files its easy to reach inode limits on some hosts.

[N6REJ]

I'm confused, you say DON"T include the sprites which are 3 files. Don't you mean don't include the svg's?

[N6REJ]

@sandewt hold off till we get this merged then we'll work on your pr if thats ok.

[ceford]

I have tested this item ✅ successfully on 8eb52e8

I had a look at a few of the SVGs - read somewhere that they are to be preferred to fonts. Looks good. We will need some documentation on how to use them.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31079.}

[sandewt]

@sandewt hold off till we get this merged then we'll work on your pr if thats ok.

I'm waiting. Although a PR for this PR can quickly be realized (with or without the improvement proposal).

[N6REJ]

I think we talk about different things here, shipping additional 2000 files for so isn't a good option.
Anyway the PR on it's own is not useful.

It's not SUPPOSED to be useful on its on.. sheeesh.. its part of what @sandewt needs for his pr and what we will need in the icon team for the transition to svg

[N6REJ]

you guys please come to a consensus or merge the pr.. one of the 2. I understand both sides of the arguments.

[N6REJ]

@sandewt hold off till we get this merged then we'll work on your pr if thats ok.

I'm waiting. Although a PR for this PR can quickly be realized (with or without the improvement proposal).

whichever you want to do, I'm trying to be helpful

[N6REJ]

@N6REJ please do not add the sprites. PHP is a capable language to concatenate some selected icons (that's what's a sprite)...

Edit: This PR shouldn't be merged unless there is an API endpoint for using the SVGs, something like #28351 . The files alone are useless without can official way to use them...

as I said to harald this is the beginning of what will be done in the icon team.

[brianteeman]

as I said to harald this is the beginning of what will be done in the icon team.

Then you should explain that in the PR. As it is right now it serves no purpose at all. Not everyone is a mind reader or a member of your private club. Without an explanation of the need for this it is just fluff.

[N6REJ]

as I said to harald this is the beginning of what will be done in the icon team.

Then you should explain that in the PR. As it is right now it serves no purpose at all. Not everyone is a mind reader or a member of your private club. Without an explanation of the need for this it is just fluff.

not a private club.. you want to participate join JBS Icon Team in glip

[brianteeman]

is its been asked for by many people

That doesn't answer my question

[N6REJ]

then ask other people please. I've given you my answer.

[N6REJ]

perhaps this will help you https://www.lambdatest.com/blog/its-2019-lets-end-the-debate-on-icon-fonts-vs-svg-icons/

[brianteeman]

then ask other people please. I've given you my answer.

some people asked for it is not an answer.

[brianteeman]

For reference - the accessibility argument is false. They can be accessible and contain useful information but generic svg such as these very rarely do.

[dgrammatiko]

For reference - the accessibility argument is false. They can be accessible and contain useful information but generic svg such as these very rarely do.

That is totally false. Icon Fonts fail miserably for people with dyslexia and there is an open issue for that: #30177

[brianteeman]

That issue is nothing to do with the use of an icon font per se

The accessibility benefit of using svg is not something that will just happen by switching to svg .

SVGs are armed with built in semantic elements – like < title > and < desc > that makes it accessible to screen reader and text browsers.

True but svg icons being added by this pr do not contain any semantic elements

Unlike icon fonts, SVGs are treated as an image and not as a text.

True but we already have the code in place to "hide" the text (although this PR proposes still using aria-hidden)

Moreover SVG is compatible with WAI-ARIA specifications – aria-labelledby attribute.

True but this PR doesn't implement that

[dgrammatiko]

True but svg icons being added by this pr do not contain any semantic elements

Because this PR is plain wrong. If the project wants to transition to SVG probably should base the work on my old PR: #28351 which was covering correctly these points (and much more)

[sandewt]

Even more information about sprites.

https://fontawesome.com/how-to-use/on-the-web/advanced/svg-sprites

[N6REJ]

SVGs are armed with built in semantic elements – like < title > and < desc > that makes it accessible to screen reader and text browsers.

True but svg icons being added by this pr do not contain any semantic elements

I didn't add it cause I didn't want to create too many params. I've done so since it's apparently important

Moreover SVG is compatible with WAI-ARIA specifications – aria-labelledby attribute.

True but this PR doesn't implement that

aria-labelledby & aria-describedby would go in the span NOT the svg unless I'm mistaken. If they do belong in the svg then does aria-labelledby provide the tooltip that title does? Each of those override "<title>" & "<desc>"
either way its now supported.

[N6REJ]

Even more information about sprites.

https://fontawesome.com/how-to-use/on-the-web/advanced/svg-sprites

I had already included that in the docs 🤣
"It returns a string consisting of the html to display the svg as mentioned here"

[N6REJ]

True but svg icons being added by this pr do not contain any semantic elements

Because this PR is plain wrong. If the project wants to transition to SVG probably should base the work on my old PR: #28351 which was covering correctly these points (and much more)

it was made abundantly clear that including 1600+ icon files would not be supported so please don't rag on my pr just because you didn't want sprites and don't like me/my work.

Instead of you & brain complaining perhaps you could explain what would make it better and more importantly how/why. It's clear I'm not going to make both of you happy so I'm not even going to try unless the above criteria is applied.

@brianteeman ty for explaining what was wrong with the a11y part of it. I wondered about that but since you know far more about a11y then I do and hadn't mentioned it, I wasn't aware that was the issue.

[dgrammatiko]

it was made abundantly clear that including 1600+ icon files would not be supported so please don't rag on my pr just because you didn't want sprites and don't like me/my work

My pr was approved already by the project leader for j4, so although everybody is entitled to an opinion in many cases that opinion is irrelevant, in sort the number of files is irrelevant. Also the way you’re doing it here has very little advantages over the font, you still deliver whatever is given by font awesome (which is a lot of things that a user never asked for) so I’ll stick on what I said: the pr is wrong

[dgrammatiko]

Instead of you & brain complaining perhaps you could explain what would make it better and more importantly how/why

Fork my pr and then ask Ciaran and Allon for help

[N6REJ]

Also the way you’re doing it here has very little advantages over the font, you still deliver whatever is given by font awesome (which is a lot of things that a user never asked for) so I’ll stick on what I said: the pr is wrong

I don't disagree that it's simply extending fa to another icon type(?)
My understanding is J5 will be 100% FA so there's that. Changing my work to NON-FA only shouldn't be that hard ( one line ) when/if the time comes.

[dgrammatiko]

I don't disagree that it's simply extending fa to another icon type(?)

That’s not what I meant. Right now a page with only a user icon needs around 100kb of crap, although it only uses one icon... you didn’t fix that and that’s the performance part that needs fixing

[N6REJ]

I don't disagree that it's simply extending fa to another icon type(?)

That’s not what I meant. Right now a page with only a user icon needs around 100kb of crap, although it only uses one icon... you didn’t fix that and that’s the performance part that needs fixing

ok, & that is probably above my knowledge set. I do the best I can with the knowledge that I have. I'm almost always open to being taught a better way but I won't follow blindly. I need to understand the code I write if I can possibly do so.

[brianteeman]

I think you are getting a bit confused.

<span aria-hidden="true"><?php echo HTMLHelper::_('sprite', 'icon-joomla', 'brands', '', 'Joomla version', 'Icon showing Joomla\'s current version'); ?></span>

Joomla Version is a title
Icon showing Joomla's current version is a description

The purpose of the title and the description is to provide information about the svg image
Think of the <title> as the alt text of the svg and <desc> as the longer description of the svg

The desc is never visually displayed and the title may be visually displayed as a tooltip (depending on the browser settings)

They are both however exposed to the accessibility api of the browser which is great and means that the text can be announced by a screen reader.

However in this code you have wrapped the svg inside a span with aria-hidden=true. That code makes everything inside the span hidden from the accessibility api. Which completely negates all your work adding the title and desc

[the reason the icon fonts are wrapped in that span is because they are fonts and therefore would be recognised as text by a screen reader which would announce garbage]

Final comment is that of course you need to make sure the text is translatable so you need to use language constants and pass them through JText

[brianteeman]

Oops forgot to add that we also have to consider if there is any benefit to the icon being described or is it just decorative.

If it is just decorative then you probably dont want to have it announced.

Think of it like the current markup.

1. an icon and visually displayed text.
   In this example you would probably not want the svg title as the svg is purely decorative

2. an icon and visually hidden text (class=sr-only).
   In this example you would probably want the svg title and can remove the hidden text

[N6REJ]

@brianteeman

[the reason the icon fonts are wrapped in that span is because they are fonts and therefore would be recognised as text by a screen reader which would announce garbage]
considering svgs are treated as images by the browser thats probably not what we want then.

do you have a suggestion on how to make it work well with a11y?

[brianteeman]

There is no one size fits all solution

Based on the current code base I would say that everywhere we have text in an sr-only span that can be deleted and used as the title element of the svg and you remove the aria-hidden on the svg
but everywhere that the text is not hidden then you keep the aria-hidden and dont add a title.

quick examples below - its late so I havent tested the code but its just there as an illustration

Example 1

<div class="mr-2">
	<span class="icon-wifi" aria-hidden="true"></span>
	Blog Sample Data	
</div>

So here you would put

<div class="mr-2">
	<span aria-hidden="true"><?php echo HTMLHelper::_('sprite', 'icon-wifi', 'solid'); ?></span>
	Blog Sample Data	
</div>

Example 2

<button type="button" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false" class="btn" id="dropdownMenuButton-87">
	<span class="icon-cog" aria-hidden="true"></span>
	<span class="sr-only">Edit the 'Sample Data' module</span>
</button>

So here you would put

<button type="button" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false" class="btn" id="dropdownMenuButton-87">
	<?php echo HTMLHelper::_('sprite', 'icon-wifi', 'solid', '','Edit the 'Sample Data' module'); ?>
</button>

[brianteeman]

As stated privately - the small performance benefits do not justify the amount of work involved in making this change throughout the codebase

[N6REJ]

As I know your well aware its a crap ton of work... already a yr in the making...
considering what your saying perhaps the best thing to do, at least for now, is have the ability in place ( that work is basically already done ) but NOT start changing things till PLT says do it.

[N6REJ]

So, Had a productive conversation in JBS @ 0300 regarding this pr and it was pretty much decided that without being able to have individual svg icons like webfonts or images that it's not practical to use svg in this way.
As brian stated, converting to all svg would be a minimum 3 mo venture due to the enormous amount of work that would be involved.
IF one icon from each icon set ( solid, brands, regular ) were loaded it would result in 2.8mb of files being downloaded for as little as 3 icons.
Thats not practical. So for that reason I'm closing this pr and advising icon team that we're done going down this rabbit hole. If someone creates a PLT accepted way of handing svg icon SETS we'll go back at it.

[dgrammatiko]

If someone creates a PLT accepted way of handing svg icon SETS we'll go back at it.

There was an accepted way as demoed in #28351 The reason that I closed that PR was because there wasn't any common ground on the implementation of the helper (jLayout, or HTMLHelper, or JDocument function). I've told you already this on our private talk like months ago but you didn't (and it seems you still don't want to) listen...