Skip to content

[EuiColorPicker] Prevent duplicate color output for screen readers #8639

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.

Sign up for GitHub

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

Merged
mgadewoll merged 7 commits into from
Apr 29, 2025
Merged

[EuiColorPicker] Prevent duplicate color output for screen readers #8639

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0240d50
refactor(colorPicker): prevent duplicate screen reader output
mgadewoll Apr 25, 2025
a2624eb
refactor(colorPicker): add tooltips and improve accessible labels
mgadewoll Apr 25, 2025
5b836a4
refactor(tooltip): prevent additional key stops when tooltip has disa…
mgadewoll Apr 25, 2025
dc7bd8c
test: update snapshots
mgadewoll Apr 25, 2025
2b2ef0e
test(vrt): update reference images
mgadewoll Apr 25, 2025
ac2676f
chore: add changelog
mgadewoll Apr 25, 2025
417ede0
chore: update changelog
mgadewoll Apr 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified .../reference/chrome_desktop_Forms_EuiColorPicker_EuiColorPicker_High_Contrast.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified .../chrome_desktop_Forms_EuiColorPicker_EuiColorPicker_High_Contrast_Dark_Mode.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified ...chrome_desktop_Forms_EuiColorPicker_EuiColorPicker_Inline_With_All_Elements.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified ...i/reference/chrome_mobile_Forms_EuiColorPicker_EuiColorPicker_High_Contrast.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified ...e/chrome_mobile_Forms_EuiColorPicker_EuiColorPicker_High_Contrast_Dark_Mode.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified .../chrome_mobile_Forms_EuiColorPicker_EuiColorPicker_Inline_With_All_Elements.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
7 changes: 7 additions & 0 deletions packages/eui/changelogs/upcoming/8639.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
**Accessibility**

- Improved the accessibility of `EuiColorPicker` by:
- preventing duplicate color output for screen readers
- adding tooltips with visual color labels for the selected colors on the saturation and hue sliders
- updated accessible labels and announcements to be more descriptive

