chore(pf): updated @import to @use#6538
Merged
mcoker merged 4 commits intopatternfly:v6from Apr 17, 2024
Merged
Conversation
Collaborator
|
Preview: https://patternfly-pr-6538.surge.sh A11y report: https://patternfly-pr-6538-a11y.surge.sh |
mcoker
requested changes
Apr 9, 2024
Contributor
There was a problem hiding this comment.
NICE! Just a couple of nits I noticed looking at the code. Also when I run the dist build, it's taking about twice as long (~40s compared to ~20s before), any idea what's causing that?
12dff31 to
666bee3
Compare
mcoker
requested changes
Apr 17, 2024
Contributor
There was a problem hiding this comment.
ran the visual regression tool and found a few things that need to be updated:
.fa-fwstill referencesdiv(), so that class isn't working. That can probably just usecalc()like the update you made to the FA-stackclass- The whole
base/tokensdir stylesheets are being included in patternfly-base, and that contains the chart tokens. For the time being anyways, we don't want to include the chart styles in PF by default. That could change, but let's stick with the way it's setup now and there are no chart global vars in patternfly.css _variables.scss/patternfly-variables.scsswas removed, and I think it should be added back for a couple of reasons- We still have global vars that are not tokens and that would be a great place to include them. It's the 2 blocks that reference
pf-v6-set-inverse()from that removed file - we'll need those back. They're a var that is1in LTR and-1in RTL that we use with things liketranslate()on the x-axis to use a negative value in RTL sincetranslate()doesn't have logical position support.- We have an RTL var that was removed in this update (that was in
_variables.scss) that is breaking some RTL styles.
- We have an RTL var that was removed in this update (that was in
- React-tokens (and maybe other react scripts) parse this stylesheet and unless there is a strong business case to remove this file, I'd like to remove the barrier for needing devs to figure out that part of react-tokens before being able to bump to this version of core.
- We still have global vars that are not tokens and that would be a great place to include them. It's the 2 blocks that reference
- patternfly-no-globals.scss is empty.
FWIW, these are the updates I made locally to get the visual regression job to pass.
diff --git a/src/patternfly/assets/fontawesome/_variables.scss b/src/patternfly/assets/fontawesome/_variables.scss
index 020d11782..0b94bf439 100644
--- a/src/patternfly/assets/fontawesome/_variables.scss
+++ b/src/patternfly/assets/fontawesome/_variables.scss
@@ -9,7 +9,7 @@ $fa-version: "5.6.3" !default;
$fa-border-color: #eee !default;
$fa-inverse: #fff !default;
$fa-li-width: 2em !default;
-$fa-fw-width: div(20em, 16);
+$fa-fw-width: calc(20em / 16);
// Convenience function used to set content property
@function fa-content($fa-var) {
diff --git a/src/patternfly/base/_index.scss b/src/patternfly/base/_index.scss
index 6d231e9e1..f071145ef 100644
--- a/src/patternfly/base/_index.scss
+++ b/src/patternfly/base/_index.scss
@@ -7,4 +7,4 @@
@forward './patternfly-fa-icons';
@forward './patternfly-pf-icons';
@forward './patternfly-svg-icons';
-@forward './tokens';
+@forward './patternfly-variables';
diff --git a/src/patternfly/patternfly-no-globals.scss b/src/patternfly/patternfly-no-globals.scss
index 9a20d1d97..5cbccdee4 100644
--- a/src/patternfly/patternfly-no-globals.scss
+++ b/src/patternfly/patternfly-no-globals.scss
@@ -2,3 +2,4 @@
$pf-v6-global--enable-reset: false,
$pf-v6-global--enable-normalize: false
);
+@use './patternfly' as *;With the contents of patternfly-variables.scss of:
@use '../sass-utilities' as *;
@use "./tokens/workspace-overrides" as *;
@use "./tokens/tokens-font" as *;
@use "./tokens/tokens-palette" as *;
@use "./tokens/tokens-default" as *;
@use "./tokens/tokens-dark" as *;
:where(:root) {
@include pf-v6-set-inverse(false);
}
@include pf-v6-rtl {
@include pf-v6-set-inverse;
}666bee3 to
e3218d6
Compare
This was referenced Apr 17, 2024
Closed
srambach
pushed a commit
to srambach/patternfly
that referenced
this pull request
Apr 22, 2024
Collaborator
|
🎉 This PR is included in version 6.0.0-alpha.120 🎉 The release is available on: Your semantic-release bot 📦🚀 |
mattnolting
added a commit
to mattnolting/patternfly
that referenced
this pull request
Jun 21, 2024
dgdavid
added a commit
to agama-project/agama
that referenced
this pull request
Dec 12, 2024
All of them comming from Patternfly dependency, most probably becuase of the change in latest sass-loader release for using the modern Sass JS API, see webpack/sass-loader@10be1ba In any case, it is just silencing two deprecations, - "import" > @import rules are deprecated and will be removed in Dart Sass 3.0.0. https://sass-lang.com/documentation/js-api/interfaces/deprecations/#import NOTE: already fixed in Patternfly v6, see patternfly/patternfly#6538 - "global-builtin" > Global built-in functions are deprecated and will be removed in Dart Sass 3.0.0. https://sass-lang.com/documentation/js-api/interfaces/deprecations/#global_builtin To know more, see https://sass-lang.com/documentation/breaking-changes/import/
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #6518
closes #6524