Skip to content
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

feat: [Select] make dropdown role=group and pass correct id to options list for a11y #897

Merged
merged 3 commits into from
Oct 25, 2024

Conversation

sprajapati-eightfold
Copy link
Contributor

@sprajapati-eightfold sprajapati-eightfold commented Oct 23, 2024

SUMMARY:

The Select Component in octuple internally uses DropDown and List component to show the list of options. Fixing the following a11y issues found by Axe Devtools in Select component:-

  1. Inside the Select component, the dropdown div is the parent of list (ul/ol) but both contain role="listbox". This raises an issue that listbox role could not be inside another listbox role. Hence giving the role="group" for outer div.
Screenshot 2024-10-21 at 11 20 04
  1. Inside the Select component, the aria-label is missing from the input field list (ul/ol) which has the role="listbox". This is because correct id has not been passed onto the <ul> element so it could be corrected labeled by the <input> element using aria-controls.
Screenshot 2024-10-21 at 11 19 33

GITHUB ISSUE (Open Source Contributors)

JIRA TASK (Eightfold Employees Only):

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

CHANGE TYPE:

  • Bugfix Pull Request
  • Feature Pull Request

TEST COVERAGE:

  • Tests for this change already exist
  • I have added unittests for this change

TEST PLAN:

  1. Run storybook with yarn storybook
  2. Inspect the Select component. Verify the <input> and <ul role="listbox"> elements are correctly linked through aria-controls={id} (red boxes in below screenshot)
  3. Verify the dropdown div (blue box in below screenshot) now contains role="group", which fixes nested the role="listbox" elements issue.
Screenshot 2024-10-23 at 16 47 12

Copy link

codesandbox-ci bot commented Oct 23, 2024

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.

@sprajapati-eightfold sprajapati-eightfold changed the title feat: make dropdown role=group and fix id for a11y in Select feat: [Select] make dropdown role=group and pass correct id to options list for a11y Oct 23, 2024
Copy link

codecov bot commented Oct 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.68%. Comparing base (6ed1508) to head (58d8842).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #897      +/-   ##
==========================================
+ Coverage   84.67%   84.68%   +0.01%     
==========================================
  Files        1099     1099              
  Lines       20418    20419       +1     
  Branches     7719     7720       +1     
==========================================
+ Hits        17288    17291       +3     
+ Misses       3043     3041       -2     
  Partials       87       87              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sprajapati-eightfold sprajapati-eightfold merged commit 758e580 into main Oct 25, 2024
7 of 10 checks passed
dcoblentz-eightfold pushed a commit that referenced this pull request Oct 31, 2024
…s list for a11y (#897)

* feat: make dropdown role=group and fix id for a11y in Select

* feat: make dropdown role=group and fix id for a11y in Select-update snapshots

* feat: select a11y fixes - improve code coverage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants