[EuiRange]: Add conditional `aria-valuetext` when `ticks` prop is passed by 1Copenut · Pull Request #7675 · elastic/eui

1Copenut · 2024-04-11T15:59:38Z

Summary

This PR closes #7613.

PR adds aria-valuenow attribute to <EuiRange /> component, and conditionally adds aria-valuetext when range sliders have tick marks. Users can extend the aria-valuetext attribute by passing an optional accessibleLabel string to the ticks array of objects now.

PR will be easiest to review by commit. I tried to keep each commit small and focused on one aspect of the changeset.

Video demonstrating the aria-valuenow and aria-valuetext when value and tick[value] line up.

EuiRangeSlider-ariavaluetext.mov

QA

Remove or strikethrough items that do not apply to your PR.

General checklist

Browser QA
- Checked in both light and dark modes
- Checked in mobile
- Checked in ~~Chrome~~, ~~Safari~~, Edge, and Firefox
- Checked for accessibility including keyboard-only and screenreader modes
Docs site QA
- Added documentation
- Props have proper autodocs (using @default if default values are missing) and playground toggles
  ~~- [ ] Checked Code Sandbox works for any docs examples~~
Code quality checklist
- Added or updated jest and cypress tests
Release checklist
- A changelog entry exists and is marked appropriately.

…text if it exists.

1Copenut

Added thoughts to how I approached the helper functions, and why I went the way I did.

1Copenut · 2024-04-11T16:11:08Z

src/components/form/range/range.tsx

+  handleAriaValueNow = (currentVal: string | number): number | undefined => {
+    if (!currentVal) return;
+
+    let ariaValueNow;
+    const target = Number(currentVal); // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/Number#return_value
+
+    if (!Number.isNaN(target)) {
+      ariaValueNow = target;
+      return ariaValueNow;
+    }
+
+    return ariaValueNow; // undefined
+  };
+
+  handleAriaValueText = (
+    ticks: EuiRangeTick[],
+    currentVal: string | number
+  ): string | undefined => {
+    let ariaValueText;
+    const target = ticks.find(
+      (tick) => tick.value.toString() === currentVal.toString()
+    );
+
+    if (target && !target.accessibleLabel) {
+      ariaValueText = target.value.toString();
+      return ariaValueText;
+    }
+
+    if (target && target.accessibleLabel) {
+      ariaValueText = `${target.value.toString()}, (${target.accessibleLabel})`;
+      return ariaValueText;
+    }
+
+    return ariaValueText; // undefined
+  };
+


Both of these functions required some heady type coercion to get acceptable return values. aria-valuenow requires a number, aria-valuetext a string. I took what TypeScript was giving me, and used native methods and guard clauses to return early when a match was found.

If no match was found / NaN, then I returned undefined because there was nothing meaningful to populate the ARIA attributes with. And I tried to hew closely to the first rule of ARIA.

The first rule of ARIA is also why I opted not to include this functionality for EuiDualRange. The "thumbs" already announce their current position accurately and we have no true input[type="slider"] to hang the ARIA attributes on. We'd be introducing a potentially worse UX.

src/components/form/range/range_slider.tsx

src/components/form/range/range.tsx

…uncement.

mgadewoll

🚢 🐈‍⬛ Thanks for the changes and verification! LGTM 👍

1Copenut · 2024-04-12T16:00:27Z

Buildkite test this

src-docs/src/views/range/levels.tsx

src/components/form/range/range.tsx

src/components/form/range/range.test.tsx

cee-chen

I'd like the question in #7675 (comment) to be resolved first as I think that has a potential baseline improvement OOTB rather than requiring consumers to apply an extra prop, in addition to the cleanup comments left

cee-chen · 2024-04-12T20:25:23Z

BTW, is the PR title still accurate? it looks like we're no longer applying aria-valuenow? (which is totally valid, just want to make sure I'm understanding the intention correctly!)

1Copenut · 2024-04-12T20:34:15Z

I just updated the PR title to represent that we are only adding aria-valuetext now.

changelogs/upcoming/7675.md

cee-chen · 2024-04-16T15:37:37Z

src-docs/src/views/range/ticks.tsx

+          { label: '8GB', value: 42, accessibleLabel: 'eight gigabytes' },
+          { label: '16GB', value: 56, accessibleLabel: 'sixteen gigabytes' },
+          { label: '32GB', value: 70, accessibleLabel: 'thirty-two gigabytes' },
+          { label: '64GB', value: 84, accessibleLabel: 'sixty-four gigabytes' },


😍 Now this is an example that makes sense to me! Huzzah!

Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>

src/components/form/range/range.tsx

Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>

src/components/form/range/range.test.tsx

src-docs/src/views/range/range_example.js

src/components/form/range/range.tsx

This is more clear and succinct. Using it exactly as is. Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>

Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>

cee-chen

Thanks so much for going through feedback rounds with me Trevor! This feels really really solid in terms of feature and code! 🎉

1Copenut · 2024-04-16T17:52:18Z

Buildkite test this

kibanamachine · 2024-04-16T18:20:38Z

Preview staging links for this PR:

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

elasticmachine · 2024-04-16T18:20:41Z

💚 Build Succeeded

Buildkite Build
Commit: 87ea5d5

History

💔 Build #1773 failed 87ea5d5
💔 Build #1770 failed fed091f
💚 Build #1756 succeeded b8439df
💚 Build #1742 succeeded 7a2b1ca
💔 Build #1740 failed 7a2b1ca
💚 Build #1728 succeeded cb7bfe8

cc @1Copenut

`v94.1.0-backport.0` ⏩ `v94.2.1-backport.0` _[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)_ --- ## [`v94.2.1-backport.0`](https://github.com/elastic/eui/releases/v94.2.1-backport.0) **This is a backport release only intended for use by Kibana.** - Reverted the `EuiFlexGroup`/`EuiFlexItem` `component` prop feature due to Kibana typing issues ## [`v94.2.1`](https://github.com/elastic/eui/releases/v94.2.1) **Bug fixes** - Fixed an `EuiTabbedContent` edge case bug that occurred when updated with a completely different set of `tabs` ([#7713](elastic/eui#7713)) - Fixed the `@storybook/test` dependency to be listed in `devDependencies` and not `dependencies` ([#7719](elastic/eui#7719)) ## [`v94.2.0`](https://github.com/elastic/eui/releases/v94.2.0) - Updated `getDefaultEuiMarkdownPlugins()` to allow excluding the following plugins in addition to `tooltip`: ([#7676](elastic/eui#7676)) - `checkbox` - `linkValidator` - `lineBreaks` - `emoji` - Updated `EuiSelectable`'s `isPreFiltered` prop to allow passing a configuration object, which allows disabling search highlighting in addition to search filtering ([#7683](elastic/eui#7683)) - Updated `EuiFlexGroup` and `EuiFlexItem` prop types to support passing any valid React component type to the `component` prop and ensure proper type checking of the extra props forwarded to the `component`. ([#7688](elastic/eui#7688)) - Updated `EuiSearchBar` to allow the `@` special character in query string searches ([#7702](elastic/eui#7702)) - Added a new, optional `optionMatcher` prop to `EuiSelectable` and `EuiComboBox` allowing passing a custom option matcher function to these components and controlling option filtering for given search string ([#7709](elastic/eui#7709)) **Bug fixes** - Fixed an `EuiPageTemplate` bug where prop updates would not cascade down to child sections ([#7648](elastic/eui#7648)) - To cascade props down to the sidebar, `EuiPageTemplate` now explicitly requires using the `EuiPageTemplate.Sidebar` rather than `EuiPageSidebar` - Fixed `EuiFieldNumber`'s typing to accept an icon configuration shape ([#7666](elastic/eui#7666)) - Fixed `EuiFieldText` and `EuiFieldNumber` to render the correct paddings for icon shapes set to `side: 'right'` ([#7666](elastic/eui#7666)) - Fixed `EuiFieldText` and `EuiFieldNumber` to fully ignore `icon`/`prepend`/`append` when `controlOnly` is set to true ([#7666](elastic/eui#7666)) - Fixed `EuiColorPicker`'s input not setting the correct right padding for the number of icons displayed ([#7666](elastic/eui#7666)) - Visual fixes for `EuiRange`s with `showInput`: ([#7678](elastic/eui#7678)) - Longer `append`/`prepend` labels no longer cause a background bug - Inputs can no longer overwhelm the actual range in width - Fixed a visual text alignment regression in `EuiTableRowCell`s with the `row` header scope ([#7681](elastic/eui#7681)) - Fixed `toolTipProps` type on `EuiSuperUpdateButton` to use `Partial<EuiToolTipProps>` ([#7692](elastic/eui#7692)) - Fixes missing prop type for `popperProps` on `EuiDatePicker` ([#7694](elastic/eui#7694)) - Fixed a focus bug with `EuiDataGrid`s with `leadingControlColumns` when moving columns to the left/right ([#7701](elastic/eui#7701)) ([#7698](elastic/eui#7698)) - Fixed `EuiSuperDatePicker` to validate date string with respect of locale on `EuiAbsoluteTab`. ([#7705](elastic/eui#7705)) - Fixed a visual bug with `EuiSuperDatePicker`'s absolute tab on small mobile screens ([#7708](elastic/eui#7708)) - Fixed i18n of empty and loading state messages for the `FieldValueSelectionFilter` component ([#7718](elastic/eui#7718)) **Dependency updates** - Updated `@hello-pangea/dnd` to v16.6.0 ([#7599](elastic/eui#7599)) - Updated `remark-rehype` to v8.1.0 ([#7601](elastic/eui#7601)) **Accessibility** - Improved `EuiBasicTable` and `EuiInMemoryTable`'s selection checkboxes to have unique aria-labels per row ([#7672](elastic/eui#7672)) - Added `aria-valuetext` attributes to `EuiRange`s with tick labels for improved screen reader UX ([#7675](elastic/eui#7675)) - Updated `EuiAccordion` to keep focus on accordion trigger instead of moving to content on click/keypress ([#7696](elastic/eui#7696)) - Added `aria-disabled` attribute to `EuiHorizontalSteps` when status is "disabled" ([#7699](elastic/eui#7699)) --------- Co-authored-by: Tomasz Kajtoch <tomasz.kajtoch@elastic.co>

1Copenut added 7 commits April 10, 2024 16:29

Added aria-valuetext logic to EuiRangeSlider.

b6085ce

Added accessibleLabel type to EuiRangeTick, logic to build aria-value…

aab49b5

…text if it exists.

Adding aria-valuenow logic for EuiRange.

a8517c8

Adding unit test logic for aria-valuenow and aria-valuetext.

da14962

Updated EuiRange ticks doc with accessibleLabel.

5cc66a5

Updated Tick marks docs to highlight accessibleLabel property.

7c155dc

Linting error on target declaration.

bd2275e

1Copenut added feature request accessibility labels Apr 11, 2024

1Copenut self-assigned this Apr 11, 2024

1Copenut requested a review from a team as a code owner April 11, 2024 15:59

Added changelog for 7675.

cb7bfe8

1Copenut commented Apr 11, 2024

View reviewed changes

1Copenut changed the title ~~Feature/range tick ariavaluetext~~ [EuiRange]: Add aria-valuenow and conditional aria-valuetext when ticks prop is passed Apr 11, 2024

mgadewoll reviewed Apr 12, 2024

View reviewed changes

src/components/form/range/range_slider.tsx Outdated Show resolved Hide resolved

src/components/form/range/range.tsx Outdated Show resolved Hide resolved

1Copenut added 3 commits April 12, 2024 09:44

Removing aria-valuenow attribute because it is not needed for SR anno…

20d11d6

…uncement.

Refactored handleAriaValueText into one ternary from two guard clauses.

8372187

Refactoring ternary for better readability.

7a2b1ca

mgadewoll approved these changes Apr 12, 2024

View reviewed changes

cee-chen reviewed Apr 12, 2024

View reviewed changes

src-docs/src/views/range/levels.tsx Outdated Show resolved Hide resolved

cee-chen reviewed Apr 12, 2024

View reviewed changes

src/components/form/range/range.tsx Outdated Show resolved Hide resolved

cee-chen reviewed Apr 12, 2024

View reviewed changes

src/components/form/range/range.tsx Outdated Show resolved Hide resolved

cee-chen reviewed Apr 12, 2024

View reviewed changes

src/components/form/range/range.test.tsx Outdated Show resolved Hide resolved

cee-chen suggested changes Apr 12, 2024

View reviewed changes

1Copenut changed the title ~~[EuiRange]: Add aria-valuenow and conditional aria-valuetext when ticks prop is passed~~ [EuiRange]: Add conditional aria-valuetext when ticks prop is passed Apr 12, 2024

Refactored handleAriaValueText to allow string labels.

1a1f48d

1Copenut added 2 commits April 15, 2024 11:18

Updated test logic to include label typeof string.

5ef3d4e

Updated docs to illustrate aria-valuetext with label typeof string.

b8439df

1Copenut requested a review from cee-chen April 15, 2024 19:34