-
Notifications
You must be signed in to change notification settings - Fork 83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(SITES-2874 Accessibility) - Visual list is not marked up as list. #320
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the curly braces.
For the complexity warning, if you cannot simplify the complexity, and it's code that wasn't introduced by you, it might generate a code smell report but should be fine to merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try the aria role 'list' instead of the 'listitem'
@petcud check warnings not related to the actual change, should we resolve this reports for every PR created as long as it were not generated by this PR? |
Hi @petcud the role "list" is only to be use for the wrapper of the list and the role "listitem" is to be used for the each element from inside the wrapper as suggested in the ticket and in the accessibility guides https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/listitem_role |
@Pareesh is there anything else we should do regarding this ticket? If not could you merge it? |
Ticket: SITES-2874
Added role list and listitem to the required elements