fix: [Menu]: Add role = 'presentation' to MenuItem for accessibility issue

EightfoldAI / octuple

The Octuple component library

MIT License

17 stars 51 forks source link

fix: [Menu]: Add role = 'presentation' to MenuItem for accessibility issue #788

Closed rkondra-eightfold closed 7 months ago

rkondra-eightfold commented 7 months ago

SUMMARY:

fixes issue: list item does not have a <ul> or <ol> parent element without a role, or a role="list"

GITHUB ISSUE (Open Source Contributors)

JIRA TASK (Eightfold Employees Only):

https://eightfoldai.atlassian.net/browse/ENG-79383

CHANGE TYPE:

[x] Bugfix Pull Request
[ ] Feature Pull Request

TEST COVERAGE:

[x] Tests for this change already exist
[ ] I have added unittests for this change

TEST PLAN:

Open story book > Menu
Click on Menu
Run axe DevTools accessibility tests Before:

After:

Should not see list item does not have a <ul> or <ol> parent element without a role, or a role="list" issue.

codesandbox-ci[bot] commented 7 months ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 84.83%. Comparing base (e828a69) to head (8098723).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #788 +/- ## ========================================== - Coverage 84.83% 84.83% -0.01% ========================================== Files 999 999 Lines 19552 19552 Branches 7302 7302 ========================================== - Hits 16587 16586 -1 - Misses 2885 2886 +1 Partials 80 80 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dkilgore-eightfold commented 7 months ago

For this, use itemProps={{ role: 'presentation' }} as this will break the overall A11y fix for Menu, Dropdown, List, and Select. Or <Menu role="listbox" / >