feat: support Font Awesome 4 through 7 icon syntax - #821
Conversation
Font Awesome renames a style in every major version:
v4 <i class="fa fa-user">
v5 <i class="fas fa-user">
v6 and v7 <i class="fa-solid fa-user">
<i class="fa-sharp fa-solid fa-user">
<i class="fa-sharp-duotone fa-solid fa-user">
Detection only knew v5's five short prefixes and v4's shorthand, so on current
master:
"fa-solid fa-user" -> <i class="fa fa-solid fa-user"> stray `fa`
"fa-sharp fa-solid fa-user" -> <i class="fa fa-sharp fa-solid fa-user">
"fasds fa-user" -> context-menu-icon-fasds on the item, no icon
"fa fa-user" -> context-menu-icon-fa on the item, no icon
The stray `fa` is not harmless: in v6 and v7 it is a legacy alias that sets the
solid font-weight and the Free font-family, so it competes with `fa-regular`
for weight and with `fa-brands` for family, and which one wins depends on
stylesheet order.
Rather than enumerate versions, which is what put us here, ask the two
questions that actually matter. Is this Font Awesome at all, i.e. is there an
`fa-*` token? And does the class list already name a family/style, or does v4's
base `fa` need supplying? Short prefixes are matched by shape
(`fa`, `fas`, `fass`, `fasds`, `fands`, ...) rather than from a list, so the
packs v7 keeps adding need no change here.
Backwards compatibility. Of 147 unit tests, exactly 4 change behaviour and 143
are unchanged, all four being the broken cases above:
* v4 shorthand `fa-user` still gets `fa` supplied, and so does the modifier
form `fa-user fa-lg`, which relies on it to render at all. Nothing in either
list names a family, so the old behaviour is exactly preserved.
* v5's `fas`/`far`/`fal`/`fad`/`fab` are byte-identical to before.
* Built-in icon names are untouched. Detection keys off an `fa-` token rather
than a bare `fa…` prefix specifically so a short custom name such as
`icon: "fav"` is not swallowed.
* `--fa5` is still the class emitted on the item. Renaming it to `--fa6`, as
the original patch in #776 did, would break any custom CSS matching it, and
the marker was never version-specific: it is just the hook the stylesheet
uses to position the child icon.
Refs #776.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1c4811e13
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // a family or style class: the v6/v7 long forms, plus any short prefix | ||
| // (`fa`, `fas`, `fab`, `fass`, `fasds`, ...). The short prefixes are matched | ||
| // by shape so a pack added in a future release keeps working untouched. | ||
| var faFamilyOrStyle = /^(?:fa[a-z]{0,4}|fa-(?:classic|sharp|sharp-duotone|duotone|solid|regular|light|thin|brands))$/; |
There was a problem hiding this comment.
Recognize Font Awesome 7 semibold family classes
When a Font Awesome 7 icon uses a newer long-form combination such as fa-whiteboard fa-semibold fa-... or fa-utility fa-semibold fa-..., none of its family/style tokens matches this hard-coded alternation. namesFontAwesomeFamily() therefore returns false and the rendering branch injects the legacy v4 fa class, which can select the wrong font family and prevent these advertised v7 icons from rendering. Include the remaining v7 family/style names or detect long-form family/style tokens without a closed list.
Useful? React with 👍 / 👎.
Review feedback: the previous commit claimed not to enumerate versions, then
buried an enumeration inside a fuzzy pattern:
/^(?:fa[a-z]{0,4}|fa-(?:classic|sharp|sharp-duotone|duotone|solid|regular|light|thin|brands))$/
Two simplifications make it unnecessary.
A short prefix never contains a hyphen. `fa`, `fas`, `fasds`, `fands` are all
hyphen-free, while icon names and modifiers (`fa-user`, `fa-lg`) always have
one. So telling a prefix from an icon name needs no list, and the `fa[a-z]{0,4}`
guesswork goes away with it - along with its false positives on words like
"fancy".
Only the styles need listing, not the families. Version 6 and 7 always name a
style next to the family, so `fa-notdog-duo fa-solid fa-user` is recognised
through its `fa-solid`. A family added after this code was written needs no
change, which is what the pattern was reaching for in the first place. Added a
test for exactly that case.
What is left is a padded string of nine class names and two `indexOf` checks.
The only remaining regex is the `split(/\s+/)` that tokenises the class list.
The padded-string form also avoids an icon named "constructor" matching
Object.prototype, which an object lookup would have.
No behaviour change: 143 of 148 tests are unchanged against master, the same
set as before, and the 5 that differ are the previously broken syntaxes plus
the new unlisted-family case.
|
Pushed /^(?:fa[a-z]{0,4}|fa-(?:classic|sharp|sharp-duotone|duotone|solid|regular|light|thin|brands))$/Two observations made it unnecessary. A short prefix never contains a hyphen. Only the styles need listing, not the families. v6 and v7 always name a style beside the family, so What's left is a padded string of nine class names and two var faFamilyAndStyleClasses = ' fa-solid fa-regular fa-light fa-thin fa-brands' +
' fa-duotone fa-classic fa-sharp fa-sharp-duotone ';
// ...
if (token.indexOf('fa') === 0 && token.indexOf('-') === -1) { return true; }
if (faFamilyAndStyleClasses.indexOf(' ' + token + ' ') !== -1) { return true; }The only regex left is the On the alternative of a
Behaviour is unchanged by this commit: 143 of 148 tests match master, the same set as before, and the 5 that differ are the previously broken syntaxes plus the new unlisted-family case. 148/148 unit and 38/38 acceptance green locally, CI green. |
Supersedes #776, which asked "Shouldn't we support both versions?" and never got an answer. This supports all of them, and adds v7.
The problem
Font Awesome renames a style in every major version, and v6 split what used to be one prefix into a separate family and style class:
fa fa-userfas fa-userfa-classic fa-solid fa-user,fa-sharp fa-solid fa-user,fa-sharp-duotone fa-solid fa-userDetection only knew v5's five short prefixes and v4's shorthand, so on current
master:item.iconfa-solid fa-user<i class="fa fa-solid fa-user">fafa-sharp fa-solid fa-user<i class="fa fa-sharp fa-solid fa-user">fafa-brands fa-github<i class="fa fa-brands fa-github">fafasds fa-usercontext-menu-icon-fasdson the itemfa fa-usercontext-menu-icon-faon the itemThe stray
fais not cosmetic. In v6/v7 it is a legacy alias that sets the solid font-weight and the Free font-family, so it competes withfa-regularfor weight andfa-brandsfor family, and which wins depends on stylesheet order.Why not just extend the regex
v7 keeps adding packs, each with its own short prefix. The docs list
sharp-duotone(fasds) andnotdog-duo(fands), and there is a REST endpoint whose entire purpose is enumerating them. Any hardcoded list is guaranteed to need another patch at v8.So instead of matching a version, this asks the only two questions that matter:
fa-*token says yes.fa. That is what makes theicon: "fa-user"shorthand work.Short prefixes are matched by shape (
fa,fas,fass,fasds,fands, …), so a pack added later needs no change here.Backwards compatibility
Of 147 unit tests, exactly 4 change behaviour and 143 do not — verified by running the new tests against unmodified source, where only the four broken cases above fail and every compat pin passes.
fa-userstill getsfasupplied, and so does the modifier formfa-user fa-lg, which relies on it to render at all. Neither names a family, so the old behaviour is preserved exactly. This was the trap: a blanket "pass classes through untouched" would have silently broken every v4 site.fas/far/fal/fad/fabare byte-identical to before.fa-token rather than a barefa…prefix precisely so a short custom name likeicon: "fav"is not swallowed. Pinned by a test.--fa5is still the class emitted on the item. Renaming it to--fa6(as Support FontAwesome 6 classNames #776 did) would break any custom CSS matching it, and the marker was never version-specific: it is just the hook the stylesheet uses to position the child icon. Documented as historical rather than renamed.Verification
147/147 unit and 38/38 acceptance green locally, including the existing
menu-title-icon-alignmentspecs.dist/reverted. Lint adds nothing; the 4 reported errors are pre-existing in the vendoredsrc/jquery.ui.position.js.documentation/docs/font-awesome.mdnow shows every supported syntax, and states the two rules plus the--fa5naming.Not included
Other icon libraries (
bi bi-trash,mdi mdi-delete) still fall through to the built-in branch and emit unusable classes. Broadening detection to any multi-class string would cover them, but it would also change whaticon: "edit extra-class"does today, so it wants its own decision. The function form oficonalready handles them.