-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[ Fabric ] Added ariaHidden accessibility prop across all components #14836
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
base: main
Are you sure you want to change the base?
Conversation
…essibility and accessibilityElementsHidden as ( android and ios )
vnext/src-win/Libraries/Components/View/ViewPropTypes.windows.js
Outdated
Show resolved
Hide resolved
@@ -333,7 +330,7 @@ const View: component( | |||
// [Windows | |||
accessible={_accessible} | |||
children={ | |||
importantForAccessibility === 'no-hide-descendants' | |||
computedImportantForAccessibility === 'no-hide-descendants' |
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.
This conditional rendering of the children using childrenWithImportantForAccessibility was a compromise to implement this property in paper since there was no good way to configure the Xaml automation peers to do what we needed. With fabric we have complete control over the automation providers we should be able to avoid using this forked code when running in fabric. I'd really like us to render children as normal when running fabric as this code has caused issues in apps.
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.
That makes sense, I actually thought of removing it but wanted to make sure this doesn't affect the current behaviour for fabric and paper . Now I have removed the conditional rendering of the children using childrenWithImportantForAccessibility for fabric and for paper It remains the same.
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.
on line 280 : children={otherProps.children} this is part of actual view which is used for fabric.
…ildren : childrenWithImportantForAccessibility
Please keep the PR draft until all CI tests pass. |
Description
All three properties (importantForAccessibility, ariaHidden, accessibilityElementsHidden) are properly unified to use the same underlying mechanism.
The behavior matches the expected accessibility standards where elements marked with any of these properties are hidden from assistive technologies along with their descendants ( childeren).
Type of Change
Why
What is the motivation for this change? Add a few sentences describing the context and overall goals of the pull request's commits.
(importantForAccessibility, ariaHidden, accessibilityElementsHidden) are properly unified to use the same underlying mechanism of hiding elements from assistive technologies along with their descendants ( children ).
Resolves [Add Relevant Issue Here]
#14541
What
Any of the three properties being true results in importantForAccessibility: 'no-hide-descendants'
means parent has no-hide-descendants -> IsHiddenByParent helper function correctly walks up the parent chain
Both UIA_IsContentElementPropertyId and UIA_IsControlElementPropertyId return VARIANT_FALSE when hidden
then elements are properly hidden from screen readers
Screenshots
ariahidden.mp4
Testing
tested using playground and accessibility insights
cant be tested in rntester as UIA tree for aria-hidden props would be hidden so cant compare testid
Changelog
yes
Add a brief summary of the change to use in the release notes for the next release.
Added support for (ariaHidden , importantForAccessibility, accessibilityElementsHidden) to have unified underlying behaviour acrross all components.