Skip to content

Comments

[4.0] Change to use FontAwesome 5#24648

Merged
wilsonge merged 6 commits intojoomla:4.0-devfrom
brianteeman:fa-5
Apr 21, 2019
Merged

[4.0] Change to use FontAwesome 5#24648
wilsonge merged 6 commits intojoomla:4.0-devfrom
brianteeman:fa-5

Conversation

@brianteeman
Copy link
Contributor

@brianteeman brianteeman commented Apr 20, 2019

This is quite a big change and will need a composer and npm install to test

Many of the icons have changed name with FA5 see https://fontawesome.com/how-to-use/on-the-web/setup/upgrading-from-version-4#name-changes

The icomoon mapping has been updated with the name changes

Where possible I have updated any icons in core to the new names but I may have missed some.

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Apr 20, 2019
@Hackwar
Copy link
Member

Hackwar commented Apr 20, 2019

Thank you for doing all this work. There is a CSS to keep on using FA4 names with FA5. Should we use this to keep the changes smaller? I also really, REALLY don't like that we can't use the Joomla icon in our own CMS from that icon font. Should we contact them and ask them to move it to the free group?

@mbabker
Copy link
Contributor

mbabker commented Apr 20, 2019

I also really, REALLY don't like that we can't use the Joomla icon in our own CMS from that icon font.

Umm, why? I updated https://github.com/joomla/framework.joomla.org to use FA5 a while ago and use the Joomla icon there without issue, you just have to make sure you're loading the brand icons (none of which are in the pro group).

@brianteeman
Copy link
Contributor Author

I must have misread the terms of the brand file.

As for the v4 to v5 shim @wilsonge correctly pointed out that joomla has never shipped a release with fa4

@brianteeman
Copy link
Contributor Author

I will fix to include the brand if requested but for the sake of one icon I would prefer a different solution with less weight

@wilsonge
Copy link
Contributor

For the Joomla brand icon let's create our own class that loads the logo.svg in the template https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/templates/atum/images/logo.svg

@brianteeman
Copy link
Contributor Author

yes that's what i was thinking. that should be done in its own pr

@wilsonge
Copy link
Contributor

The other option thinking about it is to load the brand icons in the backend template only. For things like the social media classes might be useful.

@brianteeman
Copy link
Contributor Author

if the brand icons are included then it should be in both

…s/tree/item.vue

Co-Authored-By: brianteeman <brian@teeman.net>
@wilsonge
Copy link
Contributor

Personally I don't think that's right. We can be more tight with the resources loaded in the frontend than the backend

@brianteeman
Copy link
Contributor Author

The use case for social media icons its much greater in the front end

@mbabker
Copy link
Contributor

mbabker commented Apr 20, 2019

If anything it's more common to load brand icons in the frontend than it is in the backend.

@dgrammatiko
Copy link
Contributor

@wilsonge according to your decision ( joomla/40-backend-template#441 (comment) ) the front end shouldn't have a dependency on FA. I hope you didn't change your mind on this.

PS. Also for this case to stand true all of the form fields should be font awesome free, as the fields are common to both sides frontend -backend...

@brianteeman
Copy link
Contributor Author

I have added brands and updated the markup where necessary to use it (fa => fab)
It is a separate commit to can easily be removed

@richard67
Copy link
Member

richard67 commented Apr 21, 2019

@brianteeman Shouldn't there be a change "fa" => "fas" for those icons in the standard package, e.g. change "fa fa-pen-square" to "fas fa-pen-square"? See https://fontawesome.com/icons/pen-square?style=solid

That means for Fontawesome 5, the "fa" is in general changed to a three letter class where the last letter is the subpackage where it belongs to, i.e. "b" for brand, "s" for solid, "l" for light and "r" for regular.

@brianteeman
Copy link
Contributor Author

no there is no need for that. FA5 supports both fa and fas

@richard67
Copy link
Member

Hmm, their icon pages don't show that.

@brianteeman
Copy link
Contributor Author

@richard67
Copy link
Member

I see: "fas or fa = Font Awesome Solid". Thanks for the info. Seems I missed that in past.

@brianteeman
Copy link
Contributor Author

@richard67 and of course I followed best practice and checked that it worked before I submitted the PR

@richard67
Copy link
Member

@brianteeman I never assumed something else. But as we are all humans only, we all can make mistakes ;-)

@wilsonge wilsonge merged commit 99dec42 into joomla:4.0-dev Apr 21, 2019
@wilsonge
Copy link
Contributor

wilsonge commented Apr 21, 2019

That'll do for now :) We can definitely work on how we optimise things in frontend. But this is now just a straight version upgrade so we don't have to worry about that here

Thanks!

@wilsonge wilsonge added this to the Joomla 4.0 milestone Apr 21, 2019
@brianteeman
Copy link
Contributor Author

Thanks - learnt a lot writing this pr

@Scrabble96
Copy link
Contributor

It's a shame, but fontawesome 4.7 fonts like file-o, which used to be white/transparent with a colour outline, have now been classed as 'light' in v 5.0 - and are now Pro, so no longer free.

In Joomla 3x, in the template editor directory tree, for example, the file icon is light and easily distinguishable at a glance from from folders:

J3-white-icons

In Joomla 4.0 dev they are all solid:
j4-solid-icons

See pics of the different fontawesome options:

4.7 free:
fontawesome-4 7-file-o

5.0 free
fontawesome-5 0-free-file

5.0 Pro
fontawesome-5 0-pro-file

Is there a case for keeping both versions?

@ghost
Copy link

ghost commented Aug 17, 2019

@Scrabble96 can you please open an new Issue (plus link to this) as Comments on closed one didn't get much notice.

@Scrabble96
Copy link
Contributor

@Scrabble96 can you please open an new Issue (plus link to this) as Comments on closed one didn't get much notice.

Hi, @franz-wohlkoenig. Thanks, I have just done so on issue #25915.

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

Labels

NPM Resource Changed This Pull Request can't be tested by Patchtester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants