Skip to content

Conversation

@SergeySmirnov-Akvelon
Copy link
Contributor

@SergeySmirnov-Akvelon SergeySmirnov-Akvelon commented Aug 2, 2021

Fixes #3058

Related issue: #3421

Proposed changes

  • The issue is reproduced because the old logic sometimes returned incorrect results for "NextSibling" and "PreviousSibling", which led to incorrect shortcuts behavior. To fix the issue, UIA support was added in which we return the correct results
  • Updated "SupportsUiaProviders" flag.
  • Added and implemented accessible objects for the TabControl and TabPage
  • Added unit tests.
  • Fixed issue with Narrator

Customer Impact

Before fix:
Shortcuts in some cases did not work for TabControl

After fix:
3058-expected

Regression?

  • No

Risk

  • Minimal

Test methodology

  • CTI team
  • Unit tests

Accessibility testing

  • Narrator
  • Accessibility Insights
  • Inspect

Test environment(s)

  • Microsoft Windows [Version 10.0.19041.388]
  • .NET Core SDK: 6.0.100-preview.3.21202.5
Microsoft Reviewers: Open in CodeFlow

@SergeySmirnov-Akvelon SergeySmirnov-Akvelon requested a review from a team as a code owner August 2, 2021 10:21
@dreddy-work dreddy-work added the waiting-author-feedback The team requires more information from the author label Aug 2, 2021
@vladimir-krestov vladimir-krestov self-requested a review August 3, 2021 05:59
@ghost ghost removed the waiting-author-feedback The team requires more information from the author label Aug 3, 2021
@vladimir-krestov
Copy link
Contributor

Fix that:
image

@dreddy-work dreddy-work added the waiting-author-feedback The team requires more information from the author label Aug 3, 2021
@ghost ghost removed the waiting-author-feedback The team requires more information from the author label Aug 4, 2021
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-3058_Add_UIA_support_TabControl branch from 148cd65 to 37c9842 Compare August 5, 2021 14:05
@dreddy-work
Copy link
Member

@Tanya-Solyanik , can you take a final look at this?

@RussKie RussKie changed the title Fixing UIA navigation through Accessibility Insights shortcuts Fix UIA navigation for TabControl Aug 16, 2021
RussKie
RussKie previously approved these changes Aug 16, 2021
Copy link
Contributor

@RussKie RussKie left a comment

Choose a reason for hiding this comment

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

👍 with few comments

Copy link
Contributor

Choose a reason for hiding this comment

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

redundant

@RussKie RussKie added the waiting-author-feedback The team requires more information from the author label Aug 16, 2021
@ghost ghost removed the waiting-author-feedback The team requires more information from the author label Aug 16, 2021
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-3058_Add_UIA_support_TabControl branch from 37c9842 to 57e4152 Compare August 16, 2021 09:48
RussKie
RussKie previously approved these changes Aug 16, 2021
@RussKie RussKie added the waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) label Aug 16, 2021
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-3058_Add_UIA_support_TabControl branch from 57e4152 to e4960e3 Compare August 16, 2021 10:03
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.

@dreddy-work - what is the appropriate label for new public surface.

Looks good, please see my comments though.

@RussKie
Copy link
Contributor

RussKie commented Aug 18, 2021

what is the appropriate label for new public surface.

We're adding overrides here, which is acceptable; and thus we don't treat these as "new API", which require an API Review Board approval, documentations, etc.

@Tanya-Solyanik
Copy link
Contributor

@RussKie - Overrides require new documentation topics, are they generated automatically?

Copy link
Contributor

@Tanya-Solyanik Tanya-Solyanik Aug 18, 2021

Choose a reason for hiding this comment

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

Please add a test to see if clients are listening.

@RussKie
Copy link
Contributor

RussKie commented Aug 18, 2021

Overrides require new documentation topics, are they generated automatically?

I believe the override docs are generated automatically. @gewarren @carlossanlop is this correct?

@gewarren
Copy link
Contributor

Overrides require new documentation topics, are they generated automatically?

I believe the override docs are generated automatically. @gewarren @carlossanlop is this correct?

Yes the override methods will appear on the main page for the type, like this: https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.metrics.observablecounter-1?view=net-6.0#methods

Copy link
Member

@dreddy-work dreddy-work left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Contributor

Choose a reason for hiding this comment

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

What happens when we hit the spinner control? Suppose we have more tab headers than fits into the control width, then we show a left/right arrows to navigate between the tab headers. Do we hit-test those arrows correctly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have investigated this question and did not find direct access to the Spinner from the TabControl. We are now using the native Spinner implementation. I have not seen any difference in behavior since .NET Framework 4.7.2, except that now the Spinner is displayed first in the list of control elements, and not second as in .NET Framework 4.7.2. How critical do you think this is?

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's keep the hierarchy as is and follow up with Ryan. We can address this separately.

Copy link
Contributor

Choose a reason for hiding this comment

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

The difference between UIA clients listening and AO create APIs is that the former will force creation of AO if AT (or touch driver) is listening but the object had not been created yet. This might be desired if tab is the first object accessed by narrator. Note that touch driver needs AO created to work as well as AT does.

…lity Insights For Windows) keyboard combination Shift+Ctrl+F5/F6/F7/F8/F9 doesn't work with the TabControl dotnet#3058

Updated "SupportsUiaProviders" flag.

Added and implemented accessible objects for the TabControl and TabPage

Added unit tests.

Fixed issue with Narrator
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-3058_Add_UIA_support_TabControl branch from 8288bbd to 52288b6 Compare August 23, 2021 13:12
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 to me, please get a review from Vladimir and follow up with Ryan if we have to create a custom provider for the spinner control.

@dreddy-work dreddy-work merged commit 962e1d3 into dotnet:main Aug 23, 2021
@ghost ghost added this to the 7.0 alpha1 milestone Aug 23, 2021
@dreddy-work
Copy link
Member

@vladimir-krestov , I merged this. If you have further feedback, please feel free to update this and @SergeySmirnov-Akvelon will update them in the next PR along with the pending investigation.

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

Labels

waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UIA navigation doesn't work with the TabControl

6 participants