57 changes: 32 additions & 25 deletions packages/eui/src/components/color_picker/__snapshots__/color_picker.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,19 +313,17 @@ exports[`EuiColorPicker inline 1`] = `
class="euiSaturation__saturation emotion-euiSaturation__saturation"
/>
</div>
<button
aria-describedby="generated-id-instructions"
aria-label="#ffeedd"
aria-roledescription="HSV color mode saturation and value 2-axis slider"
class="euiSaturation__indicator emotion-euiSaturation__indicator"
id="generated-id-saturationIndicator"
style="inset-inline-start: 0; inset-block-start: 0;"
/>
<span
aria-live="assertive"
hidden=""
class="euiToolTipAnchor emotion-euiToolTipAnchor-inlineBlock-euiSaturation__tooltip"
style="inset-inline-start: 0; inset-block-start: 0;"
>
#ffeedd
<button
aria-describedby="generated-id-instructions"
aria-label="Select a color"
aria-roledescription="HSV color mode saturation and value 2-axis slider."
class="euiSaturation__indicator emotion-euiSaturation__indicator"
id="generated-id-saturationIndicator"
/>
</span>
<span
hidden=""
Expand All @@ -343,22 +341,31 @@ exports[`EuiColorPicker inline 1`] = `
>
Select the HSV color mode 'hue' value
</label>
<p
aria-live="polite"
class="emotion-euiScreenReaderOnly"
<span
class="euiToolTipAnchor emotion-euiToolTipAnchor-inlineBlock-euiHue__tooltip"
>
#ffeedd
</p>
<input
class="euiHue__range emotion-euiHue__range"
id="undefined-hue"
max="359"
min="0"
step="1"
type="range"
value="30"
/>
<input
aria-roledescription="Hue slider"
aria-valuetext="Hue 30°"
class="euiHue__range emotion-euiHue__range"
id="undefined-hue"
max="359"
min="0"
step="1"
type="range"
value="30"
/>
</span>
</div>
<p
aria-atomic="true"
aria-live="polite"
class="emotion-euiScreenReaderOnly"
>
Selected color
:
#FFEEDD
</p>
<ul
class="euiColorPicker__swatches emotion-euiColorPicker__swatches"
>
Expand Down
96 changes: 50 additions & 46 deletions packages/eui/src/components/color_picker/__snapshots__/hue.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,23 @@ exports[`EuiHue accepts a hex value 1`] = `
>
Select the HSV color mode 'hue' value
</label>
<p
aria-live="polite"
class="emotion-euiScreenReaderOnly"
<span
class="euiToolTipAnchor emotion-euiToolTipAnchor-inlineBlock-euiHue__tooltip"
>
#00FFFF
</p>
<input
aria-label="aria-label"
class="euiHue__range emotion-euiHue__range"
data-test-subj="test subject string"
id="undefined-hue"
max="359"
min="0"
step="1"
type="range"
value="180"
/>
<input
aria-label="aria-label"
aria-roledescription="Hue slider"
aria-valuetext="Hue 180°"
class="euiHue__range emotion-euiHue__range"
data-test-subj="test subject string"
id="undefined-hue"
max="359"
min="0"
step="1"
type="range"
value="180"
/>
</span>
</div>
</div>
`;
Expand All @@ -43,21 +43,23 @@ exports[`EuiHue accepts a hue value 1`] = `
>
Select the HSV color mode 'hue' value
</label>
<p
aria-live="polite"
class="emotion-euiScreenReaderOnly"
/>
<input
aria-label="aria-label"
class="euiHue__range emotion-euiHue__range"
data-test-subj="test subject string"
id="undefined-hue"
max="359"
min="0"
step="1"
type="range"
value="180"
/>
<span
class="euiToolTipAnchor emotion-euiToolTipAnchor-inlineBlock-euiHue__tooltip"
>
<input
aria-label="aria-label"
aria-roledescription="Hue slider"
aria-valuetext="Hue 180°"
class="euiHue__range emotion-euiHue__range"
data-test-subj="test subject string"
id="undefined-hue"
max="359"
min="0"
step="1"
type="range"
value="180"
/>
</span>
</div>
</div>
`;
Expand All @@ -73,21 +75,23 @@ exports[`EuiHue is rendered 1`] = `
>
Select the HSV color mode 'hue' value
</label>
<p
aria-live="polite"
class="emotion-euiScreenReaderOnly"
/>
<input
aria-label="aria-label"
class="euiHue__range emotion-euiHue__range"
data-test-subj="test subject string"
id="undefined-hue"
max="359"
min="0"
step="1"
type="range"
value="1"
/>
<span
class="euiToolTipAnchor emotion-euiToolTipAnchor-inlineBlock-euiHue__tooltip"
>
<input
aria-label="aria-label"
aria-roledescription="Hue slider"
aria-valuetext="Hue 1°"
class="euiHue__range emotion-euiHue__range"
data-test-subj="test subject string"
id="undefined-hue"
max="359"
min="0"
step="1"
type="range"
value="1"
/>
</span>
</div>
</div>
`;
46 changes: 24 additions & 22 deletions packages/eui/src/components/color_picker/__snapshots__/saturation.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`EuiHue accepts a color 1`] = `
exports[`EuiSaturation accepts a color 1`] = `
<div
aria-label="aria-label"
class="euiSaturation testClass1 testClass2 emotion-euiSaturation-euiTestCss"
Expand All @@ -15,17 +15,18 @@ exports[`EuiHue accepts a color 1`] = `
class="euiSaturation__saturation emotion-euiSaturation__saturation"
/>
</div>
<button
aria-describedby="generated-id-instructions"
aria-roledescription="HSV color mode saturation and value 2-axis slider"
class="euiSaturation__indicator emotion-euiSaturation__indicator"
id="generated-id-saturationIndicator"
style="inset-inline-start: 0; inset-block-start: 0;"
/>
<span
aria-live="assertive"
hidden=""
/>
class="euiToolTipAnchor emotion-euiToolTipAnchor-inlineBlock-euiSaturation__tooltip"
style="inset-inline-start: 0; inset-block-start: 0;"
>
<button
aria-describedby="generated-id-instructions"
aria-label="Select a color"
aria-roledescription="HSV color mode saturation and value 2-axis slider."
class="euiSaturation__indicator emotion-euiSaturation__indicator"
id="generated-id-saturationIndicator"
/>
</span>
<span
hidden=""
id="generated-id-instructions"
Expand All @@ -35,7 +36,7 @@ exports[`EuiHue accepts a color 1`] = `
</div>
`;

exports[`EuiHue is rendered 1`] = `
exports[`EuiSaturation is rendered 1`] = `
<div
aria-label="aria-label"
class="euiSaturation testClass1 testClass2 emotion-euiSaturation-euiTestCss"
Expand All @@ -50,17 +51,18 @@ exports[`EuiHue is rendered 1`] = `
class="euiSaturation__saturation emotion-euiSaturation__saturation"
/>
</div>
<button
aria-describedby="generated-id-instructions"
aria-roledescription="HSV color mode saturation and value 2-axis slider"
class="euiSaturation__indicator emotion-euiSaturation__indicator"
id="generated-id-saturationIndicator"
style="inset-inline-start: 0; inset-block-start: 0;"
/>
<span
aria-live="assertive"
hidden=""
/>
class="euiToolTipAnchor emotion-euiToolTipAnchor-inlineBlock-euiSaturation__tooltip"
style="inset-inline-start: 0; inset-block-start: 0;"
>
<button
aria-describedby="generated-id-instructions"
aria-label="Select a color"
aria-roledescription="HSV color mode saturation and value 2-axis slider."
class="euiSaturation__indicator emotion-euiSaturation__indicator"
id="generated-id-saturationIndicator"
/>
</span>
<span
hidden=""
id="generated-id-instructions"
Expand Down
11 changes: 11 additions & 0 deletions packages/eui/src/components/color_picker/color_picker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
} from '../form';
import { useEuiI18n } from '../i18n';
import { EuiPopover } from '../popover';
import { EuiScreenReaderOnly } from '../accessibility';

