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

Fix menu item without sub menu leaking focus on arrow keys #8592

Merged

Conversation

v-elnovikova
Copy link
Contributor

@v-elnovikova v-elnovikova commented Feb 8, 2023

Fixes #8591

Proposed changes

  • Handle Up and Down keys in ToolStripDropDownItem class in case when item has no sub-items.

Customer Impact

  • Screen readers do not announce other controls when a user tries to open menu item without sub menu.

Regression?

  • No

Risk

  • Minimal

Screenshots

Before

When empty menu item is focused, Down key switches focus to form button:

Before.mp4

After

When empty menu item is focused, Down key does nothing, menu is still focused:

After.mp4

Test methodology

  • Manual

Test environment(s)

  • .NET 8.0.0-alpha.1.23057.5
Microsoft Reviewers: Open in CodeFlow

@v-elnovikova v-elnovikova requested a review from a team as a code owner February 8, 2023 14:53
@ghost ghost assigned v-elnovikova Feb 8, 2023
@v-elnovikova v-elnovikova added the waiting-review This item is waiting on review by one or more members of team label Feb 8, 2023
@@ -542,12 +542,12 @@ protected internal override bool ProcessDialogKey(Keys keyData)
{
Keys keyCode = (Keys)keyData & Keys.KeyCode;

// Items on the overflow should have the same kind of keyboard handling as a toplevel
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: please end comments with a dot

@v-elnovikova v-elnovikova force-pushed the Issue_8591_Fix_ProcessDialogKey_method branch from 1ca0df4 to 7dca352 Compare February 9, 2023 10:14
Copy link
Contributor

@dmitrii-drobotov dmitrii-drobotov 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!

@Tanya-Solyanik
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@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 Feb 9, 2023
@dreddy-work dreddy-work enabled auto-merge (squash) February 9, 2023 18:29
@dreddy-work dreddy-work merged commit a8db5c3 into dotnet:main Feb 9, 2023
@ghost ghost added this to the 8.0 Preview2 milestone Feb 9, 2023
@ghost ghost removed the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Feb 9, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 12, 2023
@v-elnovikova v-elnovikova deleted the Issue_8591_Fix_ProcessDialogKey_method branch May 25, 2023 08:30
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.

Menu item without sub menu leaking focus on arrow keys
4 participants