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

Jump to bottom

[Fabric] Implement IExpandCollapseProvider #13892

Merged
merged 6 commits into from
Oct 9, 2024
Merged

[Fabric] Implement IExpandCollapseProvider #13892

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
71fbaa5
Implement IExpandCollapseProvider
chiaramooney Sep 24, 2024
007447d
Change files
chiaramooney Sep 24, 2024
3dbc61a
Adjust Example
chiaramooney Sep 25, 2024
2e7a128
Format + Update Snapshots
chiaramooney Sep 26, 2024
27b6fdb
Merge branch 'main' of https://github.com/microsoft/react-native-wind…
chiaramooney Sep 26, 2024
2f22ae9
Merge branch 'main' of https://github.com/microsoft/react-native-wind…
chiaramooney Oct 3, 2024
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-9696c96d-9a44-4ffc-84ea-dd5451818206.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Implement IExpandCollapseProvider",
"packageName": "react-native-windows",
"email": "34109996+chiaramooney@users.noreply.github.com",
"dependentChangeType": "patch"
}
23 changes: 12 additions & 11 deletions packages/@react-native-windows/tester/src/js/examples/View/ViewExample.windows.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,7 @@ class AccessibilityExample extends React.Component<
> {
state: {tap: number} = {
tap: 0,
expanded: true,
};

render(): React.Node {
Expand All @@ -554,10 +555,10 @@ class AccessibilityExample extends React.Component<
accessibilityRole="button"
accessibilityValue={{now: this.state.tap}}
accessibilityActions={[
{name: 'cut', label: 'cut'},
{name: 'copy', label: 'copy'},
{name: 'paste', label: 'paste'},
{name: 'expand', label: 'expand'},
{name: 'collapse', label: 'collapse'},
]}
accessibilityState={{expanded: this.state.expanded}}
accessibilityPosInSet={1}
accessibilitySetSize={1}
accessibilityLiveRegion='polite'
Expand All @@ -566,19 +567,19 @@ class AccessibilityExample extends React.Component<
focusable
onAccessibilityAction={event => {
switch (event.nativeEvent.actionName) {
case 'cut':
Alert.alert('Alert', 'cut action success');
break;
case 'copy':
Alert.alert('Alert', 'copy action success');
break;
case 'paste':
Alert.alert('Alert', 'paste action success');
case 'expand':
this.setState({expanded: true})
break;
case 'collapse':
this.setState({expanded: false})
}
}}
onAccessibilityTap={() => {
this.setState({tap: this.state.tap + 1});
}}
onPress={()=>{
this.setState({expanded: !this.state.expanded});
console.log('Pressed');
}}>
<Text>A View with accessibility values.</Text>
<Text>Current Number of Accessibility Taps: {this.state.tap}</Text>
Expand Down
1 change: 1 addition & 0 deletions packages/e2e-test-app-fabric/test/__snapshots__/ViewComponentTest.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1094,6 +1094,7 @@ exports[`View Tests Views can have customized accessibility 1`] = `
"Automation Tree": {
"AutomationId": "accessibility",
"ControlType": 50000,
"ExpandCollapsePattern.ExpandCollapseState": "Expanded",
"HelpText": "Accessibility Hint",
"IsKeyboardFocusable": true,
"LiveSetting": "Polite",
Expand Down
18 changes: 10 additions & 8 deletions packages/e2e-test-app-fabric/test/__snapshots__/snapshotPages.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77745,16 +77745,12 @@ exports[`snapshotAllPages View 22`] = `
accessibilityActions={
[
{
"label": "cut",
"name": "cut",
"label": "expand",
"name": "expand",
},
{
"label": "copy",
"name": "copy",
},
{
"label": "paste",
"name": "paste",
"label": "collapse",
"name": "collapse",
},
]
}
Expand All @@ -77764,6 +77760,11 @@ exports[`snapshotAllPages View 22`] = `
accessibilityPosInSet={1}
accessibilityRole="button"
accessibilitySetSize={1}
accessibilityState={
{
"expanded": true,
}
}
accessibilityValue={
{
"now": 0,
Expand All @@ -77773,6 +77774,7 @@ exports[`snapshotAllPages View 22`] = `
focusable={true}
onAccessibilityAction={[Function]}
onAccessibilityTap={[Function]}
onPress={[Function]}
testID="accessibility"
>
<Text>
Expand Down
30 changes: 30 additions & 0 deletions packages/e2e-test-app-fabric/windows/RNTesterApp-Fabric/RNTesterApp-Fabric.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,23 @@ void InsertToggleStateValueIfNotDefault(
}
}

void InsertExpandCollapseStateValueIfNotDefault(
const winrt::Windows::Data::Json::JsonObject &obj,
winrt::hstring name,
ExpandCollapseState value,
ExpandCollapseState defaultValue = ExpandCollapseState::ExpandCollapseState_Collapsed) {
if (value != defaultValue) {
switch (value) {
case 0:
obj.Insert(name, winrt::Windows::Data::Json::JsonValue::CreateStringValue(L"Collapsed"));
break;
case 1:
obj.Insert(name, winrt::Windows::Data::Json::JsonValue::CreateStringValue(L"Expanded"));
break;
}
}
}

winrt::Windows::Data::Json::JsonObject ListErrors(winrt::Windows::Data::Json::JsonValue payload) {
winrt::Windows::Data::Json::JsonObject result;
winrt::Windows::Data::Json::JsonArray jsonErrors;
Expand All @@ -339,6 +356,7 @@ void DumpUIAPatternInfo(IUIAutomationElement *pTarget, const winrt::Windows::Dat
BOOL isReadOnly;
ToggleState toggleState;
IValueProvider *valuePattern;
ExpandCollapseState expandCollapseState;
HRESULT hr;

// Dump IValueProvider Information
Expand All @@ -365,6 +383,18 @@ void DumpUIAPatternInfo(IUIAutomationElement *pTarget, const winrt::Windows::Dat
}
togglePattern->Release();
}

// Dump IExpandCollapseProvider Information
IExpandCollapseProvider *expandCollapsePattern;
hr = pTarget->GetCurrentPattern(UIA_ExpandCollapsePatternId, reinterpret_cast<IUnknown **>(&expandCollapsePattern));
if (SUCCEEDED(hr) && expandCollapsePattern) {
hr = expandCollapsePattern->get_ExpandCollapseState(&expandCollapseState);
if (SUCCEEDED(hr)) {
InsertExpandCollapseStateValueIfNotDefault(
result, L"ExpandCollapsePattern.ExpandCollapseState", expandCollapseState);
}
expandCollapsePattern->Release();
}
}

winrt::Windows::Data::Json::JsonObject DumpUIATreeRecurse(
Expand Down
1 change: 1 addition & 0 deletions packages/e2e-test-app/test/__snapshots__/ViewComponentTest.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ exports[`ViewTests Views can have a custom nativeID 1`] = `
exports[`ViewTests Views can have accessibility customization 1`] = `
{
"AccessibilityRole": "Button",
"AccessibilityStateExpanded": true,
"AutomationId": "accessibility",
"AutomationPositionInSet": 1,
"AutomationSizeOfSet": 1,
Expand Down
59 changes: 59 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,18 @@ bool accessibilityValueHasValue(const facebook::react::AccessibilityValue &value
return (value.min.has_value() && value.max.has_value()) || value.now.has_value() || value.text.has_value();
}

bool expandableControl(const facebook::react::SharedViewProps props) {
if (props->accessibilityState.has_value() && props->accessibilityState->expanded.has_value())
return true;
auto accessibilityActions = props->accessibilityActions;
for (size_t i = 0; i < accessibilityActions.size(); i++) {
if (accessibilityActions[i].name == "expand" || accessibilityActions[i].name == "collapse") {
return true;
}
}
return false;
}

HRESULT __stdcall CompositionDynamicAutomationProvider::GetPatternProvider(PATTERNID patternId, IUnknown **pRetVal) {
if (pRetVal == nullptr)
return E_POINTER;
Expand Down Expand Up @@ -181,6 +193,15 @@ HRESULT __stdcall CompositionDynamicAutomationProvider::GetPatternProvider(PATTE
AddRef();
}

if (patternId == UIA_ExpandCollapsePatternId &&
(accessibilityRole == "combobox" || accessibilityRole == "splitbutton" || accessibilityRole == "treeitem" ||
(expandableControl(props) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

I know we do this pattern elsewhere, but do we really need to gate on accessibilityRole if expandable is set? Is there an actual limitation in UIA where a JS component author can't define say, a switch with expandable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

UIA would not stop us. We would be able to specify a Switch control type that implements the ExpandCollapse provider, but that would have the wrong accessibility conventions. i.e. a control like that would not meet "correct" accessibility rules. The role checks here were mostly added as "guard rails" for JS developers who aren't as familiar with UIA accessibility expectations. They help prevent JS developers from building apps with wonky accessibility implementations.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a case where we are potentially blocking something, which while unusual, might be desired.

What if I have a switch which expands/collapses a bunch of UI based on its state. Maybe I want it to report both expand state and the rest of the switch state.

I always worry about these kinds of restrictive policies. What if I'm designing some completely new kind of control type, where expandable very much makes sense, but I dont want UIA reporting that I'm a combobox (or any of these other options). If I set accessibilityState.expanded - I'd expect that to report to the accessibility tools. It would be completely non-obvious that actually for that to work I need to set the role to one of several special values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we've followed this pattern for other providers so far I don't want to merge this PR with a different behavior pattern. I'm happy to rediscuss this concern, but let's do it in a separate issue and if we decide to remove the guard rails we can do so for all providers in one PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Issue filed #13934

(accessibilityRole == "toolbar" || accessibilityRole == "menuitem" || accessibilityRole == "menubar" ||
accessibilityRole == "listitem" || accessibilityRole == "group" || accessibilityRole == "button")))) {
*pRetVal = static_cast<IExpandCollapseProvider *>(this);
AddRef();
}

return S_OK;
}

Expand Down Expand Up @@ -496,4 +517,42 @@ HRESULT __stdcall CompositionDynamicAutomationProvider::Toggle() {
return S_OK;
}

HRESULT __stdcall CompositionDynamicAutomationProvider::get_ExpandCollapseState(ExpandCollapseState *pRetVal) {
if (pRetVal == nullptr)
return E_POINTER;
auto strongView = m_view.view();

if (!strongView)
return UIA_E_ELEMENTNOTAVAILABLE;

auto props = std::static_pointer_cast<const facebook::react::ViewProps>(
winrt::get_self<winrt::Microsoft::ReactNative::implementation::ComponentView>(strongView)->props());

if (props == nullptr)
return UIA_E_ELEMENTNOTAVAILABLE;

*pRetVal = props->accessibilityState->expanded.has_value()
? GetExpandCollapseState(props->accessibilityState->expanded.value())
: ExpandCollapseState_Collapsed;
return S_OK;
}

HRESULT __stdcall CompositionDynamicAutomationProvider::Expand() {
auto strongView = m_view.view();

if (!strongView)
return UIA_E_ELEMENTNOTAVAILABLE;
DispatchAccessibilityAction(m_view, "expand");
return S_OK;
}

HRESULT __stdcall CompositionDynamicAutomationProvider::Collapse() {
auto strongView = m_view.view();

if (!strongView)
return UIA_E_ELEMENTNOTAVAILABLE;
DispatchAccessibilityAction(m_view, "collapse");
return S_OK;
}

} // namespace winrt::Microsoft::ReactNative::implementation
10 changes: 8 additions & 2 deletions vnext/Microsoft.ReactNative/Fabric/Composition/CompositionDynamicAutomationProvider.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ class CompositionDynamicAutomationProvider : public winrt::implements<
IInvokeProvider,
IScrollItemProvider,
IValueProvider,
IToggleProvider> {
IToggleProvider,
IExpandCollapseProvider> {
public:
CompositionDynamicAutomationProvider(
const winrt::Microsoft::ReactNative::Composition::ComponentView &componentView) noexcept;
Expand Down Expand Up @@ -47,10 +48,15 @@ class CompositionDynamicAutomationProvider : public winrt::implements<
virtual HRESULT __stdcall get_Value(BSTR *pRetVal) override;
virtual HRESULT __stdcall get_IsReadOnly(BOOL *pRetVal) override;

// inherited via IToggleProivder
// inherited via IToggleProvider
virtual HRESULT __stdcall get_ToggleState(ToggleState *pRetVal) override;
virtual HRESULT __stdcall Toggle() override;

// inherited via IExpandCollapseProvider
virtual HRESULT __stdcall get_ExpandCollapseState(ExpandCollapseState *pRetVal) override;
virtual HRESULT __stdcall Expand() override;
virtual HRESULT __stdcall Collapse() override;

private:
::Microsoft::ReactNative::ReactTaggedView m_view;
};
Expand Down
8 changes: 8 additions & 0 deletions vnext/Microsoft.ReactNative/Fabric/Composition/UiaHelpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,4 +206,12 @@ void DispatchAccessibilityAction(::Microsoft::ReactNative::ReactTaggedView &view
}
}

ExpandCollapseState GetExpandCollapseState(const bool &expanded) noexcept {
if (expanded) {
return ExpandCollapseState_Expanded;
} else {
return ExpandCollapseState_Collapsed;
}
}

} // namespace winrt::Microsoft::ReactNative::implementation
1 change: 1 addition & 0 deletions vnext/Microsoft.ReactNative/Fabric/Composition/UiaHelpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,5 @@ std::string extractAccessibilityValue(const facebook::react::AccessibilityValue

void DispatchAccessibilityAction(::Microsoft::ReactNative::ReactTaggedView &view, const std::string &action) noexcept;

ExpandCollapseState GetExpandCollapseState(const bool &expanded) noexcept;
} // namespace winrt::Microsoft::ReactNative::implementation
Loading