-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Accessibility and UIA Support for XAML Fabric implementation #15466
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?
Accessibility and UIA Support for XAML Fabric implementation #15466
Conversation
|
/azp run PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
iamAbhi-916
left a comment
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.
lgtm
but one thing we are not traversing into children, thats intentional? ContentIslandComponentView this will be the boundary
| facebook::react::Point pt, | ||
| facebook::react::Point &localPt, | ||
| bool ignorePointerEvents) const noexcept { | ||
| facebook::react::Point ptLocal{pt.x - m_layoutMetrics.frame.origin.x, pt.y - m_layoutMetrics.frame.origin.y}; |
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.
Is it possible there's a scale or other transform that needs to be applied?
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.
Yes, this is a valid concern. The hitTest method should consider the scale/transform. Looking at the ViewComponentView::hitTest implementation, it correctly handles the origin offset
| facebook::react::Point ptLocal{pt.x - m_layoutMetrics.frame.origin.x, pt.y - m_layoutMetrics.frame.origin.y}; | ||
|
|
||
| // Check if the point is within the bounds of this ContentIslandComponentView. | ||
| // This ensures that hit tests correctly return this view's tag for UIA purposes, |
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.
Is this function ("hitTest") only called for UIA, or can it be called for other reasons?
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.
hitTest is called for multiple purposes, not just UIA:
- UIA/Accessibility via RootComponentView::UiaProviderFromPoint
- Pointer/Touch Events via CompositionEventHandler
- Focus Navigation:
|
/azp run PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Yes, this is intentional and correct. Here's why:
|
Description
Accessibility and UIA Support for XAML Fabric implementation
Type of Change
Why
To ensure Accessibility props are read in Accessibility Insights.
Resolves [https://github.com//issues/15325]
What
Override hitTest in ContentIslandComponentView to properly handle hit testing for content inside the ChildSiteLink/ContentIsland. The method is checking if the point is within the ContentIslandComponentView's bounds
If yes, return the tag of the ContentIslandComponentView itself (or the tag that represents it properly)
This allows the accessibility tree to properly identify the container of the XAML content
Screenshots & Testing
xaml.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.
Accessibility and UIA Support for XAML Fabric implementation
Microsoft Reviewers: Open in CodeFlow