Skip to content

Conversation

@chrisglein
Copy link
Member

@chrisglein chrisglein commented Aug 17, 2023

Description

Text components were showing in the accessibility tree but not with any value. This change has UIA_NamePropertyId default to the text value but can be overriden by accessibilityLabel.

Type of Change

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

Why

Text components were showing in the accessibility tree but not with any value.

Resolves #12030

What

Added a virtual DefaultAccessibleName() that enables individual Fabric components to provide a fallback if accessibilityLabel isn't specified. Then ParagraphComponentView overrides this to provide its string value.

Screenshots

Add any relevant screen captures here from before or after your changes.

Before After
image image

Testing

  • Tested first in the Artifical Chat app
  • Then tested in PlaygroundComposition on the Text test page to ensure that 1. Names started showing up, and 2. accessibilityLabel was still respected if specified.

Changelog

Yes? This is somewhere between bug fix (expected behavior not working) and feature (it wasn't working before).

Fabric Text components use their text value as the accessibility name by default (if accessibilityLabel is not specified

Microsoft Reviewers: Open in CodeFlow

Text component falls back to the content if accessibleLabel isn't set
@chiaramooney
Copy link
Contributor

Didn't see the PR. Left some comments on the commit itself. chrisglein@a58cc6d

Can you add this PR to this issue #11901 in the "Name" row. And update status to In Progress?

Can you also verify that you've tested the FastRefresh case?

@chrisglein
Copy link
Member Author

Didn't see the PR. Left some comments on the commit itself. chrisglein@a58cc6d
Can you add this PR to this issue #11901 in the "Name" row. And update status to In Progress?
Can you also verify that you've tested the FastRefresh case?

  • Addressed the comments on the commit with an additional commit.
  • I've validated fast refresh on adding/changing/removing accessibilityLabel (and accessibilityRole, since I touched that line).
    • Should I modify the static text that's there right now to make it interactive with a Switch to add/remove role? Or is that overkill?
  • And I've added a note to the issue.

Copy link
Contributor

@chiaramooney chiaramooney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! The switch to turn on/off setting accessibilityRole is probably not needed for now, but would be a good thing to have once we move into adding functional testing for Fabric. So up to you if you have the time.

@chrisglein chrisglein merged commit c030b7c into microsoft:main Aug 22, 2023
@chrisglein chrisglein deleted the textAccessibleName branch August 22, 2023 19:57
@jonthysell jonthysell added the New Architecture Broad category for issues that apply to the RN "new" architecture of Turbo Modules + Fabric label Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Accessibility Area: Fabric Support Facebook Fabric Area: Text New Architecture Broad category for issues that apply to the RN "new" architecture of Turbo Modules + Fabric

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Fabric Text components do not provide their text value (Name) to accessibility tree

4 participants