search

PerfectMemory / ngx-contextmenu

A context menu component for Angular
https://perfectmemory.github.io/ngx-contextmenu
MIT License
42 stars 12 forks source link

Accessibility issue with Context Menu #26

Closed innov-richa76 closed 1 year ago

innov-richa76 commented 1 year ago

Describe the bug When we are trying to use ANDI tool for checking the accessibility issue for ngx-contextmenu, we are seeing the error the below error [Element referenced by [aria-labelledby] with [id=menubutton] not found.]. This is happening because aria-labelledby="menubutton" doesn't exist anywhere.

To Reproduce Steps to reproduce the behavior:

1.Go to https://perfectmemory.github.io/ngx-contextmenu/iframe.html?args=&globals=backgrounds.grid:!false;backgrounds.value:!hex(F8F8F8)&id=context-menu-demo--demo&viewMode=story 2.Right Click on the text to see the context menu 3.Open developer tool or use ANDI tool 4.See the below image highlighted with 'menubutton' as aria-labelledby for context-menu

image

Expected behavior The aria-labelledby attribute value = 'menubutton' should exist. Stackblitz Example Please fork this [base example](https://stackblitz.com/edit/ngx-contextmenu-example) to demonstrate the issue you're experiencing. image

innov-richa76 commented 1 year ago

Hi Team, Can you please confirm if this is actually the issue or we need to add something from our end? Thanks.

sroucheray commented 1 year ago

Hi @innov-richa76,

You are right this is an issue in the lib. I have two options:

  1. remove the aria-labelledBy altogether
  2. set the aria-labelledBy attribute value with the id of the context menu target item (if exists, otherwise go back to option 1)
    • For submenus the id should be placed on the menu that triggered the submenu, but that's not possible because it is a <ng-tempate>. So I am tempted to do option 1. What do you think ?
innov-richa76 commented 1 year ago

Hi @sroucheray ,

Thank you for getting back to me. From accessibility point of view, adding aria-labelledBy attribute value with the id of the context menu to the target item would be a great solution but as you mentioned for sub menus it wont be possible, so it will be okay to remove the aria-labelledBy attribute altogether. However, anything is fine with us as long as we have the accessibility in place.

Thanks.

sroucheray commented 1 year ago

Fixed in 16.0.1