Skip to content
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 EuiToken colors #8250

Merged
merged 2 commits into from
Dec 19, 2024

Conversation

ryankeairns
Copy link
Contributor

Related to https://github.com/elastic/platform-ux-team/issues/561
Closes elastic/kibana#202760

Summary

During QA of Borealis for Discover, it was noted that the EuiToken colors were too light in the field list.
This was reviewed with the theme designers and the following recommendations were made:

  1. (this PR) adjust the colors for greater contrast in the border and icon
  2. (future) discontinue using EUI vis color palette and, instead, create a palette specific for EuiToken use

QA

(screenshots also found in the design issue)

In EUI
Running EUI docs locally, EuiToken should look like:

In Kibana
For reference, this was also run locally in Kibana which looked like:

@@ -49,7 +49,7 @@ const getTokenColor = (
: euiTheme.colors.darkShade;

const backgroundLightColor = isDarkMode
? shade(iconColor, 0.7)
? shade(iconColor, 0.9)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we change the value like this, it will affect Amsterdam colors as well.
We're in the process of adding Borealis enabled EUI packages to 8.x also where this change would apply as well then.

If we don't want Amsterdam to change, we'll need to address this conditionally.

Also fyi, this is one of the few places shade/tint are still in use since there wasn't a clear guideline on those colors yet, so it's great to hear there will be some definitions in the future 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, of course!
I'll see how it looks in Amsterdam... perhaps it is subtle enough to allow for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the token.styles.ts changes in Amsterdam, and they are subtle.

In my estimation, it seems likely to go unnoticed and, better yet, improves the contrast in Amsterdam too. I am personally comfortable with moving ahead with this minor change, as is, and foregoing the conditional check.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, the change is subtle and actually improves visibility 👍

@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

Copy link
Contributor

@mgadewoll mgadewoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM 👍

@mgadewoll mgadewoll merged commit c11b692 into eui-theme/borealis Dec 19, 2024
4 checks passed
@mgadewoll mgadewoll deleted the rk/561-EuiToken-colors branch December 19, 2024 10:34
mgadewoll added a commit to elastic/kibana that referenced this pull request Dec 19, 2024
`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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants