Implementing accessibilityStates ('disabled' and 'selected') view props, #2573 #2638
Conversation
ghost
added
the
|
|
||
| void DynamicAutomationPeer::AddToSelection() const | ||
| { | ||
|
|
There was a problem hiding this comment.
should it throw exception that not implemented
This reverts commit a136e4c.
| } | ||
|
|
||
| shouldBeControl = (pViewShadowNode->AccessibilityRole() != AccessibilityRoles::None); | ||
| } |
There was a problem hiding this comment.
I think this should not happen if propertyValue.isNull() is true, right?
There was a problem hiding this comment.
I think shouldBeControl needs to be true if we modify accessiblity properties, because otherwise they're no-opts without the DynamicAutomationPeer.
There was a problem hiding this comment.
But if they all go back to a state of not needing the peer why would we keep it? I don't know that the dynamic case like this would be common, but it seems odd to keep it as a Control when that's not needed. We've tried to update these conditions on properties so the result is accurate when a property is no longer bound.
In reply to: 295914113 [](ancestors = 295914113)
There was a problem hiding this comment.
But isn't this method run only to update properties that have changed? If the first batch sets a property that requires a control, a later batch trying to change a different property can't decide to get rid of that control. We need a way of maintaining when we need the automationpeer.
| } | ||
| } | ||
|
|
||
| shouldBeControl = true; |
There was a problem hiding this comment.
shouldBeControl = true; [](start = 8, length = 23)
As above, I don't think this should happen if propertyValue.isNull() is true.
| disabled = viewControl->AccessibilityState(::react::uwp::AccessibilityStates::Disabled); | ||
| } | ||
| } | ||
| catch (winrt::hresult_error const & ex) {} |
There was a problem hiding this comment.
Why exception here? If it's because viewControl memory is freed, this code would be very dangerous, and I prefer not to ship this feature until the problem is resolved.
Even if it's not crash in debug version, how about release version?
4 participants
disabledoverrides IsEnabled https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.automation.automationelementidentifiers.isenabledpropertyselectedoverrides IsSelected https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.automation.selectionitempatternidentifiers.isselectedpropertyMicrosoft Reviewers: Open in CodeFlow