[4.0][Frontend][RFC] Inline SVGs instead of FA#28262
[4.0][Frontend][RFC] Inline SVGs instead of FA#28262ciar4n wants to merge 1 commit intojoomla:4.0-devfrom
Conversation
|
|
|
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. |
|
@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 |
|
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
|
|
Obviously some added benefit to what you suggest. Comparable to layouts my concerns on your PR would be..
|
Sure, in the well known way that css or js can be overridden: add a file in the
It's not any different than
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 <?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
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 |
|
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. |
|
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. |
|
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. |
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-cssfor updating the changed SCSS. Check login module in the frontend. Check it displays correctly.