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

[EuiStat] Accessibility - removed aria-hidden=true attribute #7864

Merged
merged 7 commits into from
Jul 5, 2024

Conversation

alexwizp
Copy link
Contributor

@alexwizp alexwizp commented Jul 1, 2024

@kibanamachine
Copy link

Preview staging links for this PR:

@alexwizp alexwizp marked this pull request as ready for review July 1, 2024 10:52
@alexwizp alexwizp requested a review from a team as a code owner July 1, 2024 10:52
@alexwizp alexwizp requested a review from cee-chen July 1, 2024 13:29
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.

I tested this briefly on my local machine and feel it's a better overall solution than consolidating the two text nodes using aria-hidden. VoiceOver announced each section as expected, and the visual cursor remained on the screen.

I'll leave the final code approval to @cee-chen but wanted to give an accessibility road-test comment to be part of the record.

it means JS doesn't have to instantiate a new reference
- memoize styles
- use `useMemo` for JSX elements
- remove `Fragment`
- some code reordering
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.

I pushed up a couple unrelated/optional perf cleanups. The move from aria-hidden to aria-label was excellent, thanks so much for this PR Alexey!

@cee-chen cee-chen enabled auto-merge (squash) July 5, 2024 08:12
@cee-chen cee-chen merged commit fe13fc5 into elastic:main Jul 5, 2024
5 checks passed
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cee-chen pushed a commit to elastic/kibana that referenced this pull request Jul 25, 2024
`v95.3.0` ⏩ `v95.4.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)_

---

## [`v95.4.0`](https://github.com/elastic/eui/releases/v95.4.0)

- Added `anomalyChart`, `anomalySwimLane`, `changePointDetection`,
`fieldStatistics`, `logPatternAnalysis`, `logRateAnalysis` and
`singleMetricViewer` glyph to `EuiIcon`
([#7873](elastic/eui#7873))

**Bug fixes**

- Fixed overlapping content in `EuiBasicTable` for expanded and
selectable table rows
([#7895](elastic/eui#7895))
- Fixed the alignment of `EuiBasicTable` mobile actions
([#7895](elastic/eui#7895))

**Accessibility**

- Improved `EuiStat`'s screen reader accessibility
([#7864](elastic/eui#7864))

---

## Additional Changes

- reverts temporary fix for overlapping content in nested tables done in
PR [#188374](#188374)

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants