[EuiSteps] Remove chroma transparency warning, misc cleanup by cee-chen · Pull Request #7454 · elastic/eui

cee-chen · 2024-01-09T19:01:16Z

Summary

Several Kibana devs have reported this chroma.js warning for disabled EuiSteps:

Unfortunately, it looks like #7032 did not fully resolve the console warnings. The usage is coming from here:

eui/src/components/steps/step_number.styles.ts

Lines 72 to 74 in a107059

    
                 color: ${makeHighContrastColor(euiTheme.colors.disabledText)( 
        
                   euiButtonFillColor(euiThemeContext, 'disabled').backgroundColor 
        
                 )};

euiButtonFillColor(euiThemeContext, 'disabled').backgroundColor is a color with alpha transparency on it, so we cannot call makeHighContrastColor() on it without this warning.

I've opted to revert disabled step numbers to use the same text & background colors as euiButtonFillColor() entirely, while leaving the step title the same higher contrast color. IMO, this is significantly DRYer approach to take - if we need to increase color contrast, we should do so on buttons as well as steps.

Before	After

This PR also DRYs out/cleans up some other CSS, which can be viewed by commit.

QA

https://eui.elastic.co/pr_7454/#/navigation/steps#horizontal-steps
Confirm no console errors about contrast and transparency in dev tools
Confirm zero axe browser plugin violations for low color contrast

General checklist

Browser QA
- Checked in both light and dark modes
- Checked in Chrome, Safari, Edge, and Firefox
- Checked for accessibility including keyboard-only and screenreader modes
  - [ ] Checked in mobile
Docs site QA - N/A
Code quality checklist - N/A, CSS only changes
Release checklist
- A changelog entry exists and is marked appropriately.
  ~~- [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)~~
Designer checklist
- Updated the Figma library counterpart

- It's creating a `chroma.js` warning that devs are concerned about - we should really just be reusing the default EuiButton colors anyway, and tweaking those if they have color contrast issues, so that multiple places at once get the updates

- remove multiple `euiButtonFillColor()` calls in favor of just one

- remove CSS nesting - replace hardcoded 2px with token

cee-chen · 2024-01-09T19:09:49Z

@1Copenut do you mind weighing in on this since you approved #7032? IMO, the step number is less important in terms of contrast than the step title/text. If we still feel it needs higher contrast, I think we should do so in the euiButtonFillColor and propagate that to all buttons instead of just steps.

kibanamachine · 2024-01-09T19:33:08Z

Preview staging links for this PR:

Docs site: https://eui.elastic.co/pr_7454/
Storybook: https://eui.elastic.co/pr_7454/storybook

elasticmachine · 2024-01-09T19:33:12Z

💚 Build Succeeded

Buildkite Build
Commit: 6b80f3c

1Copenut

👍 LGTM! I took a look at the view in light and dark themes and ran the axe browser plugin. Zero issues or console output. Contrast ratio on the gray text was 4.78:1 and 5:26(?):1 on light and dark themes respectively.

Clean refactor @cee-chen!

cee-chen · 2024-01-09T22:08:46Z

Thanks Trevor!

`v91.3.1`⏩`v92.0.0-backport.0` --- ## [`v92.0.0-backport.0`](https://github.com/elastic/eui/releases/v92.0.0-backport.0) **This is a backport release only intended for use by Kibana.** **Bug fixes** - Fixed an `EuiTreeView` JSX Typescript error ([#7452](elastic/eui#7452)) - Fixed a color console warning being generated by disabled `EuiStep`s ([#7454](elastic/eui#7454)) ## [`v92.0.0`](https://github.com/elastic/eui/releases/v92.0.0) - Updated generic types of `EuiBasicTable`, `EuiInMemoryTable` and `EuiSearchBar.Query.execute` to add `extends object` constraint ([#7340](elastic/eui#7340)) - This change should have no impact on your applications since the updated types only affect properties that exclusively accept object values. - Added a new `EuiFlyoutResizable` component ([#7439](elastic/eui#7439)) - Updated `EuiTextArea` to accept `isClearable` and `icon` as props ([#7449](elastic/eui#7449)) **Bug fixes** - `EuiRange`/`EuiDualRange`'s track ticks & highlights now update their positions on resize ([#7442](elastic/eui#7442)) **Deprecations** - Updated `EuiFilterButton` to remove the second `.euiFilterButton__textShift` span wrapper. Target `.euiFilterButton__text` instead ([#7444](elastic/eui#7444)) **Breaking changes** - Removed deprecated `EuiNotificationEvent`. We recommend copying the component to your application if necessary ([#7434](elastic/eui#7434)) - Removed deprecated `EuiControlBar`. We recommend using `EuiBottomBar` instead ([#7435](elastic/eui#7435))

