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

Add automation name to RadioButtons #5094

Merged
merged 3 commits into from
Jun 8, 2021
Merged

Conversation

teaP
Copy link
Contributor

@teaP teaP commented May 26, 2021

Fixes #3183. RadioButtons had an AccessibilityView of Raw, so it didn't show up in the tree and its items were not grouped together. This is unlike other list-like controls, which appear like this:

list "Header Text"
    text "Header Text"
    list item "item 1"
    list item "item 2"
    etc.

So, RadioButtons now appears in the accessible tree as a group item when items inside it. If the control isn't given an accessible name, the header text is used, so it will appear like this:

group "Header Text"
    text "Header Text"
    radio button "item 1"
    radio button "item 2"
    etc.

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label May 26, 2021
Comment on lines 68 to 74
const auto name = winrt::AutomationProperties::GetName(*this);
if (name.empty())
{
// If the automation name isn't set, use header text as the name.
winrt::AutomationProperties::SetName(*this, SharedHelpers::TryGetStringRepresentationFromObject(Header()));
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should also do this on property changed for our header so it's staying up to date. Also could/should do it when the AutiomationProperties.Name property changes.

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 realized it wasn't really the right way to do it -- the header text should be returned from the automation peer. So this is all fixed up now. :)

@ranjeshj ranjeshj added area-RadioButtons team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels May 26, 2021
@teaP
Copy link
Contributor Author

teaP commented May 26, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Collaborator

@marcelwgn marcelwgn left a comment

Choose a reason for hiding this comment

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

Changes look good to me. Could you add a simple test for this? Small example of this would be this test (although it is not testing for the name property):

public void VerifyAutomationPeerBehavior()

@teaP
Copy link
Contributor Author

teaP commented May 27, 2021

Could you add a simple test for this?

Sure, I added a part in the existing UIA test to verify that a group item exists with the header name.

@teaP
Copy link
Contributor Author

teaP commented May 27, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Collaborator

@marcelwgn marcelwgn 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! :shipit:

@teaP teaP merged commit 7168b0b into main Jun 8, 2021
@teaP teaP deleted the user/teaP/RadioButtonsAutomationName branch June 8, 2021 17:13
@crutkas
Copy link
Member

crutkas commented Jun 8, 2021

Will this be in a 2.x release or only 3?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-RadioButtons team-Controls Issue for the Controls team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Screen reader does not read the heading of RadioButtons control
4 participants