primeng icon indicating copy to clipboard operation
primeng copied to clipboard

Accessibility | Step component bugs and improvements

Open GiacomoDM opened this issue 2 years ago • 5 comments

Describe the bug

The Step component has some accessibility problems:

  1. When navigated with a screen reader, this will read each step two times: one for the li element and one for the a inside, adding links information such as visited and so on. The correct behaviour is the same one as the TabView component, so the aria tags and role should be switched between the a (role tab and all the aria tags) and the li (role presentation and no aria tags).
  2. Currently the steps will never receive focus when navigating with keyboard only. You have to select an element with the mouse to apply focus and then the arrows navigation is available. The correct behaviour is the same one as the TabView component, so when tabbing in the Stepper the focus should go to the active step (the only one with tabindex=0), a consequent tab should leave the Stepper and go to the next focusable element on the page. Shift-tabbing back to the stepper should return focus to the active step.
  3. Currently if you move focus with the arrows il will correctly change the tabindex of the focused element and remove it from the previously focused one. When tabbing out and back again, the focus should go to the active tab, not the last focused tab with arrows.
  4. Disabled or readonly steps don't have aria-disabled as they should.
  5. Disabled or readonly steps still have aria-current='step', that makes the screen reader read them as they were interactable. The aria tag should be present only when not disabled or readonly.

Also, since the Stepper has role tablist and each step role tab, they should also have aria-controls with the id of the controlled element, that in turn should have aria-labelledby. This is more complicated to handle, since the current implementation of the Step component doesn't have a concept of step content. This could be solved by adding a new optional property to the MenuItem: overrideAriaControls or something like that, that delegates to the user the responsibility to correctly link the MenuItem to the actual id of the controlled element. While this would be simple to handle on the Step level, it would be more complicated for all the other components that use a MenuItem, as they should be updated to use this property when present instead of the automatically set one (eg: see TabView that already correctly implements aria-controls etc.)

I will open two different PRs, one for all the 5 points above and one to showcase the implementation of the aria-controls property in the MenuItems for the Step component only. If accepted, it should be ported to all other components too.

Environment

Windows

Reproducer

https://primeng.org/steps/

Angular version

16.X

PrimeNG version

16.3.1

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

18.13

Browser(s)

No response

Steps to reproduce the behavior

No response

Expected behavior

No response

GiacomoDM avatar Sep 26 '23 08:09 GiacomoDM

Hi @GiacomoDM, thanks for report issues and for help and contributing with your PRs. PrimeNG right now is working in accessibility. With your messsage, if i'm not wrong are you saying the implementation that they are doing is not 100% ok and they should add all the things that you wrote in each component?

Accessibility in web development is a "complicated world" and sometimes you don't have the necessary things to test them with elements that a person with a disability uses, so all the help or PRs you can provide here I suppose they would appreciate it.

SoyDiego avatar Sep 27 '23 06:09 SoyDiego

Hi @GiacomoDM,

Thanks a lot for your investigation and PRs. Don't hesitate to open issues or PRs about accessibility, since we're currently implementing it, there would be some points that we've missed. A helping hand is always appreciated.

We appreciate the effort you put in!

cetincakiroglu avatar Sep 27 '23 06:09 cetincakiroglu

Hi @SoyDiego and @cetincakiroglu.

are you saying the implementation that they are doing is not 100% ok and they should add all the things that you wrote in each component?

Yes, they declared the work on accessibility for the Step component done with version 16.1.0 (#13264). We reviewed the development, testing it with an actual blind person and using the screen reader JAWS professional (most used professional screen reader). The bugs here reported are to aim to be compliant to W3C's WCAG 2.1 level of accessibility.

We start accessibility tests as soon as we can, on the components that are declared completed accessibility-wise in each release. We plan to open bugs with PRs for each problem found.

If you want me to describe the issues differently/provide som kind of further proof let me know.

GiacomoDM avatar Sep 27 '23 08:09 GiacomoDM

If you want me to describe the issues differently/provide som kind of further proof let me know.

Thanks for all your support, help and information. We appreaciate the PRs!

SoyDiego avatar Sep 27 '23 09:09 SoyDiego

Hi @GiacomoDM,

Could you please resolve conflicts in your PR's so we can merge them

cetincakiroglu avatar Oct 11 '23 12:10 cetincakiroglu