-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix for heading level for accessibility #15193
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
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
Description: Please improve, thanks |
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.
Test cases?
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.
Pull Request Overview
This PR fixes a bug where the accessibilityLevel
prop was not working properly in React Native Windows Fabric architecture due to incorrect casting in the native layer. The issue was that native code was casting props to the base ViewProps
class instead of the Windows-specific HostPlatformViewProps
class where accessibility properties are actually defined.
Key changes:
- Fixed casting to use
HostPlatformViewProps
instead ofViewProps
in accessibility-related code - Added proper null checking for the cast pointer to prevent potential crashes
- Ensured all Windows-specific accessibility properties are accessed correctly
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionViewComponentView.cpp | Fixed casting to HostPlatformViewProps and updated all accessibility property references to use the correct cast |
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionDynamicAutomationProvider.cpp | Fixed casting to HostPlatformViewProps and added null checking for safer property access |
change/react-native-windows-48182af8-4e3f-46a5-a980-f041f56ff6ce.json | Added changelog entry for the accessibility fix |
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionViewComponentView.cpp
Outdated
Show resolved
Hide resolved
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionDynamicAutomationProvider.cpp
Outdated
Show resolved
Hide resolved
PR title should be a short summary in imperative mood ( A good commit message
Please refer How to Write a Git Commit Message. |
Add a brief summary of the change to use in the release notes for the next release. |
The commit message starts with Fix |
|
…micAutomationProvider.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ComponentView.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
A line of actual summary should be added after that template line which is missing. The script picks up summary of line after that template line. |
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionDynamicAutomationProvider.cpp
Show resolved
Hide resolved
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
Description
The issue was that accessibilityLevel prop was not working properly in React Native Windows Fabric architecture. While the prop was defined in TypeScript and JavaScript, and the UIA (UI Automation) infrastructure existed, there was a casting problem in the native layer.
Type of Change
Fixes
microsoft/react-native-gallery#699
Why
heading level for accessibility is not defined
What
CompositionDynamicAutomationProvider.cpp and CompositionViewComponentView.cpp were incorrectly casting props to the base facebook::react::ViewProps class instead of the Windows-specific facebook::react::HostPlatformViewProps class where the accessibilityLevel property is actually defined.
Testing
yarn install && yarn build passed
Tested view component in playground app
699-after.mp4
Changelog
Should this change be included in the release notes: yes
Add a brief summary of the change to use in the release notes for the next release.
Microsoft Reviewers: Open in CodeFlow