`v91.3.1`⏩`v92.0.0-backport.0` --- ## [`v92.0.0-backport.0`](https://github.com/elastic/eui/releases/v92.0.0-backport.0) **This is a backport release only intended for use by Kibana.** **Bug fixes** - Fixed an `EuiTreeView` JSX Typescript error ([elastic#7452](elastic/eui#7452)) - Fixed a color console warning being generated by disabled `EuiStep`s ([elastic#7454](elastic/eui#7454)) ## [`v92.0.0`](https://github.com/elastic/eui/releases/v92.0.0) - Updated generic types of `EuiBasicTable`, `EuiInMemoryTable` and `EuiSearchBar.Query.execute` to add `extends object` constraint ([elastic#7340](elastic/eui#7340)) - This change should have no impact on your applications since the updated types only affect properties that exclusively accept object values. - Added a new `EuiFlyoutResizable` component ([elastic#7439](elastic/eui#7439)) - Updated `EuiTextArea` to accept `isClearable` and `icon` as props ([elastic#7449](elastic/eui#7449)) **Bug fixes** - `EuiRange`/`EuiDualRange`'s track ticks & highlights now update their positions on resize ([elastic#7442](elastic/eui#7442)) **Deprecations** - Updated `EuiFilterButton` to remove the second `.euiFilterButton__textShift` span wrapper. Target `.euiFilterButton__text` instead ([elastic#7444](elastic/eui#7444)) **Breaking changes** - Removed deprecated `EuiNotificationEvent`. We recommend copying the component to your application if necessary ([elastic#7434](elastic/eui#7434)) - Removed deprecated `EuiControlBar`. We recommend using `EuiBottomBar` instead ([elastic#7435](elastic/eui#7435))

`v92.0.0-backport.0`⏩ `v92.1.1` --- ## [`v92.1.1`](https://github.com/elastic/eui/releases/v92.1.1) **Bug fixes** - Minor `EuiDataGrid` cell performance fixes ([#7465](elastic/eui#7465)) ## [`v92.1.0`](https://github.com/elastic/eui/releases/v92.1.0) - Updated `EuiResizableButton` to allow customizing the `indicator` style with either `handle` (default) or `border` ([#7455](elastic/eui#7455)) - Enhanced `EuiResizableContainer` to preserve the drag/resize event when the user's mouse leaves the parent container and re-enters ([#7456](elastic/eui#7456)) **Bug fixes** - Fixed an `EuiTreeView` JSX Typescript error ([#7452](elastic/eui#7452)) - Fixed a color console warning being generated by disabled `EuiStep`s ([#7454](elastic/eui#7454)) **Accessibility** - `EuiDataGrid`'s keyboard/screenreader experience has been tweaked to be more consistent for varying complex data: ([#7448](elastic/eui#7448)) - Headers are now always navigable by arrow key, regardless of whether the header cells contain interactive content - Non-expandable cells containing any amount of interactive content now must be entered via Enter or F2 keypress - Expandable cells continue to be toggled via Enter or F2 keypress - `EuiDataGrid` now provides a direct screen reader hint for Enter key behavior for expandable & interactive cells ([#7448](elastic/eui#7448))

