SAP / ui5-webcomponents-react

A wrapper implementation for React of the UI5 Web Components that are compliant with the SAP Fiori User Experience
https://sap.github.io/ui5-webcomponents-react/
Apache License 2.0
449 stars 101 forks source link

[SF][AnalyticalTable]: A11y - Table header cell of checkbox should have aria-hidden="true" in multi-selection mode #6530

Closed summer2200 closed 4 weeks ago

summer2200 commented 1 month ago

Describe the bug

When Analytical table is in multi selection mode, there's a checkbox in first column of table header. aria-hidden="true" needs to be added to the table header cell element of the checkbox too, because the element is a table header and it does not have any text inside, so it's being detected by AXE tool. Please see below screenshot: Screenshot 2024-10-21 at 14 17 20

Isolated Example

No response

Reproduction steps

1. 2. 3. ...

Expected Behaviour

No response

Screenshots or Videos

No response

UI5 Web Components for React Version

2.2.0

UI5 Web Components Version

2.3.0

Browser

Chrome

Operating System

No response

Additional Context

No response

Relevant log output

No response

Organization

No response

Declaration

Lukas742 commented 1 month ago

Hi @summer2200

adding aria-hidden to the header cell, would lead to the header not being announced at all which is in my opinion incorrect behavior. Please always include the error raised by the tool in the description of the issue and why this error has a negative impact on the users. Also, please see this section on how to test ui5 web components (and ui5 web components for react) components for accessibility issues. Please, especially ensure the following:

please ensure that the issue is not false positive, a real accessibility concern is present, and there is an impact on the users.

summer2200 commented 1 month ago

Hi @Lukas742 Below screenshot shows the description of the issue detected by AXE tool:

Screenshot 2024-10-24 at 14 27 31

Here's the target element being inspected:

Screenshot 2024-10-24 at 14 28 07

And this is why the table header should not be empty: https://dequeuniversity.com/rules/axe/4.10/empty-table-header?application=AxeChrome

Lukas742 commented 1 month ago

Hi @summer2200 there are no specific global specifications regarding this, only that the content or purpose of a cell should be described properly. Also, I think adding aria-hidden or a hidden text element that is only picked up by screen readers, is not correct here.

Please verify whether there is a real accessibility issue. If so, we would need specifications for the intended invisible text, and the request would have to go through the inner source process.

Additionally, it's possible to add invisible text using a plugin hook. Please let me know if you'd like an example of how to do that.

summer2200 commented 1 month ago

Hi @Lukas742 Adding aria-hidden="true" was only my first thought to fix this issue, but I believe adding invisible text to the table header cell works too! Please share an example of it, thanks!

Lukas742 commented 1 month ago

@summer2200, you can find the example here. Depending on your use case, you might also consider using the "invisible messaging" feature of UI5 Web Components instead of adding an invisible element yourself.

Please, let me know if this solution works for you, or if you'd prefer to create an internal BLI.

summer2200 commented 1 month ago

Hi @Lukas742 I get that you're trying to show me how to add invisible text to table header through "accessor" (please correct me if not). But when the table is in "Multiple" selectionMode, how to add invisible text to the "select all" checkbox in table header?

Lukas742 commented 1 month ago

@summer2200 I'm sorry, I've sent you the wrong link or StackBlitz didn't save the changes. Here's the correct link for the example: https://stackblitz.com/edit/ui5wcr-at-xun3j8?file=src%2FApp.tsx

summer2200 commented 4 weeks ago

@Lukas742 Really appreciate your help, I've double confirmed that the table header cell of select all checkbox has its invisible text in DOM, so AXE tool has detected an invalid problem.

So I suppose this issue can be closed now. Thanks!