-
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] Update color token mappings #8211
[Visual Refresh] Update color token mappings #8211
Conversation
- this was a mistake as those colors are borealis specific and don't exist in amsterdam
…rectation label - the CSS hook is useful to prevent having to define variant styles per component
- ensures docs example work as expected for both themes
"euiColorVis7_behindText": "#FFC0B8", | ||
"euiColorVis8_behindText": "#E6AB01", | ||
"euiColorVis9_behindText": "#FCD279", | ||
"euiColorVis0": "#16C5C0", |
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.
Are these colors for the data visualization? If so, where can I see the mapping to test against?
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.
These JSON files are legacy files that used to be generated from the Sass files but now only exist until Kibana migrated to use our theme tokens via EuiProvider instead.
The theme vis color tokens are defined here.
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.
Thanks, Lene! 💚 What I wanted to know is what is the purpose of euiColorVis*
tokens. I checked in Kibana and it does seem they are predominantly used in data visualization (charts).
And is there a design definition of the mapping anywhere, so that I can verify that e.g. euiColorVis0 -> #16C5C0
is the correct mapping?
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.
Sorry, yes. vis colors are visualization colors.
And is there a design definition of the mapping anywhere, so that I can verify that e.g. euiColorVis0 -> #16C5C0 is the correct mapping?
Yes and no. There is this Figma file but when implementing the tokens I was told by @gvnmagni that it's not up to date and he provided the expected mappings 🫠
The same is mirrored in the Figma variables in this file, I'm not sure if those are the latest state though? @gvnmagni
ℹ️ There is also still ongoing work on parts of the vis color palettes for severity (issue)
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.
@mgadewoll my eyes burn after so much HEX BUT they all look okay 👌🏻 Feel free to resolve or wait for @gvnmagni confirmation 😄
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 think we can proceed also without @gvnmagni - in case there is a need to update something, we can do that separately any time 🙂
packages/eui-theme-borealis/src/variables/colors/_colors_dark.ts
Outdated
Show resolved
Hide resolved
packages/eui-theme-borealis/src/variables/colors/_colors_dark.ts
Outdated
Show resolved
Hide resolved
packages/eui-theme-borealis/src/variables/colors/_semantic_colors.ts
Outdated
Show resolved
Hide resolved
packages/eui-theme-borealis/src/variables/colors/_semantic_colors.ts
Outdated
Show resolved
Hide resolved
packages/eui-theme-borealis/src/variables/colors/_semantic_colors.scss
Outdated
Show resolved
Hide resolved
packages/website/docs/components/tabular_content/data_grid_style_and_display.mdx
Show resolved
Hide resolved
packages/eui/src/themes/amsterdam/global_styling/variables/_colors.ts
Outdated
Show resolved
Hide resolved
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'm really sorry @mgadewoll for checking this PR so slowly 🙏🏻 I've been going over the changes since yesterday, little by little. And thank you for granular commits with meaningful messages, it really helped to review more efficiently! For the future, we might want to break down the task into smaller chunks after all.
Amazing work on the changes 🎉
Testing notes:
❌ we have to update the color mapping in a couple of places (see comments for details)
✅ all components are correct: EuiTourFooter, EuiCodeBlock, EuiCommentEvent, EuiFlyoutFooter, EuiDataGrid, EuiBeacon, EuiBadge, EuiBetaBadge
✅ Sass Colors page updates correctly when changing the color mode (light / dark) (compared to other staging env)
- utilizes chroma for conversion of hex to rgb; as this runs only once on initial theme built, the impact of the calculation should be no problem
Preview staging links for this PR:
|
💚 Build Succeeded
History
|
`v98.1.0-borealis.0`⏩`v98.2.1-borealis.2` _[Questions? Please see our Kibana upgrade FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_ --- # `@elastic/eui` ## [`v98.2.1`](https://github.com/elastic/eui/releases/v98.2.1) - Updated the EUI theme color values to use a full 6 char hex code format ([#8244](elastic/eui#8244)) ## [`v98.2.0`](https://github.com/elastic/eui/releases/v98.2.0) - Added two new icons: `contrast` and `contrastHigh` ([#8216](elastic/eui#8216)) - Updated `EuiDataGrid` content to have a transparent background. ([#8220](elastic/eui#8220)) **Accessibility** - When the tooltips components (`EuiTooltip`, `EuiIconTip`) are used inside components that handle the Escape key (like `EuiFlyout` or `EuiModal`), pressing the Escape key will now only close the tooltip and not the entire wrapping component. ([#8140](elastic/eui#8140)) - Improved the accessibility of `EuiCodeBlock`s by ([#8195](elastic/eui#8195)) - adding screen reader only labels - adding `role="dialog"` on in fullscreen mode - ensuring focus is returned on closing fullscreen mode # Borealis updates - [Visual Refresh] Update color token mappings ([#8211](elastic/eui#8211)) - [Visual Refresh] Introduce shared popover arrow styles to Borealis ([#8212](elastic/eui#8212)) - [Visual Refresh] Add forms.maxWidth token ([#8221](elastic/eui#8221)) - [Visual Refresh] Set darker shade for subdued text ([#8224](elastic/eui#8224)) - [Visual Refresh] Remove support for accentSecondary badges ([#8224](elastic/eui#8227)) - [Visual Refresh] Add severity vis colors ([#8247](elastic/eui#8247)) - [Visual Refresh] Fix transparent color variable definitions ([8249](elastic/eui#8249)) - [Visual Refresh] Update EuiToken colors ([8250](elastic/eui#8250)) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Summary
closes https://github.com/elastic/eui-private/issues/147
Note
This PR merges into a feature branch.
This PR updates color token mappings according to design changes (figma).
Changes
color token changes
text
/textParagraph
andtitle
/textHeading
color mappingsbackgroundBaseHighlighted
color tokenbackgroundBaseInteractiveSelect
mappingsuccess
to use coloraccentSecondary
accent
to useaccent
instead ofaccentText
for Borealisdefault
EuiBadge background mappingadditional
QA
check updated color values with Figma definitions
component specific QA:
backgroundBaseHighlighted
background (shade10
/shade135
)backgroundBaseHighlighted
backgroundhighlighted
variant uses usesbackgroundBaseHighlighted
background ( Amsterdam usessubdued
mapping)backgroundBaseHighlighted
backgroundBaseInteractiveSelect
backgrounds (primary10
/primary130
)success
usesaccentSecondary
colordefault
has ashade20
/shade130
backgroundaccent
usesaccent90
instead ofaccent100
for Borealisverify EUI docs Sass variables update as expected when changing color mode (docs)
General checklist
Checked in mobileChecked for accessibility including keyboard-only and screenreader modesAdded documentationProps have proper autodocs (using@default
if default values are missing) and playground togglesChecked Code Sandbox works for any docs examplesUpdated visual regression testsIf applicable, file an issue to update EUI's Figma library with any corresponding UI changes. (This is an internal repo, if you are external to Elastic, ask a maintainer to submit this request)