Skip to content

fix(dropdown): only set aria-controls while the overlay is open - #1172

Open
agundewar-eightfold wants to merge 2 commits into
mainfrom
fix/dropdown-aria-controls
Open

agundewar-eightfold wants to merge 2 commits into
mainfrom
fix/dropdown-aria-controls

Conversation

@agundewar-eightfold

@agundewar-eightfold agundewar-eightfold commented Sep 29, 2026 •

Copy link
Copy Markdown

SUMMARY:

Dropdown set aria-controls on its reference element on every render, but the overlay element that id points to is only mounted while the dropdown is open. While closed, aria-controls referenced an element that does not exist, which fails WCAG 4.1.2 (Name, Role, Value) in accessibility audits.

aria-controls is now only present while the overlay is rendered:

  • Cloned reference element: aria-controls is dropdownId when open and omitted when closed.
  • ariaRef reference element: the attribute is set when the overlay opens and removed when it closes. A value the consumer set on the element is left untouched, as before.

This affects every component that renders a Dropdown trigger (for example Menu buttons, Navbar, Pagination, Table filters and dropdown Tabs). Closed triggers no longer carry aria-controls; open triggers are unchanged.

GITHUB ISSUE (Open Source Contributors)

N/A

JIRA TASK (Eightfold Employees Only):

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

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. yarn storybook → Dropdown story. With the dropdown closed, inspect the trigger button: aria-expanded="false" and no aria-controls attribute.
  2. Open the dropdown. The trigger has aria-expanded="true" and aria-controls equal to the id of the rendered overlay.
  3. Close the dropdown (click the trigger or press Escape). aria-controls is removed again and focus returns to the trigger.
  4. Repeat steps 1–3 on a story that uses ariaRef (e.g. a TextInput-based dropdown). The attribute is added and removed on the ariaRef element.
  5. Regression: Menu, Navbar, Pagination, Table filter and dropdown Tabs stories open, close, and navigate by keyboard as before.

Closed dropdowns pointed aria-controls at an unmounted overlay id (WCAG 4.1.2).
@codesandbox-ci

codesandbox-ci Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

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

codecov Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.46%. Comparing base (f39c1bf) to head (c5eb59c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1172      +/-   ##
==========================================
- Coverage   85.46%   85.46%   -0.01%     
==========================================
  Files        1231     1231              
  Lines       21780    21786       +6     
  Branches     8295     8300       +5     
==========================================
+ Hits        18614    18619       +5     
- Misses       3078     3079       +1     
  Partials       88       88              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/components/Dropdown/Dropdown.tsx
@agupta-eightfold

Copy link
Copy Markdown
Collaborator

src/components/Select/Select.tsx:1323

This aria-controls is unconditional but the Menu carrying that id only renders while dropdownVisible, so Select still has the dangling idref this PR is fixing, and the new ariaRef logic cannot reach it since Select sets the attribute itself. aria-owns at 1276 is already gated the right way, so this line looks missed. Note Select is a combobox, so the fix here is keeping the menu mounted rather than gating.

@agundewar-eightfold

Copy link
Copy Markdown
Author

src/components/Select/Select.tsx:1323

@agupta-eightfold Good catch, fixed in c5eb59c. Keeping the menu mounted would mean a larger Dropdown change, and the menu also isn't rendered while loading or when no options match. So aria-controls is now gated like aria-owns at 1276: dropdownVisible ? selectMenuId?.current : undefined. I added a test for the closed state (no aria-controls) and the open state (aria-controls → rendered menu id), and updated the 13 Select snapshots that only dropped aria-controls="list-".

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.

2 participants