Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More recent version of 5.x #7

Closed
laryn opened this issue Sep 1, 2020 · 11 comments · Fixed by #11
Closed

More recent version of 5.x #7

laryn opened this issue Sep 1, 2020 · 11 comments · Fixed by #11

Comments

@laryn
Copy link
Member

laryn commented Sep 1, 2020

I was troubleshooting why a certain icon wasn't appearing despite it being there on the Font Awesome site -- turns out it was added in a later version than what this module calls.

laryn added a commit that referenced this issue Sep 1, 2020
@laryn
Copy link
Member Author

laryn commented Sep 1, 2020

It looks like the most recent version is 5.14.0.

PR here: #8

@ghost
Copy link

ghost commented Dec 16, 2020

I'm concerned that breaking changes between the versions might be an issue for people if we just change the version automatically. https://github.com/FortAwesome/Font-Awesome/blob/master/UPGRADING.md

Maybe we need to add an option to more specifically choose the version, so peop,e using the older one stay on that, while new sites get the latest...?

@ghost ghost added the enhancement label Dec 16, 2020
@herbdool
Copy link
Contributor

herbdool commented Mar 3, 2021

@BWPanda as per your comment, I've added a PR which adds the latest as an option. I figure each could be updated so long as there aren't breaking changes.

A couple other thoughts: I think some of the font awesome changes are retroactive, such as where they remove icons or move them to the premium. So even in v5.2 things might be broken in some cases.

And I'm wondering if there's a URL (at use.fontawesome.com) to use the latest release automatically. Perhaps that could be an option for some people?

@ghost
Copy link

ghost commented Mar 4, 2021

I've added a PR which adds the latest as an option. I figure each could be updated so long as there aren't breaking changes.

Thanks @herbdool! I'd suggest that instead of calling v5.2 "v5", we make the latest version "v5" and name other ones after their version number. For example:

'v5.2' => t('<a href="https://fontawesome.com/how-to-use/on-the-web/referencing-icons/basic-use" target="_blank">v5.2</a>'),
'v5 (latest)' => t('<a href="https://fontawesome.com/how-to-use/on-the-web/referencing-icons/basic-use" target="_blank">v5.15</a>'),

We can update the URL for 'v5 (latest)' as long as there are no breaking changes. As soon as there are, we make a new 'v5.15' and then 'v5 (latest)' continues to track the latest version. Does that make sense? Is it a good idea?

So even in v5.2 things might be broken in some cases.

Well we can't really help that I think...

And I'm wondering if there's a URL (at use.fontawesome.com) to use the latest release automatically.

Not that I know of. It was hard enough to find these URLs, as they seem to be wanting users to pay to use their CDN option...

Everytime I look at FontAwesome, I keep wondering whether https://forkaweso.me/Fork-Awesome/ wouldn't be better to use instead... But that'd be a separate module, since I'm sure there are users who still prefer the icons available from FontAwesome, despite the fact that it's getting less open-source and more proprietary...

@herbdool
Copy link
Contributor

herbdool commented Mar 4, 2021

I left it as v5 because it makes it backwards compatible for those who have selected it. Otherwise you'd need to update their selection in an update hook from v5 to v5.2.

Fork Awesome: interesting, looks good too. You might be able to add it as an option, no? Or does that get too complicated?

@ghost
Copy link

ghost commented Mar 4, 2021

Oops, I was confusing keys and values... Here's what I meant instead:

'v5' => t('<a href="https://fontawesome.com/how-to-use/on-the-web/referencing-icons/basic-use" target="_blank">v5.2</a>'),
'v5.15' => t('<a href="https://fontawesome.com/how-to-use/on-the-web/referencing-icons/basic-use" target="_blank">v5 (latest)</a>'),

Or something like that.

Or does that get too complicated?

Yeah, I don't think users wanting to use ForkAwesome icons would think to install the FontAwesome module... We'd either need to rename this module to be more generic (then you could select where your icons come from), or make a new module for ForkAwesome.

@herbdool
Copy link
Contributor

herbdool commented Mar 4, 2021

Or make the key more generic: "v5-latest" so it's clear that it will always have the latest and people don't have to change the setting.

--
Fork Awesome is functionally the same so it would be a shame to duplicate the effort. It wouldn't be that strange to include it as an option here. Maybe make it explicit in the readme.

@laryn
Copy link
Member Author

laryn commented Mar 4, 2021

I had never heard of Fork Awesome -- but I wouldn't be opposed to making it an option in this module, fwiw. In fact I'd probably use it most (or all) of the time now that I know of it...

@herbdool
Copy link
Contributor

herbdool commented Mar 4, 2021

It might be possible to have it load both Font Awesome and Fork Awesome in case someone wants to use icons from both libraries. Or is that crazy.

@herbdool
Copy link
Contributor

herbdool commented Mar 4, 2021

Oh, it's probably crazy. I just realized that even though .fa is deprecated in FA v5, it is still setting it via the css.

@yorkshire-pudding
Copy link
Collaborator

I came across this as I had the same problem. It's a bit beyond my current technical expertise, but here is an idea for how could handle the releases (other than manually adding each one):

Anyhow, that's a future idea for someone who knows how to extract the release from the markdown. Possibly I'll figure it out, but not yet.

I've tested the PR and it looks good.

@ghost ghost closed this as completed in #11 Aug 23, 2022
ghost pushed a commit that referenced this issue Aug 23, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants