Skip to content

[ 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

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

iamAbhi-916
Copy link
Contributor

@iamAbhi-916 iamAbhi-916 commented Jul 3, 2025

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

  • New feature (non-breaking change which adds functionality)

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

<View accessible={false} aria-label='yellowv'>
          <Text aria-label="yellow1">
            This text is STILL accessible to screen readers (accessible=false)
          </Text>
          <Button title="This button is STILL accessible" />
        </View>
        <View aria-hidden={true}>
          <Text aria-label="yellow2">
            This text should be HIDDEN from screen readers (aria-hidden=true)
          </Text>
          <Button title="This button should also be HIDDEN" />
        </View>
        <View importantForAccessibility={'no-hide-descendants'}>
          <Text accessibilityLabel="yellow3">
            This text should also be HIDDEN (importantForAccessibility)
          </Text>
          <Button title="This button should also be HIDDEN2" />
        </View>

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.

@iamAbhi-916 iamAbhi-916 requested a review from a team as a code owner July 3, 2025 10:17
@iamAbhi-916 iamAbhi-916 self-assigned this Jul 3, 2025
@@ -333,7 +330,7 @@ const View: component(
// [Windows
accessible={_accessible}
children={
importantForAccessibility === 'no-hide-descendants'
computedImportantForAccessibility === 'no-hide-descendants'
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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
@iamAbhi-916 iamAbhi-916 requested a review from acoates-ms July 4, 2025 06:30
@anupriya13
Copy link
Contributor

Please keep the PR draft until all CI tests pass.

@iamAbhi-916 iamAbhi-916 marked this pull request as draft July 7, 2025 06:00
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