Repository navigation
fix(button): forward expanded state to the interactive control - #5148
pemalama12 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
|
Found potential problems with the pull request:
|
MikitasK
left a comment
MikitasK
left a comment
There was a problem hiding this comment.
good job 👍 👍
just a few comments from my side:
| textColor: customTextColor, | ||
| children, | ||
| 'aria-label': ariaLabel, | ||
| 'aria-expanded': ariaExpanded, |
There was a problem hiding this comment.
could you please declare 'aria-expanded' in Props with a short JSDoc (just like FAB.tsx does)
docs only list props declared in src/components, so right now it has no entry on Button docs page. aria-label & accessibilityHint above are declared for the same reason
| onPressOut={hasPassedTouchHandler ? handlePressOut : undefined} | ||
| delayLongPress={delayLongPress} | ||
| aria-label={ariaLabel} | ||
| aria-expanded={ariaExpanded} |
There was a problem hiding this comment.
verified natively on iPhone simulator:
| Collapsed | Expanded |
|---|---|
 |
 |
accessibility tree matches: button reports value: "expanded". ✅
[non-blocking] accessibilityState & aria-checked/aria-selected still land on the outer Surface so accessibilityState={{ expanded: false }} quietly contradicts this.
worth a follow-up issue?
|
|
||
| expect( | ||
| screen.getByRole('button', { name: 'Show details', expanded }) | ||
| ).toBeEnabled(); |
There was a problem hiding this comment.
[nit] expanded is asserted by query option (not by toBeEnabled()) so a regression shows up as "element not found" rather than real cause
RTL 14 has toBeExpanded() / toBeCollapsed(). splitting into two cases would assert it directly:
expect(screen.getByRole('button', { name: 'Show details' })).toBeExpanded()
Motivation
A Button used to show or hide content does not expose its expanded state on the interactive control.
For example,
<Button aria-expanded={open} onPress={toggle}>Details</Button>putsaria-expandedon the outer Surface. The element withrole="button"has no expanded state. Assistive technology cannot tell whether the content is open.Forward
aria-expandedto TouchableRipple alongside the existing accessibility props. The public prop type already supports this attribute.Related issue
No existing issue found for this specific Button behavior. The reproduction and before/after results are included below.
Screenshots / Videos
No visual change.
Verified the actual DOM and accessibility tree in the running Expo web example:
truearia-expanded="true"falsearia-expanded="false"After the fix, the outer Surface no longer carries the attribute.
Test plan
To repeat the manual web check:
onPresshandler. Passaria-expanded={true}to one andaria-expanded={false}to the other.aria-expanded.Verified these steps in the Codex in-app browser with the repository's Expo example app. I did not run iOS or Android device tests.
Automated validation with Node 24.13.0 and Yarn 4.9.1:
yarn test --maxWorkers=2: 704 passed, 1 skipped; all 234 snapshots passed.yarn typecheck: passed.yarn lint: passed.yarn prepack: passed.