fix(fa-wildcard): converted @import to @use, removed *#7035
fix(fa-wildcard): converted @import to @use, removed *#7035mcoker merged 3 commits intopatternfly:mainfrom
Conversation
|
Preview: https://patternfly-pr-7035.surge.sh A11y report: https://patternfly-pr-7035-a11y.surge.sh |
There was a problem hiding this comment.
Unless I'm missing something, this doesn't conditionally include FA? You should be able to disable FA entirely with $pf-v6-global--disable-fontawesome: true; and include FA via the CDN links with $pf-v6-global--enable-fontawesome-cdn: true;
@mcoker think you need to expand the changed files view, the conditional hasn't changed 👍 |
|
@mattnolting this block is what used to conditionally include the FA stylesheets, but was removed in this PR - patternfly/src/patternfly/base/patternfly-fa-icons.scss Lines 31 to 33 in b11cb3e Also when I set |
1c47289 to
e9c6314
Compare
e9c6314 to
36fca91
Compare
There was a problem hiding this comment.
Two questions, but otherwise LGTM. Gonna merge and we can follow-up if needed on the questions.
| @@ -1,4 +1,5 @@ | |||
| @use '../sass-utilities' as *; | |||
| @use '../assets/fontawesome/variables' as *; | |||
| line-height: 1; | ||
| } | ||
|
|
||
| @if not($pf-v6-global--disable-fontawesome) { |
There was a problem hiding this comment.
Just a nit, but this conditional already exists in this file. Could you move the block here up to the existing conditional?
patternfly/src/patternfly/base/patternfly-fa-icons.scss
Lines 13 to 31 in c70a226
| width: #{$fa-fw-width}; | ||
| } | ||
|
|
||
| @debug $fa-css-prefix; No newline at end of file |
There was a problem hiding this comment.
Just a reminder to remove this before merging
|
🎉 This PR is included in version 6.0.0-prerelease.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
closes #6508
Backstop Report
Performance metrics
Before
After