storage: Improve table accessibility #20471
Conversation
|
This unfortunately brings back the Patternfly warning. |
Fixed with a hack. aria-hidden is stronger than aria-label, at least when looking at the accessibility tree in Firefox. |
mvollmer
force-pushed
the
storage-aria-hide-icons
branch
from
74a6d5f
to
de52b45
Compare
Replaced with a slightly better hack. Using |
| <Th>{_("Location")}</Th> | ||
| <Th className="storage-size-column-header">{_("Size")}</Th> | ||
| <Th aria-label={_("Actions")} /> | ||
| { show_icons && <Td aria-hidden="true" /> } |
There was a problem hiding this comment.
As far as I can tell, this is not a "hack" but completely valid.
I looked at docs for thead, tr, td, th. It seems there's nothing stopping anyone from putting either a td or th inside of a parent element of tr, even if the parent of that tr is a thead or tbody (or in some cases, even just table, if there is no caption, thead, and tbody).
There was a problem hiding this comment.
As far as I can tell, this is not a "hack" but completely valid.
I was just reading this: patternfly/patternfly#6272
Accessibility rules have changed,
<td>s in theads are no longer acceptable.
shrug
| @@ -608,19 +608,19 @@ export const PageTable = ({ emptyCaption, aria_label, pages, crossrefs, sorted, | |||
| </Card>); | |||
| } else { | |||
| const cols = [ | |||
| <Td key="1" onClick={onClick}> | |||
| <Th scope="row" key="1" onClick={onClick}> | |||
There was a problem hiding this comment.
This would be correct from the HTML side of things. I haven't looked to see if there are any stylistic implications (if it changes the look) yet, but I'll do that in a little bit.
Good catch on seeing that scope is deprecated for <td>! I felt it was just for <th> at first, but saw it used and mentioned for <td>, but I guess I saw old docs and skipped that part of the documentation on MDN. Thanks for finding and mentioning it!
|
BTW: It makes sense that |
mvollmer
force-pushed
the
storage-aria-hide-icons
branch
2 times, most recently
from
970c1ad
to
ba78f28
Compare
Hide the icons from screenreaders, and clarify the scope of things.
Patternfly complains about empty "Th" elements, but a empty "Td" looks just the same and since we hide it anyway it doesn't hurt accessibility to use the wrong kind here. See https://github.com/patternfly/patternfly/issues/6658
mvollmer
force-pushed
the
storage-aria-hide-icons
branch
from
ba78f28
to
cc1f518
Compare
Hide the icons from screenreaders, and clarify the scope of things.