8000 feat(Spinner): consumed Penta tokens by thatblindgeye · Pull Request #6154 · patternfly/patternfly · GitHub
[go: up one dir, main page]

Skip to content

feat(Spinner): consumed Penta tokens#6154

Merged
mcoker merged 1 commit intopatternfly:v6from
thatblindgeye:spinnerPenta
Dec 21, 2023
Merged

feat(Spinner): consumed Penta tokens#6154
mcoker merged 1 commit intopatternfly:v6from
thatblindgeye:spinnerPenta

Conversation

@thatblindgeye
Copy link
Contributor

Work towards #5738

@thatblindgeye thatblindgeye requested review from a team, andrew-ronaldson, lboehling, mattnolting and srambach and removed request for a team December 18, 2023 13:33
--#{$spinner}--m-sm--diameter: var(--pf-t--global--icon--size--md);
--#{$spinner}--m-md--diameter: var(--pf-t--global--icon--size--lg);
--#{$spinner}--m-lg--diameter: var(--pf-t--global--icon--size--xl);
--#{$spinner}--m-xl--diameter: var(--pf-t--global--icon--size--2xl);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we still need this modifier class since this is the default size of a spinner?

Copy link
Member

Choose a reason for hiding this comment

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

I lean to the side of leaving the modifier for completeness but I wouldn't fight about it.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO the "inline" spinner should be the default, then you would opt-in to a size variation. It seems odd that the default spinner is so large.

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

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.

🍭

--#{$spinner}--Width: var(--#{$spinner}--diameter);
--#{$spinner}--Height: var(--#{$spinner}--diameter);
--#{$spinner}--Color: var(--#{$pf-global}--primary-color--100);
--#{$spinner}--Color: var(--pf-t--global--icon--color--brand--default);
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like the right value but in figma it's defined as the border--color--brand token. cc @lboehling @andrew-ronaldson do you know which is right?

--#{$spinner}--m-sm--diameter: var(--pf-t--global--icon--size--md);
--#{$spinner}--m-md--diameter: var(--pf-t--global--icon--size--lg);
--#{$spinner}--m-lg--diameter: var(--pf-t--global--icon--size--xl);
--#{$spinner}--m-xl--diameter: var(--pf-t--global--icon--size--2xl);
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO the "inline" spinner should be the default, then you would opt-in to a size variation. It seems odd that the default spinner is so large.

@mcoker mcoker merged commit 5c7fe64 into patternfly:v6 Dec 21, 2023
@patternfly-build
Copy link
Collaborator

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

The release is available on:

Your semantic-release bot 📦🚀

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.

5 participants

0