8000 feat(DescriptionList): apply tokens by kmcfaul · Pull Request #6102 · patternfly/patternfly · GitHub
[go: up one dir, main page]

Skip to content

feat(DescriptionList): apply tokens#6102

Closed
kmcfaul wants to merge 1 commit intopatternfly:v6from
kmcfaul:descriptionlist-penta
Closed

feat(DescriptionList): apply tokens#6102
kmcfaul wants to merge 1 commit intopatternfly:v6from
kmcfaul:descriptionlist-penta

Conversation

@kmcfaul
Copy link
Contributor
@kmcfaul kmcfaul commented Dec 4, 2023

Closes #6101

@mcoker Where should I be scoping :root vs .#{$description-list}? I tried to just include the variables in root but then the preview seems to break the horizontal styling so I think I'm dividing it in the wrong place.

@patternfly-build
Copy link
Collaborator
patternfly-build commented Dec 4, 2023

@mcoker
Copy link
Contributor
mcoker commented Dec 6, 2023

@kmcfaul nice find! It's because we're effectively doing this - https://codepen.io/mcoker/pen/oNmmzQo. If you uncomment the first line and comment :root {, you'll see it works. This is something we're going to need to figure out since we are declaring vars in a parent scope, then changing vars in a child scope without redeclaring anything in the parent scope that uses the changed vars. You could leave it the way it is, or you could do something like this to retain the var scoping on the component class, but also make the vars available in the root scope for reference.

diff --git a/src/patternfly/components/DescriptionList/description-list.scss b/src/patternfly/components/DescriptionList/description-list.scss
index 845655ba6..8c19271ed 100644
--- a/src/patternfly/components/DescriptionList/description-list.scss
+++ b/src/patternfly/components/DescriptionList/description-list.scss
@@ -1,6 +1,7 @@
 // @debug $data-list; // check your variable names located in src/patternfly/sass-utilities/component-namespaces.scss
 $pf-v5-c-description-list--breakpoint-map: build-breakpoint-map("base", "sm", "md", "lg", "xl", "2xl");
 
+:root,
 .#{$description-list} {
   --#{$description-list}--RowGap: var(--pf-t--global--spacer--lg);
   --#{$description-list}--ColumnGap: var(--pf-t--global--spacer--lg);
@@ -63,7 +64,9 @@ $pf-v5-c-description-list--breakpoint-map: build-breakpoint-map("base", "sm", "m
   // Display modifiers
   --#{$description-list}--m-display-lg__description--FontSize: var(--pf-t--global--font--size--body--lg); // TODO replace with new font tokens when available
   --#{$description-list}--m-display-2xl__description--FontSize: var(--pf-t--global--font--size--heading--lg); // TODO replace with new font tokens when available
+}
 
+.#{$description-list} {
   display: grid;
   grid-template-columns: var(--#{$description-list}--GridTemplateColumns);
   row-gap: var(--#{$description-list}--RowGap);

cc @srambach @mattnolting - looks like we'll need to figure something out with this. WDYT about just scoping the vars to both :root and the component class in the short term? I wonder if we should do it for all components, or just ones where we notice problems. I'd be inclined to do it for all, then once we feel like we've got penta in a good spot, run the screenshot tool before and after updating those selectors just to :root to get an idea of what all is impacted.

@kmcfaul kmcfaul linked an issue Dec 11, 2023 that may be closed by this pull request
@kmcfaul kmcfaul force-pushed the descriptionlist-penta branch from b7a6e1e to faff368 Compare December 11, 2023 19:55
@kmcfaul kmcfaul changed the base branch from v6-old to v6 December 11, 2023 19:55
@kmcfaul kmcfaul closed this Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Description list tokens

3 participants

0