Skip to content
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.

Sign up for GitHub

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 UIA Invoke Method #11874

Merged
merged 23 commits into from
Aug 9, 2023
Merged

Implement UIA Invoke Method #11874

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
2f77b71
Implement Invoke
chiaramooney Jul 6, 2023
e0779e4
Change files
chiaramooney Jul 11, 2023
ddad53f
Format
chiaramooney Jul 11, 2023
da2dbbd
Adjust Call
chiaramooney Jul 11, 2023
50a5e90
Adjust Call
chiaramooney Jul 11, 2023
e39a92d
Address Feedback
chiaramooney Jul 13, 2023
352b505
Format
chiaramooney Jul 13, 2023
2c1bcc9
Fix
chiaramooney Jul 14, 2023
10b6a31
Merge
chiaramooney Jul 20, 2023
c80f7bc
Address Feedback
chiaramooney Jul 20, 2023
223257c
Merge
chiaramooney Jul 20, 2023
50d480a
Fix
chiaramooney Jul 20, 2023
c2d7e04
Remove
chiaramooney Jul 20, 2023
2680a00
Fix
chiaramooney Jul 24, 2023
57fe0ba
Format
chiaramooney Jul 24, 2023
b41a392
Fix UIA Tree Issue
chiaramooney Aug 1, 2023
f44b187
Address Feedback
chiaramooney Aug 3, 2023
82749ed
Implement Invoke
chiaramooney Aug 3, 2023
b7fd10f
Change files
chiaramooney Aug 3, 2023
83e4197
Merge branch 'main' of https://github.com/microsoft/react-native-wind…
chiaramooney Aug 3, 2023
1497fec
Remove Unneeded
chiaramooney Aug 3, 2023
beaab47
Format
chiaramooney Aug 3, 2023
4d84bd8
Merge branch 'main' into cm-fabric-invoke
chiaramooney Aug 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions change/react-native-windows-fcb20088-d74a-479f-8b41-d95d797de8a4.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Implement Invoke",
"packageName": "react-native-windows",
"email": "[email protected]",
"dependentChangeType": "patch"
}
38 changes: 38 additions & 0 deletions vnext/Microsoft.ReactNative/Fabric/Composition/CompositionDynamicAutomationProvider.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,24 @@ HRESULT __stdcall CompositionDynamicAutomationProvider::GetPatternProvider(PATTE

*pRetVal = nullptr;

auto strongView = m_view.view();
if (strongView == nullptr)
return UIA_E_ELEMENTNOTAVAILABLE;

auto props = std::static_pointer_cast<const facebook::react::ViewProps>(strongView->props());
if (props == nullptr)
return UIA_E_ELEMENTNOTAVAILABLE;
auto accessibilityRole = props->accessibilityRole;
// Invoke control pattern is used to support controls that do not maintain state
// when activated but rather initiate or perform a single, unambiguous action.
if (patternId == UIA_InvokePatternId &&
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's possible we could/should decouple this from accessibilityRole. Regardless of role, if we chould just check if onAccessibilityTap exists on the element, and if it does, return we're IInvokable, otherwise, not invokable. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Couple of thoughts:

  • In the UIA documentation the rules around UIA providers are very specific to each control. i.e. just because a control can be pressed doesn't mean Invokable is the right provider. The control might instead need to implement the Toggle or Selection provider. I think if we were to make this change it would put a lot of ownership on the developer to setup their controls correctly. For example, all button controls should support the Invoke provider according to UIA. But with this change there could be buttons that don't implement onAccessibilityTap that then would not support the Invoke pattern.
  • The onAccessibilityTap prop can be specified by any control. So, with this change, developers would also now be able to make controls Invokable that shouldn't have that UIA pattern. i.e. You could specify onAccessibilityTap on a Switch control, and it would have the Invoke pattern when it should only support the Toggle pattern.

UIA Reference: https://learn.microsoft.com/en-us/windows/win32/winauto/uiauto-supportbuttoncontroltype

Copy link
Contributor

Choose a reason for hiding this comment

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

I think if we were to make this change it would put a lot of ownership on the developer to setup their controls correctly

I think for JS-based controls written from the ground up, there needs to be ownership on the dev's part. We should document how these things interact and provide guidance, ideally, but we should also provide our own control offerings via FURN that solve these problems for them so they don't have to think about them as much.

My 2c:

  1. If we have something where Invoke MUST be present (e.g. Link), we should just look at the Role and expose the pattern. It's then a bug on the JS dev if they fail to provide onAccessibilityTap/onAccessibilityAction(activate).
  2. If it's something like Button or MenuItem where it can be Invoke/Toggle/ExpandCollapse in mutual exclusivity, we need to look at role & the provided accessibiliyAction, since the docs support extending the actions for custom types we'll probably want "toggle", "expand", "collapse", to make the decision what pattern to expose. In this case it'd be a bug on JS dev to supply multiple patterns (both "activate" and "toggle")

Copy link
Contributor

Choose a reason for hiding this comment

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

Some of those sound like things we could possibly make the JS start creating yellowboxes for.

(accessibilityRole == "button" || accessibilityRole == "imagebutton" || accessibilityRole == "link" ||
accessibilityRole == "splitbutton" || (accessibilityRole == "menuitem" && props->onAccessibilityTap) ||
(accessibilityRole == "treeitem" && props->onAccessibilityTap))) {
*pRetVal = static_cast<IInvokeProvider *>(this);
AddRef();
}

return S_OK;
}

Expand Down Expand Up @@ -278,4 +296,24 @@ HRESULT __stdcall CompositionDynamicAutomationProvider::get_HostRawElementProvid
return S_OK;
}

HRESULT __stdcall CompositionDynamicAutomationProvider::Invoke() {
FalseLobster marked this conversation as resolved.
Show resolved Hide resolved
auto strongView = m_view.view();

if (!strongView)
return UIA_E_ELEMENTNOTAVAILABLE;

auto baseView = std::static_pointer_cast<::Microsoft::ReactNative::CompositionBaseComponentView>(strongView);
if (baseView == nullptr)
return UIA_E_ELEMENTNOTAVAILABLE;

baseView.get()->GetEventEmitter().get()->onAccessibilityTap();
auto uiaProvider = baseView->EnsureUiaProvider();
auto spProviderSimple = uiaProvider.try_as<IRawElementProviderSimple>();
if (spProviderSimple != nullptr) {
UiaRaiseAutomationEvent(spProviderSimple.get(), UIA_Invoke_InvokedEventId);
}

return S_OK;
}

} // namespace winrt::Microsoft::ReactNative::implementation
7 changes: 6 additions & 1 deletion vnext/Microsoft.ReactNative/Fabric/Composition/CompositionDynamicAutomationProvider.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@
#include <Fabric/ReactTaggedView.h>
#include <UIAutomation.h>
#include <inspectable.h>
#include <uiautomationcore.h>

namespace winrt::Microsoft::ReactNative::implementation {

class CompositionDynamicAutomationProvider : public winrt::implements<
CompositionDynamicAutomationProvider,
IInspectable,
IRawElementProviderFragment,
IRawElementProviderSimple> {
IRawElementProviderSimple,
IInvokeProvider> {
public:
CompositionDynamicAutomationProvider(
const std::shared_ptr<::Microsoft::ReactNative::CompositionBaseComponentView> &componentView) noexcept;
Expand All @@ -31,6 +33,9 @@ class CompositionDynamicAutomationProvider : public winrt::implements<
virtual HRESULT __stdcall get_HostRawElementProvider(IRawElementProviderSimple **pRetVal) override;
// virtual HRESULT __stdcall ShowContextMenu() noexcept override;

// inherited via IInvokeProvider
virtual HRESULT __stdcall Invoke() override;

private:
::Microsoft::ReactNative::ReactTaggedView m_view;
};
Expand Down