refactor(loader): swap hard-coded values for design token - FE-7442#7941
refactor(loader): swap hard-coded values for design token - FE-7442#7941DipperTheDan wants to merge 4 commits into
Conversation
DipperTheDan
force-pushed
the
FE-7442_loader-token-updates
branch
from
95282ec to
1cac26e
Compare
| @@ -55,9 +55,9 @@ const time = "3s"; | |||
|
|
|||
| export const StyledStarSVG = styled.svg` | |||
There was a problem hiding this comment.
question: Just above this styled component declaration there are two key frames which are defined animateStopTop & animateStopBottom both of which use hard coded hex values.
Are there any design tokens which we can use here instead?
There was a problem hiding this comment.
Nope. On the design linked provided in the ticket, the values are hex values.
| ? "var(--progress-loader-inverse-bg-default)" | ||
| : "var(--progress-loader-bg-default)"; | ||
|
|
||
| if (variant === "ai") { |
There was a problem hiding this comment.
suggestion: within this if statement we have a ternary which renders different stops based on the inverse props boolean value.
Can we remove the secondary backup hard coded values here? Most consumers will be using our token provider for these new next components so the backup hard coded values are redundant.
There was a problem hiding this comment.
Didn't even spot that these had fallback values. I'll get them removed now.
| width: 15px; | ||
| height: ${barHeights[size]}; | ||
| border-radius: var(--borderRadius400); | ||
| border-radius: var(--global-size-s); |
There was a problem hiding this comment.
issue: for border-radius values we should use the global radius tokens opposed to the global size tokens
There was a problem hiding this comment.
Good spot. My bad for copying the wrong token value, it should be a radius token.
| height: 100%; | ||
| background: radial-gradient( | ||
| 1514.52% 80.26% at 56.89% 94.74%, | ||
| var(--mode-color-ai-alt-stop-1) 0%, |
There was a problem hiding this comment.
Waiting for Steven C to confirm stop positions.
There was a problem hiding this comment.
This will be removed when I update with master as I don't have the up to date gitignore file.
clairedenning
left a comment
clairedenning
left a comment
There was a problem hiding this comment.
May need to revisit the animation in the next ticket - TBC
4 participants
Proposed behaviour
Update hard-coded values with the latest design token values.
Current behaviour
Component is still using hard-coded values.
Checklist
d.tsfile added or updated if requiredQA
Additional context
N/A
Testing instructions