Skip to content

Implementing support UIA Provider for TreeView control#6432

Merged
Tanya-Solyanik merged 2 commits intodotnet:mainfrom
lisoenot:TreeView-must-support-UIA
Mar 8, 2022
Merged

Implementing support UIA Provider for TreeView control#6432
Tanya-Solyanik merged 2 commits intodotnet:mainfrom
lisoenot:TreeView-must-support-UIA

Conversation

@lisoenot
Copy link
Contributor

@lisoenot lisoenot commented Dec 27, 2021

Implemented accessible objects for TreeView and TreeNode. Added needed automation events and unit tests.

Implements part of #3421

Proposed changes

  • Switched on SupportsUiaProviders property of TreeView class.
  • Implemented accessible objects for TreeView and TreeNode classes.
  • Added needed automation events and unit tests.

Customer Impact

  • Improving development experience for TreeView control accessibility.

Regression?

  • No

Risk

  • Minimal

Screenshots

Inspect

TreeView before and after

treeInspect

Default TreeNode before and after

nodeInspect

Expanded TreeNode before and after

nodeExpandedInspect

Leaf TreeNode before and after

nodeLeafInspect

CheckBox TreeNode before and after

NodeIsCheckBox

Invisible TreeNode before and after

invisibleNode

Editable TreeNode before and after

nodeEditable

Accessibility Insights

There is no specific errors. You can see errors on next image, but they are not about accessibility changes were created in this PR.

AI

Narrator

Expand and collapse default node before and after

NBExpandCollapse

Check and uncheck node before and after

NBCheckedBox

Edit default node before and after

NBEdit

ScrollIntoView method comparison

ScrollIntoView method before

ScrollIntoViewBefore

ScrollIntoView method after

ScrollIntoViewAfter

This realization sets calling node as TopNode of TreeView if the node is invisible. Looks like it works the same.

Focus on TreeView issue

By default, when we focus on a TreeView control, Inspect and Narrator highlight the SelectedNode of
this control. As I know it takes raise an events for a selected node accessible object to notify it was focused. But calling AccessibilityObject forces creation the object. It takes accessible object of the tree, so forces creating the object if it doesn't exist yet.
To avoid force creation if this is not needed should use check IsAccessibilityObjectCreated. But I found next problem when I try to add this check. Highlight after focus doesn't work for the first time if the control has first TabIndex on form. You can see this issue behavior below.

Focus on the control with check IsAccessibilityObjectCreated

It works incorrect.

BrokenFocusOnTreeView

Focus on the control without check IsAccessibilityObjectCreated

It works great, but not safety due to forcing the object creating.

FocusOnTreeView

It was not so easy for me to resolve it in the fix. So I skipped this problem and took implementation without the check. I guess this problem needs to be investigated and resolved in out of scope issue.

Additional note

I take it if node is check box it has DefaultAction equals Check or Uncheck but before the fix DoDefaultAction does nothing in this case. I configured this method, so it performs Toggle method of TreeNodeAccessibleObject in this case.

Test methodology

  • Manually
  • Unit testing
  • CTI

Accessibility testing

  • Inspect
  • Accessibility Insights
  • Narrator

Test environment(s)

  • .Net version: 6.0.100
  • OS version: 10.0.19044
Microsoft Reviewers: Open in CodeFlow

@lisoenot lisoenot requested a review from a team as a code owner December 27, 2021 23:12
@ghost ghost assigned lisoenot Dec 27, 2021
@lisoenot lisoenot added the waiting-review This item is waiting on review by one or more members of team label Dec 27, 2021
@lisoenot lisoenot force-pushed the TreeView-must-support-UIA branch from 17e4d73 to 0a57be8 Compare December 28, 2021 00:45
Copy link
Contributor

@vladimir-krestov vladimir-krestov left a comment

Choose a reason for hiding this comment

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

Great work!
Awesome description!

Copy link
Contributor

@WPMGPRoSToTeMa WPMGPRoSToTeMa left a comment

Choose a reason for hiding this comment

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

Thank you, you've done an amazing work!

@Tanya-Solyanik Tanya-Solyanik added waiting-author-feedback The team requires more information from the author and removed waiting-review This item is waiting on review by one or more members of team labels Jan 4, 2022
@lisoenot lisoenot force-pushed the TreeView-must-support-UIA branch from 0a57be8 to 7b5902b Compare January 9, 2022 11:17
@ghost ghost removed the waiting-author-feedback The team requires more information from the author label Jan 9, 2022
@lisoenot lisoenot force-pushed the TreeView-must-support-UIA branch 3 times, most recently from b51e9c5 to 6d9dfff Compare January 9, 2022 12:21
@lisoenot lisoenot force-pushed the TreeView-must-support-UIA branch from 6d9dfff to 944d0f3 Compare January 10, 2022 09:33
@Tanya-Solyanik Tanya-Solyanik added waiting-author-feedback The team requires more information from the author waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) and removed waiting-author-feedback The team requires more information from the author labels Jan 10, 2022
Tanya-Solyanik
Tanya-Solyanik previously approved these changes Jan 11, 2022
Copy link
Contributor

