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.

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 IValueProvider #12263

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Conversation

YajurG
Copy link
Contributor

@YajurG YajurG commented Oct 18, 2023

Type of Change

  • New feature (non-breaking change which adds functionality)

Why

Implements the IValueProvider and supporting methods which is used to provide access to controls that have an intrinsic value that does not span a range, and that can be represented as a string.

Based on the implementation of ValueProvider in Paper in #5080, the get_IsReadOnly() method does not need to be implemented due to no available corresponding React Native prop.

What

Implemented SetValue and get_Value methods as part of the IValueProvider interface.

Screenshots

Before:
image

After:
image

Testing

Tested locally on playground-composition.

Changelog

Should this change be included in the release notes: yes

Implement UIA IValueProvider

Microsoft Reviewers: Open in CodeFlow
Microsoft Reviewers: Open in CodeFlow

@@ -327,4 +333,62 @@ HRESULT __stdcall CompositionDynamicAutomationProvider::Invoke() {
return S_OK;
}

} // namespace winrt::Microsoft::ReactNative::implementation
std::string convertLPCWSTRToString(LPCWSTR val) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You can probably use wint::to_string in place of this.

if (baseView == nullptr)
return UIA_E_ELEMENTNOTAVAILABLE;
winrt::IInspectable uiaProvider = baseView->EnsureUiaProvider();
UpdateUiaProperty(uiaProvider, UIA_ValueValuePropertyId, *props->accessibilityValue.text, value);
Copy link
Contributor

Choose a reason for hiding this comment

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

setValue should go to the ComponentView, which would decide if it needs to update its state based on the new value.
Then it should be up to the ComponentView to call UpdateUiaProperty(uiaProvider, UIA_ValueValuePropertyId, *props->accessibilityValue.text, value); when the value changes. Which it would also need to do when the value changes outside of a call to the AutomationPovider::SetValue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you clarify what you mean by going to ComponentView? I tried adding an UpdateUiaProperty() call in updateAccessibilityProps() which was getting called every time the accessibilityValue prop was updated, but there was no change in the UIA properties on insights.

Also. not sure if this is expected behaviour, but SetValue() never seemed to be getting called when updating accessibilityValue (breakpoint in the function was never hit). My guess was that the accessibilityValue example in accesible.tsx doesn't call SetValue() to update the property.

Copy link
Contributor

Choose a reason for hiding this comment

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

The call to SetValue on the AutomationProvider would only occur if the accessibility tool wants to manipulate the value. This would happen say through voice control or if you select the value pattern actions button in insights and set the value from there.

That value set should then go to the component view to actually modify its value.

Separately, whenever the component view updates its value it needs to notify the automation peer that its value has been changed. That should happen through a call to UpdateUiaProperty, which would notify the automation provider of the new value, and then the automation provider should call UiaRaiseAutomationEvent which will notify say insights that the value has changed, at which point it can decide if it needs the updates value, and it would call back into the automation peer's getvalue.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Author Feedback The issue/PR needs activity from its author (label drives bot activity) label Oct 18, 2023
@YajurG YajurG changed the title Implement UIA Value Provider Implement IValueProvider Oct 18, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs: Author Feedback The issue/PR needs activity from its author (label drives bot activity) label Oct 18, 2023
@@ -1305,6 +1305,17 @@ void CompositionBaseComponentView::EnsureTransformMatrixFacade() noexcept {
}
}

void CompositionBaseComponentView::updateAccessibilityValue(std::string newValue) noexcept {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@acoates-ms The value being set in SetValue() goes here and then is modified through a call to UpdateUiaProperty() - which from my understanding also calls the UiaRaiseAutomationPropertyChangedEvent(), updating insights that the property has changed. Do I still need to add a UiaRaiseAutomationEvent() here as well?

@@ -1387,6 +1398,12 @@ void CompositionViewComponentView::updateProps(
m_visual.Opacity(newViewProps.opacity);
}

if (oldViewProps.accessibilityValue.text.has_value() &&
Copy link
Contributor Author

@YajurG YajurG Oct 19, 2023

Choose a reason for hiding this comment

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

Separately, whenever the component view updates its value it needs to notify the automation peer that its value has been changed.

Does this change address this issue - whenever updateProps() is called, if accessibilityValue.text has changed, it will be modified and UpdateUiaProperty() will be called.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants