-
Notifications
You must be signed in to change notification settings - Fork 842
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
[Visual Refresh] Add behindText vis color tokens #8253
base: eui-theme/borealis
Are you sure you want to change the base?
[Visual Refresh] Add behindText vis color tokens #8253
Conversation
755606d
to
7528314
Compare
Preview staging links for this PR:
|
💚 Build Succeeded
|
@@ -266,6 +266,27 @@ export type _EuiThemeVisColors = { | |||
euiColorVis8: string; | |||
euiColorVis9: string; | |||
|
|||
/* deprecated - temp token; used only during theme migration */ |
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.
non-blocking suggestion: How about we use the JSDoc @deprecated annotation for a strikethrough highlighting?
@@ -62,6 +62,17 @@ export const colorVis: _EuiThemeVisColors = { | |||
euiColorVis8: euiPaletteColorBlind.euiColorVis8.graphic, | |||
euiColorVis9: euiPaletteColorBlind.euiColorVis9.graphic, | |||
|
|||
euiColorVisBehindText0: '#6dccb1', |
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.
non-blocking suggestion: I understand that this is temporary but for better discoverability, consistency and clarity, can we add them in packages/eui/src/themes/amsterdam/global_styling/variables/_colors_vis.ts
and use as ☝🏻 instead of hard-coding here? We could copy-paste them from packages/eui/src/themes/amsterdam/global_styling/variables/_colors_vis.scss
.
We could also add a comment above them, similar to temp token; used only during theme migration
.
Summary
Note
This PR merges into a feature branch.
This PR adds
_behindText
vis color tokens. These have not been part of the theme, as they are not in usage in the new theme.During upgrades to the new theme, we realized that it's rather cumbersome for teams to migrate these JSON token usages properly to the
euiTheme
tokens without a 1:1 equivalent.These tokens are temporary for the purpose of making migration easier for Kibana teams. We will need to remove those tokens again at some point as they are not be used in Borealis.
QA
These tokens are not used anywhere. Make sure the values make sense:
Amsterdam - uses values returned from
euiPaletteColorBlindBehindText()
Borealis maps to default vis colors
CI passes