Fix Datatable header button labels

[Bubballoo3]

Fixes #4650 on all instances of datatables: the files table, path selector, and active jobs. The central issue causing the screen reader to read extraneous text was that the aria-label on the sort buttons was being interpreted as part of the header content. This issue persisted with other approaches, like adding .sr-only spans or changing from aria-label to title.

The only approach that worked was moving the text outside of the table entirely and referencing with aria-describedby. This is not without drawbacks, specifically when the button is read (not focused, eg. with arrow keys) where it is announced as simply 'button'. This could be the same issue with NVDA as mentioned in #4943 (comment). Regardless, the button announces as expected when focused, and the substantial improvement to every row's header announcement makes the change worth the chance of less clarity on a single element.

[Bubballoo3]

Aside from the issue noted above with NVDA, I have tested and found this works well with Windows Narrator and Apple VoiceOver.

[johrstrom]

I'm not a javascript expert, so I don't know if using this. is redundant here like it would be in ruby using self. In any case, why not just use this.tableId below?

[Bubballoo3]

It wasn't my first instinct either, but it appears this gets lost when referenced inside a callback. (from console.logs it was undefined). But assigning it to a normal variable instead of referencing the instance resolved the issue. This thread discusses the central issue https://stackoverflow.com/questions/8317724/javascript-class-variable-scope-in-callback-function

[johrstrom]

OK that makes sense.

[johrstrom]

This may be an aside - and we don't have to fix it here, but we appear to be using the phrase 'id' like tableId here, but really it's a tableIdSelector with # prefix.

Again, we don't have to fix it here, just noting it because apparently CONTENTID is also not an id, but an id selector.

[Bubballoo3]

That is a great suggestion because I spent a lot of time troubleshooting the fact that tableId from the path selector (real id) and CONTENTID (selector) were not the same format.

[johrstrom]

While I think this reads the headers better in the rows themselves, it comes at the cost of a button that doesn't have a correct label and what that button does when pressed is not announced. I.e., activate to sort column descending (from aria-label) when it's focused and sorted ascending or clicked (aria-sorted).

I think we need those.

[Bubballoo3]

it comes at the cost of a button that doesn't have a correct label

I think this is arguable, while aria-describedby is not the ideal method of labeling, the key is that the non-text element has descriptive text associated with it. From https://www.w3.org/TR/WCAG20/#text-equiv-all,

If non-text content is a control or accepts user input, then it has a name that describes its purpose. (Refer to Guideline 4.1 for additional requirements for controls and content that accepts user input.)

And the referenced section 4.1 reads

For all user interface components (including but not limited to: form elements, links and components generated by scripts), the name and role can be programmatically determined; states, properties, and values that can be set by the user can be programmatically set; and notification of changes to these items is available to user agents, including assistive technologies. (Level A)

Then from the MDN definition of aria-describedby

It is used to establish a relationship between widgets or groups and the text that describes them.

Though it also differentiates between aria-labelledby and aria-describedby as follows

a label describes the essence of an object, while a description provides more information that the user might need.

Which is probably the weakest link of the whole effort, but the fact that NVDA randomly started following aria-labelledby as part of the header text forced the choice for us.

I read the key pieces of the WCAG guideline being that 'a name can be programatically determined' and 'notification of changes to these items is available to user agents'. When I focus the button with tab, the correct label is read, which seems to me to satisfy the first condition. Even if there are certain cases where NVDA or other agents do not follow the aria-describedby, I don't think the rule is broken.

What I take away from that is that the only piece we are currently missing is maybe aria-live set on the header spans, such that when the table re-draws after sorting, the updated sorting label is announced. I am guessing we got this for free before since the changing text was within user focus whenever the button was pressed.

At the end of the day, it is certainly a trade-off, and I am not in love with either solution. I do think that these changes improve the user experience, and so are worth trying to fit into the letter of the rules. However if this is a compromise we are not comfortable with (as Datatables was not when this approach was first suggested to them) I think that makes sense and I would be happy to leave things as-is.

[johrstrom]

I think the larger issue is the not the label of the button - but it's purpose (to sort) and announcements when it's used (to sort).

I believe it's critical to get messages like sorted ascending (after sorting) and to be told that the keypress will sort the table through Activate to ...

I hacked around for a minute with the same strategy you had - to rewrite the th elements. Only I tried to get the same structure we had in 4.1.

Can you take a look and let me know what you think?

files.diff.txt

I don't believe to compromise here because we had something good in 4.1. And I also don't believe we need aria-live functionality, I think that's likely overkill and something much more simple can suffice.

[johrstrom]

Can you take a look and let me know what you think?

🤦‍♂️ and of course the diff i've provided only works for NVDA. the windows screen reader continues to read the extra text.

[Bubballoo3]

And it seems to not play well with Apple VoiceOver either, as it somehow both repeats the label on every row, and doesn't announce updates 😂.

I've made some good progress with adding aria-live, it looks like the biggest drawback is that we have to move the header labels from a d-none to a sr-only. While there is nothing focusable inside, this does mean that it appears as visible text (when navigating with arrow keys) and so I had to move it to be after the table so that it gets in the way less often.

[johrstrom]

Oh boy. Still, there's no way this has to be this hard. I fee like we're overthinking it. Maybe all we need is the aria-sort label to convey it's "sorting capability". Maybe we should be looking at examples like this to see how they behave.

https://www.w3.org/WAI/ARIA/apg/patterns/table/examples/sortable-table/

I feel like this shouldn't be this hard with new divs and aria-live announcements. Clearly we need to modify these elements, but I'm convinced we're overthinking this.

That said - can you use the headerCallback like I used which just passes the thead element to the function. That'll side step the whole Id vs selector issue entirely.

[Bubballoo3]

It is using headerCallback now. I agree that it feels like overkill, and I'd definitely be open to anything else you find to work. I feel like we have such a clear idea of what the behavior ought to be, and it's really hard to believe how much it took to get there. That being said, I think we are getting what we want out of it now so I am wary of switching up the approach without good reason.

Taking a look at the linked example I am quite surprised at how little it aligns with the goals we've established for this PR. You have to infer the function of the buttons from text in the table caption, and (at least on VoiceOver) I don't get any update of sorting directly after the button is clicked. Moving away and back to the button will tell you that the row is sorted (via the aria-sort), but overall it feels like it falls far short of my idea of how the table should behave. Not that it isn't worth looking into at all, but I am surprised that an official solution could have so many problems jump out at me.

[Bubballoo3]

I have simplified the rearrangement to get closer to what we had in 4.1. This seems to result in good behavior on NVDA/chrome, although VoiceOver still announces the aria-labels as the header in rows. All things considered, I think that is a fine compromise to keep things simple and contained in the table header.

[johrstrom]

LGTM. Windows Narrator is a little buggy still, but I'm OK with that. NVDA on Chrome and Firefox works well.