Replace old custom versions of FontAwesome with new FontAwesome 6 via our own CDN

[djanelle-mit]

Developer

Previously, we had been using FontAwesome (a custom version in the 1-2 range) that we'd added directly to the repository. There are also some references from the bootstrap CDN that needed to be removed. We're now delivering FontAwesome 6 via our own CDN to avoid privacy concerns.

This PR includes the replacing of old versions of FA with the new one, as well as updating the old class="icon- syntax with the new class="fa fa- syntax.

In a few cases, the new icons were slightly larger or smaller than the previous version, so CSS updates were made to bring closer to parity with production.

I also took this opportunity to update the Alert on the location page with the new style of alert we implemented earlier this year.

The relevant ticket for this work is https://mitlibraries.atlassian.net/browse/PW-113

Stakeholder approval

• Stakeholder approval has been confirmed
• Stakeholder approval is not needed

Code Reviewer

• The commit message is clear and follows our guidelines
  (not just this pull request message)
• The changes have been verified
• The documentation has been updated or is unnecessary
• New dependencies are appropriate or there were no changes

[djanelle-mit]

I had to squash 5-6 commits to make this (hopefully) easier to understand, if there's a way to make these easier to review, would love that feedback!

[JPrevost]

Let's promote fontawesome to the prod CDN and update this before merging. I'll reach out on slack to schedule time to show you that process.

[djanelle-mit]

Updated the paths to the latest (6.7.1) version of FontAwesome on our CDN. Pushed changes to PW-113 multidev, too.

[djanelle-mit]

Ready for review! Paths point to the Production CDN now and appear to be working on local and multidev.

[matt-bernhardt]

Hey Dave, this is a partial review, I haven't looked at the full changeset, but noticed a few URLs that look like there could be some loose ends somewhere. I've tried to track down some details to make the hunting easier.

---

The first URL I noticed is /akdc/about-akdcmit/news-events/, which on the review app renders for me like this:

Other URLs that look like they might be broken:

• /exhibits/ (both in the sidebar and the "view all exhibits" button at the bottom of the page)
• /music-oral-history/interviewees/ (icons linking to interview posts in the main content table)
• Other sidebar widgets on child sites look like they're broken in the same way as the Exhibits site sidebar is - like /docs/ or /music-oral-history/about-the-project/.

Most of these come from the child theme's style.css, which doesn't look like has any changes in this PR. I see similar uses in the parent theme styles in the accordion and sidebar partials, but the one accordion I could trigger seemed like it might still work.

My question about all of this is whether the intent here is to abandon the practice of loading FA icons outside of the supported classes? If that's the case, it looks like there's a variety of style rules that would need to be removed here. If we don't mind that legacy implmentation in theory, then the question becomes to confirm that those styles still work or whether they need updates.

I've dug a bit more into the existing implementation while debugging this, but rather than dump a bunch of details that may not be needed I'll end here with this summary of my review so far:

1. It looks like some URLs (above) are broken in the review app
2. Asking whether we intend to continue supporting FA icons implemented without FA class names.

[djanelle-mit]

Thanks Matt! I think I see the fix, so I'll get things set and tag when a new review makes sense.

It's probably going to be handy to load FA outside of the classes, so I think fixing that will make the most sense. I needed to do it for the Alerts work to make sure that we can adjust the icon via CSS. We may just want to make sure that we use the classes in markup first, and only lean to the CSS solution when it's necessary.

I think the old version had the font-family specified as FontAwesome where the new version requires font-family: 'Font Awesome 6 Free'; as the syntax. Swapping this out and making sure the weight is either 400 for regular or 900 for solid should do the trick.

We could adjust the Font Awesome files on the CDN to use the FontAwesome family name, but that would require us making that fix each time we update the version on the CDN and that seems like something that would easily be forgotten. I'm leaning towards just making the fix in the CSS to reflect the new family name, but curious if you have any initial thoughts before I dig in?

[matt-bernhardt]

I agree with your instinct, to just specify Font Awesome 6 Free as the font-family. This will also make this PR easier to review, because I know any thing that still uses the old font name will be broken / should be removed. The alternative feels like we'd be signing up for backfilling in order to prevent keeping our own code current, which doesn't feel like a winning strategy.

[djanelle-mit]

@matt-bernhardt I think I found all the tendrils. I replaced each instance I found via searching, and was able to trace them back to the specific pages. I also had to add the brand file for the AKDC site that has the social links in the sidebar, so that's a new addition.

[matt-bernhardt]

I think this looks good - thanks for updating for those few pages I'd noted, and for catching that the brands package was needed on the AKDC site. I've looked over the pages affected by this changeset, and everything seems like it works. I'm not aware of anything that isn't yet updated (famous last words, perhaps, but hopefully not).

Overall, this looks like a significant improvement over our current implementation, and finally makes use of the theme inheritance in a way that makes things much cleaner! Thanks for all this work.

Gotta love a PR that removes 17,000 blocks while only adding 82.

EDIT: I see the CodeClimate flag about variable names, but those are all existing issues - not introduced here. Our use of variable names has not always been aligned with WordPress' standards, and a PR like this is not the place to fix that "issue".

[matt-bernhardt]

Is there an active alert somewhere on the multidev? I don't see one on the locations page or the homepage at the moment, which is where I know to look for these at the moment.

[matt-bernhardt]

I think I was able to trip these rules by adding a new alert to my local - so this isn't a blocking comment.