? ? Pending

User tests: Successful: Unsuccessful:

avatar ciar4n
ciar4n
7 Mar 2020

Pull Request for Issue # .

Summary of Changes

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.

Testing Instructions

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.

avatar ciar4n ciar4n - open - 7 Mar 2020
avatar ciar4n ciar4n - change - 7 Mar 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 7 Mar 2020
Category Modules Front End Templates (site)
avatar brianteeman
brianteeman - comment - 7 Mar 2020

My only issue with this approach is that it makes it impossible to view the code and work out which icon is being used.

avatar brianteeman
brianteeman - comment - 7 Mar 2020

scrub that - I can see it now. need new glasses

avatar ciar4n
ciar4n - comment - 7 Mar 2020

Still a valid point. Any SVG should include a class name detailing the icon for that reason (eg. svg-icon__user).

avatar dgrammatiko
dgrammatiko - comment - 7 Mar 2020

@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:

  • Render the output for icon front
  • Render the output for other fonts than the provided Font awesome
  • Add localised aria label per icon (simple str_replace)
  • do more magic
avatar ciar4n
ciar4n - comment - 7 Mar 2020

@dgrammatiko Any reason not to use something like <?php echo JLayoutHelper::render('joomla.icons.user'); ?> ? Create a layout for each icon?

avatar dgrammatiko
dgrammatiko - comment - 7 Mar 2020

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:

  • The code can sniff if the icon needs to be injected in the front end or backend, for front end it just inlines the icon (in the first instance and then uses 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
  • A more specific version of this callback could look like: <?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)
avatar ciar4n
ciar4n - comment - 7 Mar 2020

Obviously some added benefit to what you suggest. Comparable to layouts my concerns on your PR would be..

  • Can we override the icons?
  • Are we maybe over-complicating it a little by introducing Factory::getDocument into the views??
  • Are you willing to submit a PR? ?
avatar dgrammatiko
dgrammatiko - comment - 8 Mar 2020

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:

  • Lets assume we have a list view of 50 items, eg events
  • each event has a calendar icon an icon for buying a ticket and icon for location and couple more icons. Altogether there are 5 icons per item, so 250 icons in the page
  • With the layout approach we're gonna inflate the DOM by repeating the same icons 50 times. This could be a lot of wasted kb's over the wire.
  • With the HTMLHelper approach (at the state the relative PR was) we are gonna have only 5 icons and then for each instance some html like <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...

avatar ciar4n
ciar4n - comment - 9 Mar 2020

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?

avatar wilsonge
wilsonge - comment - 9 Mar 2020

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.

avatar ciar4n
ciar4n - comment - 9 Mar 2020

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.

avatar brianteeman
brianteeman - comment - 11 Mar 2020

I thought focusable="false" was only for ie11 ??

avatar brianteeman
brianteeman - comment - 11 Mar 2020

Maybe someone can convince the accessibility team to test this

avatar laoneo
laoneo - comment - 12 Mar 2020

Strongly supporting this, but not hardcoded. In DPCalendar I use this layout. It supports overrides by the template as well. Maybe you can use it for inspiration.

avatar wilsonge
wilsonge - comment - 13 Mar 2020

OK I'm happy to go with Dimitris approach as long as you are @ciar4n - you are the target base as a template developer here :) so you are king of this decision.

avatar ciar4n
ciar4n - comment - 14 Mar 2020

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.

avatar ciar4n ciar4n - close - 14 Mar 2020
avatar ciar4n ciar4n - change - 14 Mar 2020
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2020-03-14 09:52:49
Closed_By ciar4n
Labels Added: ? ?

Add a Comment

Login with GitHub to post a comment