Spinner: VR color variants #3912
Conversation
✅ Deploy Preview for gestalt ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for gestalt ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -59,6 +59,16 @@ svg * { | |||
| calc(var(--g-enter-delay) + 300ms); | |||
| } | |||
|
|
|||
| .spinner.grayscale circle { | |||
| animation-name: scale, none, dot-intro; | |||
| fill: var(--comp-spinner-grayscale-background); | |||
There was a problem hiding this comment.
I'm tempted to say that we should use a single token (comp-spinner-background) and ovverride the value of the token in the react component. Instead of introducing one-off component tokens.
When we do have better theming support, we can use a theme provider to provide those values
There was a problem hiding this comment.
@rlingineni can you elaborate? I thought having component tokens in this kind of situations are expected and encouraged. 🤔
this kind of situations => where we have multiple color variants for a component.
Should iOS and Android should do the same in these situations? Where should the token values be read from while setting them from the component?
There was a problem hiding this comment.
Ideally - because the function is the same for these component tokens I was thinking it'd be like this:
You've already written the color tokens like this:
-comp-spinner-color-background-1
-comp-spinner-color-background-2
-comp-spinner-color-background-3
So I would think you would override these token values, instead of making new ones.
In general for these components with component variants,
the ideal would be like this:
comp.json
-comp-spinner-color-background-1: Color1
-comp-spinner-color-background-2: Color2
-comp-spinner-color-background-3: Color3
comp-light-variant.json
-comp-spinner-color-background-1: Color1
-comp-spinner-color-background-2: Color1
-comp-spinner-color-background-3: Color1
And the provider would allow you to pull from the right variant. Right now, we're not sure on the structure of how to do these, or the provider.
So what I'm proposing is to put this "theme" override in JS in the react component.
So even applying override classes like this could probably work:
.darkVariant{
-comp-spinner-color-background-1: Color1
-comp-spinner-color-background-2: Color1
-comp-spinner-color-background-3: Color1
}
.lightVariant{
-comp-spinner-color-background-1: Color2
-comp-spinner-color-background-2: Color2
-comp-spinner-color-background-3: Color2
}
That way - themes represent values changing.
On iOS and Android, I thought the preference was to use Lottie? They also need some level of sub-theming support.
Alternatively, you can keep the current token you made, and use that to set the value of the other tokens - in case we think iOS and Android would need it. But if they need a component token, they can also add it in iOS.json or android.json files to have them.
There was a problem hiding this comment.
On iOS and Android, I thought the preference was to use Lottie?
yes but my question was in general
Summary
Added new color variants for VR Spinner.
color="white"
color="grayscale"
Links
Checklist