Skip to content

Conversation

lukeblevins
Copy link
Contributor

Fixes #2020
Fixes #2036

@lukeblevins lukeblevins requested a review from yaira2 October 5, 2020 17:13
@yaira2 yaira2 changed the title Accessibility: Fix Two Problems with Keyboard Navigation Accessibility: Fixed Two Problems with Keyboard Navigation Oct 5, 2020
yaira2
yaira2 previously approved these changes Oct 5, 2020
@yaira2 yaira2 changed the title Accessibility: Fixed Two Problems with Keyboard Navigation Fixed Two Problems with Keyboard Navigation Oct 5, 2020
@yaira2 yaira2 changed the title Fixed Two Problems with Keyboard Navigation Fixed two issues with keyboard navigation Oct 5, 2020
Copy link
Contributor

@Thomas1664 Thomas1664 left a comment

Choose a reason for hiding this comment

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

In line 77 in the file SidebarControl.xaml IsTabStop is also set to False. Can you fix that as well?

@yaira2 yaira2 added the changes requested Changes are needed for this pull request label Oct 5, 2020
@yaira2
Copy link
Member

yaira2 commented Oct 6, 2020

In line 77 in the file SidebarControl.xaml IsTabStop is also set to False. Can you fix that as well?

@duke7553 Can you look at this?

@lukeblevins
Copy link
Contributor Author

@Thomas1664 This property value doesn't affect functionality, does it?

@jaigak
Copy link
Contributor

jaigak commented Oct 7, 2020

@duke7553 It does. Disabling tab stop will prevent keyboard tab navigation for the UI element

@lukeblevins
Copy link
Contributor Author

@Jaiganeshkumaran @Thomas1664 Sorry, but what you said is incorrect for this case.

The changes I already made allow for using tab because tab stop is enabled for the items themselves already. Unless you want to have a discussion about removing the custom template for these sidebar items altogether (which I'd support), there is no need to change the template value because it already works fine with my changes.

Would enabling the template value for the tab stop give us any more benefits? In my testing, the sidebar items already work as intended.

@Thomas1664
Copy link
Contributor

@duke7553 Although your changes already work, it would be better for code style if you removed the remaining IsTabStop. Furthermore, it may save us from future bugs.

@lukeblevins
Copy link
Contributor Author

@Thomas1664 Since the template itself was redundant, I just got rid of it so we use the standard resources now.

@lukeblevins lukeblevins requested a review from yaira2 October 7, 2020 16:38
@lukeblevins lukeblevins added ready to merge Pull requests that are approved and ready to merge and removed changes requested Changes are needed for this pull request labels Oct 7, 2020
@Thomas1664
Copy link
Contributor

@Thomas1664 Since the template itself was redundant, I just got rid of it so we use the standard resources now.

Sonds great

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to use TAB key to select all sidebar items Unable to invoke certain focused buttons with Enter
4 participants