User tests: Successful: Unsuccessful:
Pull Request for Issue # .
The intent here would be to remove the FontAwesome dependency from frontend views, replacing all icons with inline SVGs. I'll finish if agreeable. Currently only applied to login module.
Apply this patch and run node build.js --compile-css
for updating the changed SCSS. Check login module in the frontend. Check it displays correctly.
Status | New | ⇒ | Pending |
Category | ⇒ | Modules Front End Templates (site) |
scrub that - I can see it now. need new glasses
Still a valid point. Any SVG should include a class name detailing the icon for that reason (eg. svg-icon__user
).
@ciar4n this is great. My only complain here is why Joomla decided not to give some sensible API for frontenders to do such commonly used patterns? I mean it's not that nobody ever proposed such thing: joomla/40-backend-template#441
PS. Looking at that PR with a couple more flags it could:
@dgrammatiko Any reason not to use something like <?php echo JLayoutHelper::render('joomla.icons.user'); ?>
? Create a layout for each icon?
It doesn't have to be a layout per se although it's possible (I think this complicates things a bit more than what it should be). In the PR I mentioned in my comment above the actual code for including an icon (as inline svg) was something like <?php echo Factory::getDocument()->setIcons('lock', 'demo-class'); ?>
. Also on my previous comment I said that this could have few more flags, so let me explain:
xref-link
or whatever is that code so it doesn't inflate unproportionally the document). If it sniffs that is referring to back end just adds the span as it is right now in most places in the codebase and also takes care to load the appropriate font/css<?php echo Factory::getDocument()->setIcons(['font-awesome', 'lock'], ['class' => 'demo-class', 'aria-label' => 'A lock icon bla bla...' ...More options that might apply to either the font icon or the inline svg ]); ?>
. In this case the code could do specifically more magic for the devs that will end up using it (fix the accessibility, properly attach unique ids for inline svgs, etc, the list can expand to fill all possible front end devs needs)Obviously some added benefit to what you suggest. Comparable to layouts my concerns on your PR would be..
Factory::getDocument
into the views??Can we override the icons?
Sure, in the well known way that css or js can be overridden: add a file in the img/vendor/font-awesome/svg
folder in the template and voila...
Are we maybe over-complicating it a little by introducing Factory::getDocument into the views
It's not any different than <?php echo JLayoutHelper::render('joomla.icons.user'); ?>
and although you can still pass any params to the layout and get the exactly same functionality as the HTMLHelper there is a crucial difference: efficiency. Let me explain this with a naive example:
<svg><use xlink:href="#someId" /></svg>
. Also with this approach we assure that the id's are unique, repeating templates can fall sort here as well.Are you willing to submit a PR?
Is the project willing to move to 2020? If not then there is no good reason for me to spend my time again on a lost cause...
Ok.. got ya.
In a scenario that you describe where icons are heavily repeated inside a loop, I guess it is still possible to use layouts. The icon layout will need the option to add an ID and to hide it. The view can then import the icon once and import that icon within the loop using <use>
.
<?php echo JLayoutHelper::render('joomla.icon.myicon', array('id' => 'myicon', 'hidden' => 'true')); ?>
<?php for ( .. ) { ?>
<svg>
<use href="#myicon" />
</svg>
<?php } ?>
There is a decision to be made on what is the best route... @wilsonge @HLeithner thoughts?
In all the component views JDocument is available with $this->document
anyhow https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/MVC/View/AbstractView.php#L33 (not the case in JLayouts
but we can probably work that out )
Is the project willing to move to 2020? If not then there is no good reason for me to spend my time again on a lost cause...
If you look when I closed that PR we agreed we'd use SVGs in the frontend anyhow. So it never got rejected - just no one ever worked on that PR after we chatted through next steps ;) joomla/40-backend-template#441 (comment)
Just FWIW this will still only be for the frontend template - I'm still just fine with font awesome coupling in the backend where we are prioritising ease for developers over performance (obviously not to extreme's but to a degree).
Honestly overall though I'm in two minds. I can see the advantages of what Dimitris is proposing (especially for 3rd party devs) but I'm also slightly concerned it's another layer of complexity in the template just to render an icon and Ciaran's stuff does make that much easier to use - but at the price of reuseability.
In some ways as a template dev @ciar4n i'm interested for you to tell me which you prefer because your the person who's most representative of the people affected by this.
Much of a much really. Layouts might be more familiar to some and already commonly used throughout the views however it is also a little strange having a PHP file for an SVG. Icons are considered 'assets' so maybe it makes more sense to treat them as such which is what @dgrammatiko has done.
I thought focusable="false" was only for ie11 ??
Maybe someone can convince the accessibility team to test this
Dimitris's PR works for me. I'll close this PR for now and maybe, @dgrammatiko can resubmit his. Failing that I think Allons suggestion is the optimal layouts solution. It gets around the strangeness of having the SVG inside a PHP file.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-03-14 09:52:49 |
Closed_By | ⇒ | ciar4n | |
Labels |
Added:
?
?
|
My only issue with this approach is that it makes it impossible to view the code and work out which icon is being used.