SAP / fundamental-ngx

Fundamental Library for Angular is SAP Design System Angular component library
https://sap.github.io/fundamental-ngx
Apache License 2.0
269 stars 129 forks source link

[Accessibility] ACC-270.6 (Level A) Popover Component - cdk-focus-trap-anchor div in popover body has tabindex=0 #11469

Closed harshitha-s-kanchan closed 1 month ago

harshitha-s-kanchan commented 9 months ago

Is this a bug, enhancement, or feature request?

Bug

Describe your proposal.

When there is an interactive element inside the fd-popover-body, the cdk-focus-trap-anchor <div> in the beginning and at the end of the popover has tabindex set to 0 which is causing accessibility issue.

<div class="cdk-visually-hidden cdk-focus-trap-anchor" aria-hidden="true" tabindex="0"></div>

Can you handle that on the application side

No, since we cannot programmatically access this div

Which versions of Angular and Fundamental Library for Angular are affected? Please, specify the exact version. (If this is a feature request, use current version.)

Angular - 15 NGX - 0.43.21 (Issue also found in latest version)

If this is a bug, please provide steps for reproducing it; the exact components you are using;

By testing the popover using 'Access assistant' tool. Below are the screenshots,

Screenshot 2024-02-26 at 7 54 14 PM Screenshot 2024-02-26 at 7 55 01 PM Screenshot 2024-02-26 at 7 55 18 PM

In case this is Accessibility related topic, did you consult with an accessibility expert? If not, please do so and share their recommendations.

This is a bug raised by Central Accessibility Team in our application. Below is the description provided by them,

"Avoid placing inactive elements in the focus order"

"Content providers should generally ensure that only active elements receive keyboard focus. Non-interactive elements such as labels and headings should be removed from the focus order by not having a tabindex attribute set. For interactive elements that are in the focus order but rendered inactive, tabindex="-1" can be used to remove them from the focus order. In certain situations such as controls like submit buttons that are currently disabled until the user enters the correct information it may be helpful to have the control in the focus order with a message letting the user know what must be completed in order to have it become enabled. In these cases proper markup such as aria-disabled may be needed to communicate the disabled state of the control to the user of assistive technology. Another example of inactive content that should be keyboard focusable is read-only content that must be copied such as codes, etc."

droshev commented 8 months ago

@nikolay-kolarov Can you advise us here?

nikolay-kolarov commented 8 months ago

Hello @droshev,

According to the latest recommendation, you can use role="none presentation" on the elements that are used to keep the focus in the dialog: https://www.w3.org/TR/wai-aria-1.2/#presentation.

Best Regards, Nikolay

droshev commented 8 months ago

@harshitha-s-kanchan What exactly is the problem with the screen reader? We are using the focus trap and they got the same problem. Here is their answer.

harshitha-s-kanchan commented 8 months ago

@droshev The issue is not with the screen reader. The issue is reported by central accessibility team found by their automation tool. The issue can be seen when inspecting the popover using Access Assistant tool (chrome extension). Error screen shot attached in the ticket description.

droshev commented 8 months ago

@harshitha-s-kanchan Can you ask them to follow up with the library and the issue i linked? According to them there isn't an issue.

droshev commented 6 months ago

@harshitha-s-kanchan any update?

AccessAbilityOfficer commented 5 months ago

@droshev - I've hit this same issue with one of my clients - so not fixed

zarend - wants the end user impact:
Screen Reader users have 2 "blank" tab stops at the top of the screen, followed by a visual tab stop for the clients logo that is a link.

Within Mat-sidenav-container
<div tabindex="0" class="cdk-visually-didden cdk-focus-trap-anchor" aria-hidden="true"></div>

IsmaelNennouche commented 4 months ago

I found a solution that works for me: Replace cdkTrapFocus with cdkTrapFocus="isActive" Where isActive is a boolean initialized with value false, and is set to true when showing the modal, and back to false when hiding it.

It looks something like this: < div cdkTrapFocus="{{ isActive }}" >

mikerodonnell89 commented 1 month ago

Closing, see https://github.com/SAP/fundamental-ngx/issues/11278#issuecomment-2405996441

@AccessAbilityOfficer the issue you're describing in your latest comment (re: blank tab stops) seems to me like a separate issue from what was filed in the original post by @harshitha-s-kanchan. So feel free to open another issue with more details about what you've described (we will likely need a code sample as your comment lacks details to reproduce the issue)