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

[EuiForm] Increase Contrast on EuiForm Section Controls to Pass WCAG AA Standards #6729

Merged
merged 13 commits into from
Apr 25, 2023

Conversation

breehall
Copy link
Contributor

@breehall breehall commented Apr 21, 2023

Fixes #6690
Fixes #6692

Summary

Increase contrast ratio for checkbox border in EuiCheckbox (#6690)

The current contrast ratio for checkbox outlines in both light and dark modes does not meet WCAG AA guidelines for UI components. This PR increases the threshold for the shade and tint that are used the generate the outline color.


What else does this affect?
In order to increase the contrast ratios, I used the euiFormCustomControlBorderColor used with both SASS and Emotion.

There should be no large visual differences with this change. This key affects the following components:


image


Contrast Details **Before** image

After
image


Increase contrast ratio for EuiSwitch Body (#6692)

The current contrast ratio for the switch toggle body in both light and dark modes does not meet WCAG AA guidelines for UI components. The $euiSwitchOffColor key was updated the adjust the background accordingly and increase the contrast ratio. I will say this is a stark difference because the ratio was super low.

Just a note:
When disabled in light mode, the contrast ratio for the switch thumb on the switch body is low. Guidelines state that this is ok because it's disabled.


image


Contrast Details **Before** image

QA

General QA

  • EuiCheckbox outline and EuiSwitch background should meet the suggested 3.0 contrast ratio.

General checklist

  • Checked in both light and dark modes
  • Checked for breaking changes and labeled appropriately
  • Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

…election controls to meet WCAG AA guidelines for graphical objects and user interface components. Updated the hard coded form styles test case that targets customControlBorderColor with the updated HEX value
…lection controls to meet WCAG AA guidelines for graphical objects and user interface components.
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6729/

…tch background color and allows it to meet WCAG AA UI guidelines
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6729/

@breehall breehall requested a review from 1Copenut April 21, 2023 21:17
@breehall breehall marked this pull request as ready for review April 21, 2023 21:17
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6729/

Copy link
Contributor

@1Copenut 1Copenut left a comment

Choose a reason for hiding this comment

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

The changes bring us in line with the 3:1 required contrast ratio. I took a few minutes to experiment with the disabled switch to see if I could make it recede a bit more like the disabled radio and checkbox.

Feel free to use or discard these suggestions. I'll wait for @andreadelrio for the final word on color.

Thanks @breehall !


Screen Shot 2023-04-24 at 10 35 52 AM


Screen Shot 2023-04-24 at 11 00 29 AM

@andreadelrio
Copy link
Contributor

andreadelrio commented Apr 24, 2023

The changes bring us in line with the 3:1 required contrast ratio. I took a few minutes to experiment with the disabled switch to see if I could make it recede a bit more like the disabled radio and checkbox.

Feel free to use or discard these suggestions. I'll wait for @andreadelrio for the final word on color.

Thanks @breehall !

Screen Shot 2023-04-24 at 10 35 52 AM Screen Shot 2023-04-24 at 11 00 29 AM

I like this proposal for light mode. For dark mode, I prefer what Bree has above, visually it stands out more.

…witch in lightmode to help distinguish the state
@breehall
Copy link
Contributor Author

I like this proposal for light mode. For dark mode, I prefer what Bree has above, visually it stands out more.

@andreadelrio @1Copenut Here's the updated disabled state for EuiSwitch in light mode.

Screen.Recording.2023-04-24.at.3.51.16.PM.mov

Comment on lines 46 to 70

&:disabled {
&:hover,
~ .euiSwitch__label:hover {
cursor: not-allowed;
}

.euiSwitch__body {
background-color: $euiSwitchOffDisabledColor;
}

.euiSwitch__thumb {
background-color: $euiSwitchOffThumbDisabledColor;
border-color: $euiSwitchOffThumbDisabledBorderColor;
box-shadow: none;
}

.euiSwitch__icon {
fill: $euiColorDarkShade;
}

+ .euiSwitch__label {
color: $euiFormControlDisabledColor;
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I moved this under the aria styles is to ensure the background color for disable overrides the selected (blue) background color if the switch is active but also disabled.

Comment on lines 43 to 46
$euiSwitchOffDisabledColor: lightOrDarkTheme(transparentize($euiColorLightShade, .5), transparentize($euiColorDarkShade, .4)) !default;
$euiSwitchOffThumbDisabledColor: rgba(0,0,0,0) !default;
$euiSwitchOffThumbDisabledBorderColor: lightOrDarkTheme(transparentize($euiColorDarkShade, .5), $euiColorDarkShade) !default;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The euiFormCustomControlBorderColor affects other components and I felt it would be best to add the switch specific styles as their own keys.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's a good call. They're unique to the switch component, so having them called out and labeled is helpful.

Copy link
Member

Choose a reason for hiding this comment

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

Quick 2c: If they're only being used by the switch, can we define them in Sass where they're used instead of as a global form variable? (i.e., in _switch.scss below line 47).

This will (hopefully) prevent downstream consumers from using them when we only want them as one-off variables.

breehall and others added 3 commits April 24, 2023 15:56
Didn't mean to add my `prettierrc`
This reverts commit a528e8b.
@breehall breehall requested a review from 1Copenut April 24, 2023 20:12
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6729/

Copy link
Contributor

@1Copenut 1Copenut left a comment

Choose a reason for hiding this comment

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

👍 LGTM!

@cee-chen
Copy link
Member

Apologies for my confusion in Slack regarding the disabled styles! My only remaining change requests are changing the color of the check/cross icons to match the thumb in both light & dark mode for readability, + moving the EuiSwitch Sass variable declarations & a quick comment update.

Seriously awesome work on this Bree! And thanks so much Andrea for the design feedback, & Trevor for the really great disabled color change suggestion!

@andreadelrio
Copy link
Contributor

@breehall @1Copenut I wonder if the checks would pass if we made the cross in EuiSwitch white instead of black? I think it looks better and would alleviate to a certain extent how much "darker" we're making this component. Can you confirm if this would work please?

Frame 7

@cee-chen
Copy link
Member

@andreadelrio You read our minds, we just chatted about that over Zoom! 100% agreed the white X is much easier to read on light mode.

…ng them directly to the component.

- Updated icon color so that icons appear white for both states in light mode and black in dark mode
- Misc comment updates
@breehall
Copy link
Contributor Author

Current Revisions

This last push includes a change to the icon colors in both light and dark modes. When in light mode and not disabled, the icons on EuiSwitch will appear in white and when in dark mode they will appear in black. I think this will really help with the distinction of the component being disabled in dark mode and also add consistency to the coloring.

Feel free to review (once the PR preview loads) and let me know what you think!

image

Apologies for my confusion in Slack regarding the disabled styles!

No worries! Thank you all for really helping me come to a decision with this! :)

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6729/

Copy link
Contributor

@andreadelrio andreadelrio left a comment

Choose a reason for hiding this comment

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

LGTM 🚀. Great job @breehall !

Copy link
Member

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

Fantastic work again Bree. Super small changelog request for you, then I think this is good to go. Also, if we could update the PR description with the final EuiSwitch designs that would be helpful for putting the next EUI newsletter together - thanks!

upcoming_changelogs/6729.md Outdated Show resolved Hide resolved
Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6729/

Comment on lines +5 to +7
$euiSwitchOffDisabledColor: lightOrDarkTheme(transparentize($euiColorLightShade, .5), transparentize($euiColorDarkShade, .4));
$euiSwitchDisabledThumbColor: lightOrDarkTheme(transparentize($euiColorDarkShade, .5), $euiColorDarkShade);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cee-chen instead of adding these in the disabled block, I had to add them at a higher level because they're being used in a different scope that targets compressed switches as well

Copy link
Member

Choose a reason for hiding this comment

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

All good! As long as they're scoped to a selector, they're not available globally 👍

Copy link
Member

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

Awesome work on this Bree! 🚢

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6729/

@breehall breehall merged commit d39c0e9 into elastic:main Apr 25, 2023
1 check passed
jbudz pushed a commit to elastic/kibana that referenced this pull request May 15, 2023
## Summary

`eui@77.2.2` ⏩ `eui@79.0.1`

🦴 The primary changes in this upgrade are around the deprecated
`EuiLoadingContent` being removed in favor of `EuiSkeletonText`.
- Most instances have been a [direct swap of
usage](327626a),
but [some replacements were a bit more
opinionated](e6ceb36)
as I saw them as potential to take advantage of `EuiSkeletonText`'s
syntactical sugar and screen reader announcements for when state
switches to loaded.

---

## [`79.0.1`](https://github.com/elastic/eui/tree/v79.0.1)

**Bug fixes**

- Fixed broken push `EuiFlyout` behavior
([#6764](elastic/eui#6764))


## [`79.0.0`](https://github.com/elastic/eui/tree/v79.0.0)

- Updated all `EuiSkeleton` components with new props that allow for
more control over screen reader live announcements:
`announceLoadingStatus`, `announceLoadedStatus`, and `ariaLiveProps`
([#6752](elastic/eui#6752))
- Improved keyboard accessibility in `EuiPageHeader` by ensuring the
right side menu items come into focus from left to right.
([#6753](elastic/eui#6753))

**Breaking changes**

- Removed deprecated `EuiLoadingContent`. Use the `EuiSkeleton`
components instead. ([#6754](elastic/eui#6754))


## [`78.0.0`](https://github.com/elastic/eui/tree/v78.0.0)

- Improved the contrast ratio of `EuiCheckbox`, `EuiRadio`, and
`EuiSwitch` in their unchecked states to meet WCAG AA guidelines.
([#6729](elastic/eui#6729))
- Added React Testing Library `*ByTestSubject` custom commands to
`within()`. RTL utilities can be imported from
`@elastic/eui/lib/test/rtl`.
([#6737](elastic/eui#6737))
- Updated `EuiAvatar` to support a new letter `casing` prop that allow
customizing text capitalization
([#6739](elastic/eui#6739))
- Updated `EuiFocusTrap` to support the `gapMode` prop configuration
(now defaults to `padding`)
([#6744](elastic/eui#6744))

**Bug fixes**

- Fixed inconsistency in `EuiSearchBar`'s AND/OR semantics between DSL
and query string generation
([#6717](elastic/eui#6717))
- Fixed `EuiFieldNumber`'s native browser validity detection causing
extra unnecessary rerenders
([#6741](elastic/eui#6741))
- Fixed the `scrollLock` property on `EuiFocusTrap` (and other
components using `EuiFocusTrap`, such as `EuiFlyout` and `EuiModal`) to
no longer block scrolling on nested portalled content, such as combobox
dropdowns ([#6744](elastic/eui#6744))

**Breaking changes**

- `EuiAvatar`s with the default `user` type will now default to
capitalizing all initials in uppercase
([#6739](elastic/eui#6739))

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
jasonrhodes pushed a commit to elastic/kibana that referenced this pull request May 17, 2023
## Summary

`eui@77.2.2` ⏩ `eui@79.0.1`

🦴 The primary changes in this upgrade are around the deprecated
`EuiLoadingContent` being removed in favor of `EuiSkeletonText`.
- Most instances have been a [direct swap of
usage](327626a),
but [some replacements were a bit more
opinionated](e6ceb36)
as I saw them as potential to take advantage of `EuiSkeletonText`'s
syntactical sugar and screen reader announcements for when state
switches to loaded.

---

## [`79.0.1`](https://github.com/elastic/eui/tree/v79.0.1)

**Bug fixes**

- Fixed broken push `EuiFlyout` behavior
([#6764](elastic/eui#6764))


## [`79.0.0`](https://github.com/elastic/eui/tree/v79.0.0)

- Updated all `EuiSkeleton` components with new props that allow for
more control over screen reader live announcements:
`announceLoadingStatus`, `announceLoadedStatus`, and `ariaLiveProps`
([#6752](elastic/eui#6752))
- Improved keyboard accessibility in `EuiPageHeader` by ensuring the
right side menu items come into focus from left to right.
([#6753](elastic/eui#6753))

**Breaking changes**

- Removed deprecated `EuiLoadingContent`. Use the `EuiSkeleton`
components instead. ([#6754](elastic/eui#6754))


## [`78.0.0`](https://github.com/elastic/eui/tree/v78.0.0)

- Improved the contrast ratio of `EuiCheckbox`, `EuiRadio`, and
`EuiSwitch` in their unchecked states to meet WCAG AA guidelines.
([#6729](elastic/eui#6729))
- Added React Testing Library `*ByTestSubject` custom commands to
`within()`. RTL utilities can be imported from
`@elastic/eui/lib/test/rtl`.
([#6737](elastic/eui#6737))
- Updated `EuiAvatar` to support a new letter `casing` prop that allow
customizing text capitalization
([#6739](elastic/eui#6739))
- Updated `EuiFocusTrap` to support the `gapMode` prop configuration
(now defaults to `padding`)
([#6744](elastic/eui#6744))

**Bug fixes**

- Fixed inconsistency in `EuiSearchBar`'s AND/OR semantics between DSL
and query string generation
([#6717](elastic/eui#6717))
- Fixed `EuiFieldNumber`'s native browser validity detection causing
extra unnecessary rerenders
([#6741](elastic/eui#6741))
- Fixed the `scrollLock` property on `EuiFocusTrap` (and other
components using `EuiFocusTrap`, such as `EuiFlyout` and `EuiModal`) to
no longer block scrolling on nested portalled content, such as combobox
dropdowns ([#6744](elastic/eui#6744))

**Breaking changes**

- `EuiAvatar`s with the default `user` type will now default to
capitalizing all initials in uppercase
([#6739](elastic/eui#6739))

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@breehall breehall deleted the form-control-accessibility branch September 18, 2023 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants