Skip to content
This repository was archived by the owner on Apr 19, 2019. It is now read-only.

Comments

[WIP] SVG Icons#441

Closed
dgrammatiko wants to merge 5 commits intojoomla:masterfrom
dgrammatiko:svg-icons
Closed

[WIP] SVG Icons#441
dgrammatiko wants to merge 5 commits intojoomla:masterfrom
dgrammatiko:svg-icons

Conversation

@dgrammatiko
Copy link
Contributor

Pull Request for Issue # .

Summary of Changes

Bye bye font-awesome font, welcome font-awesome svgs

Testing Instructions

screen shot 2018-06-05 at 16 40 49

Documentation Changes Required

@brianteeman
Copy link
Contributor

chrome_2018-06-05_15-53-42

chrome_2018-06-05_15-55-17

@brianteeman
Copy link
Contributor

@dgrammatiko
Copy link
Contributor Author

@brianteeman what browser are you using here?

@brianteeman
Copy link
Contributor

Chrome on Windows

PS are you sure this works in IE?

@dgrammatiko
Copy link
Contributor Author

IE is very picky on svgs but there is a way to have the same icons rendered without too much hustle. But before we get to that point there are couple things that needs to be resolved first:

  • Right now icons are an array in the document, I guess this needs to be either in HTMLHelper or a Library?
  • Outputing the html in the template can be done either the way I'm doing it here, (unlikely) or introducing a new <jdoc:include type="icons" /> (or something similar)
  • Getting the icons from the FA repo and adding the specific IDs (plus some more inline attributes to make IE11 happy, stroke="currentColor" etc)

@mbabker @wilsonge can you point me to the right direction for the first 2 issues?

@dgrammatiko
Copy link
Contributor Author

@brianteeman oh by the way I figured out why you're not getting the icons, your instance is in a subdirectory...

@mbabker
Copy link
Contributor

mbabker commented Jun 5, 2018

@dgrammatiko life would be so much easier if we mandated Joomla can only run at root of public_html 🤣

@brianteeman
Copy link
Contributor

Or if @dgrammatiko developed in a sub directory because he always makes the same mistake

@brianteeman
Copy link
Contributor

and would it not be better to use the svg sprites instead of individual svg?

@dgrammatiko
Copy link
Contributor Author

Sprites packaged by font awesome will get us back to where we were with the font. The idea here is:

  • decouple from font awesome
  • able to override an icon in a similar fashion to scripts and stylesheets
  • Joomla is doing the packaging here (ok we're not really packaging things here but that might be a milestone 2, if we want to go this way)

@brianteeman
Copy link
Contributor

Not going to ask what is wrong with being coupled to the font

Not sure if we can repackage the fonts under the licence - you would need to check that

@dgrammatiko
Copy link
Contributor Author

@brianteeman as always I have no clue what's the compatibility of licenses but according to their repo the license is

- Icons — CC BY 4.0 License
     In the Font Awesome Free download, the CC BY 4.0 license applies to all icons packaged as .svg and .js files types.

@brianteeman
Copy link
Contributor

ah - they';ve removed the restriction about bundling and packaging

@brianteeman
Copy link
Contributor

still no change for me :(

@brianteeman
Copy link
Contributor

Still no change :(

// Generate the file and load the stylesheet link
foreach ($this->icons as $key => $icon)
{
$file = HTMLHelper::image('vendor/font-awesome/' . $icon . '.svg', '', null,true, 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We made something similar in DPCalendar for J3. While doing that, an idea popped up, to first search in the template for an svg and then in vendor. Like that a template can override the core icons.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactly what is happening here as well thanks to the magic of HTMLHelper::image

if ($file) {
$matches = [];
preg_match('$(\/media.+)$', $file, $matches);
$files[] = @file_get_contents(JPATH_ROOT . $matches[1]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed it does. Nice one.

If you want, you can do this as an oneliner $files[] = file_get_contents(JPATH_ROOT . substr($file, strpos($file, '/media')));

Added the same technique now in DPCalendar.

@wilsonge
Copy link
Contributor

After our 30 minute group skype discussion I'm going to close this.

@wilsonge wilsonge closed this Jun 27, 2018
@wilsonge
Copy link
Contributor

Discussions outcomes:

  1. Font awesome coupling in the backend
  2. Inline SVGs in the frontend templates where required. Possibly coupling to font awesome on a per view basis (the backend template)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants