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

RadioButtons: Fix selected radio button not being focused on tab navigation when first element in TAB navigation #3048

Conversation

Felix-Dev
Copy link
Contributor

@Felix-Dev Felix-Dev commented Aug 5, 2020

Description

This PR fixes issue #3021 where focus was not set to the selected radio button when re-entering the specific RadioButtons control via TAB navigation. This issue occurs when the RadioButtons control is the first control in the TAB navigation route. In that case, GettingFocusEventArgs.OldFocusedElement() returns null, even if the focus was indeed set to another item before (in this case the last item in the TAB navigation route). With this PR, we now also set focus on the selected radio button (if any) when there is no old focused element as per GettingFocusEventArgs.

radiobuttons-focus

In order to properly test this case I had to create a new test page without the Back button and ToggleTheme button typically available on a test page as those buttons come first in the TAB navigation route so GettingFocusEventArgs.OldFocusedElement() would not be null upon tabbing into the RadioButtons control. I did not want to remove those two buttons for the existing RadioButtons test page so my only option was to create a new test page here.

I have modified an existing interaction test - FocusComingFromAnotherRepeaterTest() - to cover this specific case. The modified test is not an exact 1-to-1 re-build as I removed checks like Verify.IsTrue(item3.IsSelected); for easier testing. This should be fine as those checks are covered in the existing SelectionTest().

Motivation and Context

Fixes #3021.

How Has This Been Tested?

Tested visually and added interaction test.

Screenshots:

image
(The new landing page for RadioButtons tests)

image
(The new special focus test page)

@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Aug 5, 2020
Copy link
Contributor

@StephenLPeters StephenLPeters left a comment

Choose a reason for hiding this comment

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

:shipit:

@StephenLPeters
Copy link
Contributor

looks good, Although its seems weird to me that GettingFocusEventArgs.OldFocusedElement() returns null when focus wraps. @Austin-Lamb do you know why this is the case?

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters StephenLPeters added area-RadioButtons team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Aug 5, 2020
@StephenLPeters
Copy link
Contributor

@Felix-Dev do you happen to know if this also reproduces when RadioButtons is the last focusable elmement in the tree and you use Shift+Tab to move focus from the first element to the last element (the radiobuttons).

@Felix-Dev
Copy link
Contributor Author

Felix-Dev commented Aug 6, 2020

Yep, same issue with the other way around (GettingFocusEventArgs.OldFocusedElement() = null):

radiobuttons-focus-reverse

(And yes, the workaround works here as well, as expected.)

If this is indeed a bug in GettingFocusEventArgs, it makes sense to add such a comment above the workaround referencing your tracking issue #3066.

@Felix-Dev
Copy link
Contributor Author

@StephenLPeters See the gif above (I pinged you previously but that was in error, I posted the wrong gif. This ping is just meant to infomr you that the correct gif is now up above.)

@StephenLPeters
Copy link
Contributor

StephenLPeters commented Aug 7, 2020

That new issue has a reference to this check in so I don't think a comment is necessary.

@StephenLPeters StephenLPeters merged commit 3ff3437 into microsoft:master Aug 7, 2020
@Felix-Dev Felix-Dev deleted the user/Felix-Dev/radiobuttons-focus-fix branch August 7, 2020 00:11
Kinnara added a commit to Kinnara/ModernWpf that referenced this pull request Aug 16, 2020
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.

Tab navigation is broken when RadioButtons is first Control in visual tree
3 participants