8000 fix(layouts): applied tokens by mcoker · Pull Request #6453 · patternfly/patternfly · GitHub
[go: up one dir, main page]

Skip to content

fix(layouts): applied tokens#6453

Merged
mcoker merged 4 commits intopatternfly:v6from
mcoker:issue-6316
Mar 26, 2024
Merged

fix(layouts): applied tokens#6453
mcoker merged 4 commits intopatternfly:v6from
mcoker:issue-6316

Conversation

@mcoker
Copy link
Contributor
@mcoker mcoker commented Mar 21, 2024

fixes #6316

@patternfly-build
Copy link
Collaborator
patternfly-build commented Mar 21, 2024

@mcoker mcoker requested a review from srambach March 21, 2024 23:41
Copy link
Member
@srambach srambach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see some mixture of v5 and v6 in the examples and docs of flex, grid, and gallery. Do you want to change that now or catch it in the upcoming PR for all the v5s?

.ws-core-l-bullseye .pf-v6-l-bullseye,
.ws-core-l-bullseye .pf-v6-l-bullseye__item {
padding: .5rem;
border: 2px dashed #393f44;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the borders for the examples, do you want to use a token so that it works better in dark theme? Maybe also the border width just because?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep! Superb suggestion.

@@ -10,7 +10,7 @@
@each $property, $property-value in $pf-v5-global--spacer-properties-map {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to go ahead and change this to v6 here and on line 295 of scss-variables.scss?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heck yes!

@mcoker
Copy link
Contributor Author
mcoker commented Mar 22, 2024

@srambach I'll make that update in this PR 👍👍

Copy link
Member
@srambach srambach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The examples look beautiful with the tokens applied!

Copy link
Contributor
@thatblindgeye thatblindgeye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple comments below, nothing blocking here though

.#{$flex} {
// Emit spacer css variables that map to requested spacer values
@include pf-v5-emit-properties($pf-v5-l-flex--variable-map);
8000 @include pf-v5-emit-properties($pf-v6-l-flex--variable-map);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a TODO regarding updating mixins used in these layouts to a v6 prefix?

$pf-v5-l-flex--spacer-map: build-spacer-map();
$pf-v5-l-flex--gap-map: build-spacer-map("base", "none", "xs", "sm", "md", "lg", "xl", "2xl", "3xl", "4xl");
$pf-v5-l-flex--variable-map: build-variable-map("#{$pf-prefix}l-flex--spacer", $pf-v5-l-flex--spacer-map);
$pf-v6-l-flex--breakpoint-map: build-breakpoint-map();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker for this, but would we be able to use the pf-prefix instead of hardcoding the version number in these files

@mcoker mcoker merged commit 6b0ea7d into patternfly:v6 Mar 26, 2024
@patternfly-build
Copy link
Collaborator

🎉 This PR is included in version 6.0.0-alpha.107 🎉

The release is available on:

Your semantic-release bot 📦🚀

@kmcfaul kmcfaul linked an issue Mar 27, 2024 that may be closed by this pull request
7 tasks
@mcoker mcoker deleted the issue-6316 branch January 7, 2025 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Layouts - apply tokens

4 participants

0