-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Implement ISelectionProvider and ISelectionItemProvider #14019
Implement ISelectionProvider and ISelectionItemProvider #14019
Conversation
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionDynamicAutomationProvider.cpp
Outdated
Show resolved
Hide resolved
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionDynamicAutomationProvider.h
Outdated
Show resolved
Hide resolved
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionDynamicAutomationProvider.cpp
Outdated
Show resolved
Hide resolved
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionDynamicAutomationProvider.cpp
Show resolved
Hide resolved
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.
In general, I didn't understand when and what should be addRef'd. If you can provide some clarity here, it would be helpful.
My sincere advice here is to move into smart pointers. Then, there won't be a need to manually manage memory for each and every object. In Windows, we rely on the winrt library (see winrt::com_ptr
) to work with COM interfaces.
https://learn.microsoft.com/en-us/uwp/cpp-ref-for-winrt/com-ptr
https://learn.microsoft.com/en-us/windows/uwp/cpp-and-winrt-apis/consume-com
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionDynamicAutomationProvider.cpp
Outdated
Show resolved
Hide resolved
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionDynamicAutomationProvider.cpp
Outdated
Show resolved
Hide resolved
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionDynamicAutomationProvider.cpp
Outdated
Show resolved
Hide resolved
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionDynamicAutomationProvider.cpp
Outdated
Show resolved
Hide resolved
So unfortunately the methods here are defined by UIA (we are just adding implementations for them) so I can't change the type values of the IRawElementProviderFragment** types in most cases. My understanding is if we are returning a reference to an object then we need to AddRef so the object is not destroyed prematurely. In the cases when we don't add AddRef but it is needed the app will crash when the program executes. |
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionDynamicAutomationProvider.cpp
Show resolved
Hide resolved
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionDynamicAutomationProvider.cpp
Outdated
Show resolved
Hide resolved
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionDynamicAutomationProvider.cpp
Show resolved
Hide resolved
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionDynamicAutomationProvider.cpp
Outdated
Show resolved
Hide resolved
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionDynamicAutomationProvider.cpp
Show resolved
Hide resolved
…ey/react-native-windows into cm-selection-provider
|
||
struct AccessibilityState { | ||
bool disabled{false}; | ||
std::optional<bool> selected{std::nullopt}; // [Windows] - Do not remove; required for Windows ISelectionItemProvider Implementation |
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.
Will these ever get upstreamed? Are the new props actually from aria?
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.
Hard to say. I will file an issue to request they get upstreamed but it seems RN only decided to implement a subset of the aria props. The new props are real aria props. Meta just hasn't integrated them into the platform. Maybe they aren't needed for accessibility on mobile?
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.
) Adds support for ISelectionProvider interface Adds support for ISelectionItemProvider interface Adds support for accessibilityState:selected Adjusts AccessibilityState native type on Windows to be std::optional instead of bool Adjusts AccessibilityState native type on Windows to include multiselectable and required fields of type std::optional Adds multiselectable and required fields to AccessibilityState type in JS with type boolean | undefined Adds support for aria-multiselectable and aria-required. Adds support for the following accessibilityActions: addToSelection, removeFromSelection, select Fixes crash in AccessibilityInsights.
) * Implement ISelectionProvider and ISelectionItemProvider (#14019) Adds support for ISelectionProvider interface Adds support for ISelectionItemProvider interface Adds support for accessibilityState:selected Adjusts AccessibilityState native type on Windows to be std::optional instead of bool Adjusts AccessibilityState native type on Windows to include multiselectable and required fields of type std::optional Adds multiselectable and required fields to AccessibilityState type in JS with type boolean | undefined Adds support for aria-multiselectable and aria-required. Adds support for the following accessibilityActions: addToSelection, removeFromSelection, select Fixes crash in AccessibilityInsights. * [Fabric] Implement IRangeValueProvider and Adjust IValueProvider Implementation (#14212) * IRangeValue Provider * Adjust ReadOnly to Use Prop Value * Complete IRangeValueProvider Implementation * Change files * Update Snapshots * Rework modal to be implemented entirely using public APIs (#14256) * Rework modal implementation to use public APIs * shutdown fix * Change files * format * remove dead code * format * fixes * fix * UIA tree for root component should be kept distinct from main UIA tree * input offset fix for sub rootviews * Split modal into two componentview's so that we dont have multiple roots in our componentview tree * format * Ensure rootview removes itself from the island, before a new one adds itself on instance reload * defork some modal test pages * remove override * fix * fix * snapshots --------- Co-authored-by: Chiara Mooney <34109996+chiaramooney@users.noreply.github.com>
Description
Type of Change
Why
Accessibility on the new architecture should reach parity with Paper.
Resolves #13064
Resolves #13060
Resolves #13056
Resolves #13052
Resolves #13048
Resolves #13044
Resolves #13040
Resolves #13036
Resolves #13032
Resolves #12047
#13327
#13316
Resolves #13354
Resolves #13353
What
Testing
Locally tested for:
Changelog
Should this change be included in the release notes: Yes