-
Notifications
You must be signed in to change notification settings - Fork 860
[Amsterdam] Update values for border-strong-* tokens
#8794
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
packages/eui/src/themes/amsterdam/global_styling/variables/_colors.ts
Outdated
Show resolved
Hide resolved
| borderStrongAccent: computed( | ||
| ([borderBaseAccent]) => borderBaseAccent, | ||
| ['colors.borderBaseAccent'] | ||
| ([accent]) => tint(accent, 0.1), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, looking at the previous usages for border colors in Amsterdam, I'd think we should use the colors without tint. Using the brand colors directly for:
primary
accent
accentSecondary
success
warning
danger
And for neutral and risk I think we could use the severity colors directly then as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I trust you blindly on this one, will update 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in 9dd5924
05c202c to
9dd5924
Compare
packages/eui/src/themes/amsterdam/global_styling/variables/_colors.ts
Outdated
Show resolved
Hide resolved
packages/eui/src/themes/amsterdam/global_styling/variables/_colors.ts
Outdated
Show resolved
Hide resolved
💚 Build SucceededHistory
cc @acstll |
mgadewoll
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟢 Changes looking good, the output makes sense. Thanks for the update!
💚 Build Succeeded
History
cc @acstll |
Summary
In #8769 there was the need to update the color token used for the selected outline in the Data Grid, to be
border-strong-primary. In order to make this work in Amsterdam, the value for the token also had to be updated.In this PR all
border-strong-*tokens get an adequate value so they could actually useful beyond the change in the aforementioned PR.Why are we making this change?
Because it's needed for #8769 and it made sense to make a separate PR.
Screenshots
The codesandbox used to generate and "test" the colors.
The
border-strongcolor is the second in each block e.g.tint(accent, 0.1).Impact to users
Minimal to none because these tokens didn't exist previously in Amsterdam, before Borealis.
QA