`v92.0.0-backport.0`⏩ `v92.1.1` --- ## [`v92.1.1`](https://github.com/elastic/eui/releases/v92.1.1) **Bug fixes** - Minor `EuiDataGrid` cell performance fixes ([elastic#7465](elastic/eui#7465)) ## [`v92.1.0`](https://github.com/elastic/eui/releases/v92.1.0) - Updated `EuiResizableButton` to allow customizing the `indicator` style with either `handle` (default) or `border` ([elastic#7455](elastic/eui#7455)) - Enhanced `EuiResizableContainer` to preserve the drag/resize event when the user's mouse leaves the parent container and re-enters ([elastic#7456](elastic/eui#7456)) **Bug fixes** - Fixed an `EuiTreeView` JSX Typescript error ([elastic#7452](elastic/eui#7452)) - Fixed a color console warning being generated by disabled `EuiStep`s ([elastic#7454](elastic/eui#7454)) **Accessibility** - `EuiDataGrid`'s keyboard/screenreader experience has been tweaked to be more consistent for varying complex data: ([elastic#7448](elastic/eui#7448)) - Headers are now always navigable by arrow key, regardless of whether the header cells contain interactive content - Non-expandable cells containing any amount of interactive content now must be entered via Enter or F2 keypress - Expandable cells continue to be toggled via Enter or F2 keypress - `EuiDataGrid` now provides a direct screen reader hint for Enter key behavior for expandable & interactive cells ([elastic#7448](elastic/eui#7448))

cee-chen added 3 commits January 9, 2024 10:48

More DRYing

998515d

- remove multiple `euiButtonFillColor()` calls in favor of just one

Current step CSS cleanup

c24613d

- remove CSS nesting - replace hardcoded 2px with token

cee-chen added the tech debt label Jan 9, 2024

changelog

6b80f3c

cee-chen requested a review from 1Copenut January 9, 2024 19:08

cee-chen marked this pull request as ready for review January 9, 2024 19:24

cee-chen requested a review from a team as a code owner January 9, 2024 19:24

1Copenut approved these changes Jan 9, 2024

View reviewed changes

cee-chen merged commit 01f5bd5 into elastic:main Jan 9, 2024

cee-chen deleted the steps-cleanup branch January 9, 2024 22:08

cee-chen added a commit that referenced this pull request Jan 9, 2024

[EuiSteps] Remove chroma transparency warning, misc cleanup (#7454)

b41a160

cee-chen mentioned this pull request Jan 9, 2024

Upgrade EUI to v92.0.0 elastic/kibana#174487

Merged

cee-chen mentioned this pull request Jan 16, 2024

Upgrade EUI to v92.1.1 elastic/kibana#174955

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiSteps] Remove chroma transparency warning, misc cleanup#7454

[EuiSteps] Remove chroma transparency warning, misc cleanup#7454
cee-chen merged 4 commits intoelastic:mainfrom
cee-chen:steps-cleanup

cee-chen commented Jan 9, 2024 •

edited

Loading

Uh oh!

cee-chen commented Jan 9, 2024

Uh oh!

kibanamachine commented Jan 9, 2024

Uh oh!

elasticmachine commented Jan 9, 2024

Uh oh!

1Copenut left a comment

Uh oh!

cee-chen commented Jan 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	color: ${makeHighContrastColor(euiTheme.colors.disabledText)(
	euiButtonFillColor(euiThemeContext, 'disabled').backgroundColor
	)};

Conversation

cee-chen commented Jan 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

QA

General checklist

Uh oh!

cee-chen commented Jan 9, 2024

Uh oh!

kibanamachine commented Jan 9, 2024

Uh oh!

elasticmachine commented Jan 9, 2024

💚 Build Succeeded

Uh oh!

1Copenut left a comment

Choose a reason for hiding this comment

Uh oh!

cee-chen commented Jan 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cee-chen commented Jan 9, 2024 •

edited

Loading