@Tanya-Solyanik Tanya-Solyanik left a comment

Choose a reason for hiding this comment

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

Looks good, please send to testing!

@lisoenot
Copy link
Contributor Author

I've sent it for testing.

@lisoenot
Copy link
Contributor Author

One regression and one not regression have been found. So still WIP status, I am working on this.

@lisoenot lisoenot added draft draft PR and removed waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) labels Jan 12, 2022
@ghost ghost removed the draft draft PR label Jan 12, 2022
RussKie
RussKie previously approved these changes Jan 28, 2022
ghost pushed a commit that referenced this pull request Feb 1, 2022
<!-- Please read CONTRIBUTING.md before submitting a pull request -->

Resolve comment from pr #6432 


## Proposed changes

- Remove call empty ctor of Accessible Object class in inherited classes.

<!-- We are in TELL-MODE the following section must be completed -->

## Customer Impact

- No

## Regression? 

- No

## Risk

- No
Copy link
Contributor

@Tanya-Solyanik Tanya-Solyanik left a comment

Choose a reason for hiding this comment

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

Please see my questions, otherwise looks good.

@Tanya-Solyanik Tanya-Solyanik added waiting-author-feedback The team requires more information from the author and removed waiting-review This item is waiting on review by one or more members of team labels Feb 8, 2022
Copy link
Contributor

@vladimir-krestov vladimir-krestov left a comment

Choose a reason for hiding this comment

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

Looks good!

{
get
{
AccessibleStates state = AccessibleStates.Selectable | AccessibleStates.Focusable;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please check, if a node really selectable and focusable if its TreeView control is disabled. If no, please add a condition

Copy link
Contributor Author

@lisoenot lisoenot Feb 21, 2022

Choose a reason for hiding this comment

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

Tree Node is really selectable and focusable and it is "unavailable" in this case. So I add this state.

2

{
get
{
AccessibleStates state = AccessibleStates.Focusable;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add Enabled check for it

Copy link
Contributor Author

@lisoenot lisoenot Feb 21, 2022

Choose a reason for hiding this comment

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

Tree View is "unavailable" too, if it is disabled. So I add this state.

1


#region Selection Pattern

internal override bool IsSelectionRequired => true;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure. If a TreeView can have noone selected item, it's may not be true. Please check it.

Copy link
Contributor Author

@lisoenot lisoenot Feb 21, 2022

Choose a reason for hiding this comment

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

Yes, this is wrong. It must be false, if it has no items. Thanks

3

@ghost ghost removed the waiting-author-feedback The team requires more information from the author label Feb 21, 2022
@lisoenot lisoenot dismissed stale reviews from RussKie and WPMGPRoSToTeMa via 194bf71 February 22, 2022 12:24
@lisoenot lisoenot force-pushed the TreeView-must-support-UIA branch from c1a0760 to 194bf71 Compare February 22, 2022 12:24
@lisoenot lisoenot added the waiting-review This item is waiting on review by one or more members of team label Feb 22, 2022
Tanya-Solyanik
Tanya-Solyanik previously approved these changes Feb 22, 2022
Copy link
Contributor

@Tanya-Solyanik Tanya-Solyanik left a comment

Choose a reason for hiding this comment

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

Looks good.
@DmitryGorokhov - please investigate if providers are disconnected properly on containing window close. @ArtemTatarinov might be able to help with this investigation.
@vladimir-krestov - looks like all you comments were addressed, please take another look.

Dmitry Gorokhov added 2 commits February 24, 2022 08:44
Implemented accessible objects for TreeView and TreeNode.
Added needed automation events.
Added unit tests.
Copy link
Contributor

@Tanya-Solyanik Tanya-Solyanik left a comment

Choose a reason for hiding this comment

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

Thank you!

@Tanya-Solyanik Tanya-Solyanik added ready-to-merge PRs that are ready to merge but worth notifying the internal team. and removed waiting-review This item is waiting on review by one or more members of team labels Mar 2, 2022
@Tanya-Solyanik Tanya-Solyanik merged commit 185eda9 into dotnet:main Mar 8, 2022
@ghost ghost added this to the 7.0 Preview3 milestone Mar 8, 2022
@RussKie RussKie removed the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Mar 9, 2022
@lisoenot lisoenot deleted the TreeView-must-support-UIA branch March 9, 2022 11:08
@Cassie-Li01
Copy link

Verified on build dotnet-sdk-7.0.100-preview.3.22179.4-win-x64, screenshot as below.
TreeView:
image
Default TreeNode:
image
Expanded TreeNode:
image
Leaf TreeNode:
image
CheckBox TreeNode:
image

@ghost ghost locked as resolved and limited conversation to collaborators May 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants