[Observability:Streams] Improve Streams table accessibility#225659
Conversation
…over for streams table
|
Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs) |
|
Pinging @elastic/kibana-accessibility (Project:Accessibility) |
…tention-columns-on-streams-homepage-are-not-keyboard-accessible-and-dont-have-focus
…tention-columns-on-streams-homepage-are-not-keyboard-accessible-and-dont-have-focus
…tention-columns-on-streams-homepage-are-not-keyboard-accessible-and-dont-have-focus
awahab07
left a comment
There was a problem hiding this comment.
Great to see this implementation.
Is it possible to extract the i18n texts out into a separate file to keep the component definition clean?
I checked the page with VoiceOver on and observed a few things:
Focusing Documents column
Navigating via Tab also focuses the "Documents" column. It should not as there's no actionable element in that column.
However, navigating via control+option+
As an example, see APM -> Hosts table (notice from 00:32):
APM.Hosts.Table.-.Voice.Over.mov
VoiceOver over announces a few bits:
- It announces the stream name twice when focused, once for link and once for the stream name (notice at 00:18)
- Whenever "Documents" column is focused, it re-introduces the table (notice at 00:24 and 00:44)
- After describing the ∞, it calls it 'image' (see at 00:33). I believe such icons should be aria-hidden. Though it should describe it's purpose only.
Streams-home-page-A11y.mov
…tention-columns-on-streams-homepage-are-not-keyboard-accessible-and-dont-have-focus
awahab07
left a comment
There was a problem hiding this comment.
Thanks for the refactor, it's more readable now.
I tested the VoiceOver again and looks better now.
There's still a point where it re-introduces the table every time the streams link gets in focus as a result of Tab stepping (See 00:16 and 0035). It doesn't occur when it's navigated via control+option (see 00:55).
Check if it's because inside the Retention Column, both the EuiBadge and EuiLink have aria-labels and when focus goes inside onto EuiLink, it's considered outside of table. (see 00:27 where the focus steps inside). See if it's possible to make only one element Tab focusable (and the link is still navigable via keyboard).
Streams-re-introduction-of-table-stream-links.mov
…tention-columns-on-streams-homepage-are-not-keyboard-accessible-and-dont-have-focus
…tention-columns-on-streams-homepage-are-not-keyboard-accessible-and-dont-have-focus
…-observabilitystreams-documents-and-retention-columns-on-streams-homepage-are-not-keyboard-accessible-and-dont-have-focus
awahab07
left a comment
There was a problem hiding this comment.
Thanks for the changes, it LGTM now.
I pushed a few tweaks. Please do have a look in case I missed something. Here's a preview of how it behaves now:
streams-a11y-after-tweaks.mov
| <EuiIcon type="visLine" size="m" /> | ||
| ); | ||
|
|
||
| const chartDescription = React.useMemo(() => { |
There was a problem hiding this comment.
Removed the chart aria labels as it's now aria-hidden="true".
|
|
||
| return ( | ||
| <span | ||
| tabIndex={0} |
There was a problem hiding this comment.
Removed the Tab key focus from the infinity icon as it's not actionable. Without it, it captures the Tab focus (see at 00:06 below)
Streams-a11y-infinite-retention-focusable.mov
|
@awahab07 |
|
@rStelmach what's missing to get this merged? |
|
@flash1293 I need extra review from someone from streams |
tonyghiani
left a comment
There was a problem hiding this comment.
Codewise LGTM, there are some conflicts to fix, then should be good to go
…tention-columns-on-streams-homepage-are-not-keyboard-accessible-and-dont-have-focus
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
History
|
|
Starting backport for target branches: 8.19, 9.1 https://github.com/elastic/kibana/actions/runs/16801120467 |
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
…225659) closes: elastic#224738 ## Summary 📚 Documents and Retention columns on the Streams home page were not focusable, so keyboard users couldn’t reach the data and screen-reader users heard nothing. This PR adds: - `tabIndex` and descriptive `aria-labels` to the Documents and Retention cells, their loading states, and relevant icons/badges. - Focusable, labelled wrapper around the Documents histogram and a matching `ariaLabel` on the chart canvas. - Properly labelled ILM policy badge/link, DSL text, ∞ icon, and error badge in the Retention column. - Accessible column headers and a table caption. - Removal of redundant roles that triggered axe-core ## How to test 🧪 - Open Observability → Streams with populated data. - Navigate the table using a keyboard - Turn on VoiceOver and navigate through the table elements. --------- Co-authored-by: Joe Reuter <johannes.reuter@elastic.co> Co-authored-by: Abdul Zahid <awahab07@yahoo.com>
…225659) closes: elastic#224738 ## Summary 📚 Documents and Retention columns on the Streams home page were not focusable, so keyboard users couldn’t reach the data and screen-reader users heard nothing. This PR adds: - `tabIndex` and descriptive `aria-labels` to the Documents and Retention cells, their loading states, and relevant icons/badges. - Focusable, labelled wrapper around the Documents histogram and a matching `ariaLabel` on the chart canvas. - Properly labelled ILM policy badge/link, DSL text, ∞ icon, and error badge in the Retention column. - Accessible column headers and a table caption. - Removal of redundant roles that triggered axe-core ## How to test 🧪 - Open Observability → Streams with populated data. - Navigate the table using a keyboard - Turn on VoiceOver and navigate through the table elements. --------- Co-authored-by: Joe Reuter <johannes.reuter@elastic.co> Co-authored-by: Abdul Zahid <awahab07@yahoo.com>
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
3 similar comments
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
@rStelmach should this be skipped? |
|
@gbamparop EDIT: I would skip backports for this PR |
…225659) closes: elastic#224738 ## Summary 📚 Documents and Retention columns on the Streams home page were not focusable, so keyboard users couldn’t reach the data and screen-reader users heard nothing. This PR adds: - `tabIndex` and descriptive `aria-labels` to the Documents and Retention cells, their loading states, and relevant icons/badges. - Focusable, labelled wrapper around the Documents histogram and a matching `ariaLabel` on the chart canvas. - Properly labelled ILM policy badge/link, DSL text, ∞ icon, and error badge in the Retention column. - Accessible column headers and a table caption. - Removal of redundant roles that triggered axe-core ## How to test 🧪 - Open Observability → Streams with populated data. - Navigate the table using a keyboard - Turn on VoiceOver and navigate through the table elements. --------- Co-authored-by: Joe Reuter <johannes.reuter@elastic.co> Co-authored-by: Abdul Zahid <awahab07@yahoo.com>
closes: #224738
Summary 📚
Documents and Retention columns on the Streams home page were not focusable, so keyboard users couldn’t reach the data and screen-reader users heard nothing.
This PR adds:
tabIndexand descriptivearia-labelsto the Documents and Retention cells, their loading states, and relevant icons/badges.ariaLabelon the chart canvas.How to test 🧪