import { EuiColorPickerSwatch } from './color_picker_swatch';
import { EuiHue } from './hue';
Expand Down Expand Up @@ -212,6 +213,7 @@ export const EuiColorPicker: FunctionComponent<EuiColorPickerProps> = ({
const [
popoverLabel,
colorLabel,
selectedColorLabel,
colorErrorMessage,
transparent,
alphaLabel,
Expand All @@ -221,6 +223,7 @@ export const EuiColorPicker: FunctionComponent<EuiColorPickerProps> = ({
[
'euiColorPicker.popoverLabel',
'euiColorPicker.colorLabel',
'euiColorPicker.selectedColorLabel',
'euiColorPicker.colorErrorMessage',
'euiColorPicker.transparent',
'euiColorPicker.alphaLabel',
Expand All @@ -230,6 +233,7 @@ export const EuiColorPicker: FunctionComponent<EuiColorPickerProps> = ({
[
'Color selection dialog',
'Color value',
'Selected color',
'Invalid color value',
'Transparent',
'Alpha channel (opacity) value',
Expand Down Expand Up @@ -514,6 +518,13 @@ export const EuiColorPicker: FunctionComponent<EuiColorPickerProps> = ({
onChange={handleHueSelection}
onKeyDown={handleOnKeyDown}
/>
<EuiScreenReaderOnly>
{/* Note: using EuiScreenReaderLive didn't work as expected for VoiceOver */}
Copy link
Contributor

@weronikaolejniczak weronikaolejniczak Apr 28, 2025

Choose a reason for hiding this comment

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

non-blocking question:

Is this note necessary here? Is the usage not working for VO specific for this component?

If not, and EuiScreenReaderOnly doesn't work globally for VO, I would add a note there (e.g. packages/eui/src/components/accessibility/screen_reader_only/screen_reader_only.tsx) and remove this comment.

Copy link
Contributor Author

@mgadewoll mgadewoll Apr 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The intention for the comment here was to ensure we don't update to EuiScreenReaderLive in the future and break it 😅 I have not verified if that's a global issue so far, as it was outside of the scope for this issue.

If you feel the comment is redundant, we can remove it. It's just meant as a safeguard.

Copy link
Contributor Author

@mgadewoll mgadewoll Apr 28, 2025

Choose a reason for hiding this comment

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

Now that I think about it: I actually did look at EuiScreenReaderLive in VO as part of a different request last week and it worked as expected. So it might not be EuiScreenReaderLive specific but the combination of EuiScreenReaderOnly and EuiScreenReaderLive

Screen.Recording.2025-04-24.at.11.44.11.mov

<p aria-live="polite" aria-atomic="true">
{/* use uppercase to ensure letters are spoken separately */}
{selectedColorLabel}: {chromaColor?.hex().toUpperCase()}
</p>
</EuiScreenReaderOnly>
</>
)}
{showSwatches && (
Expand Down
8 changes: 7 additions & 1 deletion packages/eui/src/components/color_picker/hue.styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export const euiHueStyles = (euiThemeContext: UseEuiTheme) => {
})}
`,

euiHue__range: css`
euiHue__tooltip: css`
${logicalCSS('height', thumbSize)}
/* Allow for overlap */
${logicalCSS('width', `calc(100% + 2px)`)}
Expand All @@ -69,6 +69,12 @@ export const euiHueStyles = (euiThemeContext: UseEuiTheme) => {
'margin-top',
mathWithUnits(height, (x) => x / -2)
)}
`,

euiHue__range: css`
${logicalCSS('height', '100%')}
${logicalCSS('width', '100%')}


/* Resets for the range */
appearance: none;
Expand Down
Loading