[EuiDataGrid] Remove screen reader text from cell popovers#5087
[EuiDataGrid] Remove screen reader text from cell popovers#5087thompsongl merged 4 commits intoelastic:masterfrom
Conversation
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5087/ |
cee-chen
left a comment
There was a problem hiding this comment.
This LGTM, I can confirm the text no longer shows up visibly on JSON schema and but that screen readers still read out the row/column numbers. Love the elegant solution!
snide
left a comment
There was a problem hiding this comment.
Works as advertised. Only negative I can think of is that this adds one more bit of dom to the datagrid soup, which might possibly effect performance when replicated over X number of cells.
That's a good point. After stepping away for minute, I think this could work with a Fragment, which would negate the added DOM. I'll try it. |
cchaos
left a comment
There was a problem hiding this comment.
Yep, I can too attest that the formatting of the popover contents is now back and the screen-reader text is hidden. 🙇
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5087/ |
|
QA still looks great with the Fragment. Really nice catch @snide @thompsongl! 🙏 |
Summary
Fixes #4957 by rearranging elements in a way that excludes screen reader text from the scope of what is provided to the popover render function.
The
Node.innerTextmethod used by the standard json popover (as well as the custom array popover noted in the issue) does not ignore "hidden" content such as that insideEuiScreenReaderOnly. This PR removes the screen reader text from consideration while not requiring apps with custom render functions to update their methods.Checklist
- [ ] Check against all themes for compatibility in both light and dark modes- [ ] Checked in mobile- [ ] Props have proper autodocs and playground toggles- [ ] Added documentation- [ ] Checked Code Sandbox works for the any docs examples