Teams: No longer inappropriately focus reaction menus when conversation messages are focused.#14363
Merged
Conversation
seanbudd
previously approved these changes
Nov 11, 2022
See test results for failed build of commit 381781adef |
LeonarddeR
previously approved these changes
Nov 11, 2022
LeonarddeR
left a comment
Collaborator
There was a problem hiding this comment.
Given its positive impact, could we have this on beta instead?
The base branch was changed.
Member
Author
|
@LeonarddeR agreed. I actually just forgot to set the base branch in the pr correctly. Thanks for catching. Fixed now. The commits themselves were correctly already on top of beta. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to issue number:
Fixes #14355
Broadens approach from pr #11822 which was to fix #11821.
Summary of the issue:
When a conversation message is focused in Microsoft Teams (E.g. arrowing up and down a threaded conversation list) Teams displays a reaction menu along side the focused message. As Chromium therefore fires a menu popupStart event, NVDA handles this by faking focus on the menu itself
This behaviour has been historically necessary for win32 context menus that fail to focus the first item in the menu.
However, In Teams this is very disruptive.
Description of user facing changes
NVDA will no longer get stuck in a menu when arrowing up and down threaded conversations in Microsoft Teams.
Description of development approach
Broaden the approach taken in pr #11821 by suppressing handling of menu popupStart for any element in teams with 'menu' in its xml-roles attribute. Previously NVDA would also check for very particular message container classes on the parent's parent. But as Teams keeps moving these, this is no longer managable.
Testing strategy:
Performed steps in #14355. I.e. arrowed up and down a threaded conversation, ensuring each message was read and was not interuppted by a menu.
Ensured that pressing enter on the message still allowed the user to interact with the reaction menu when its items truely got focus.
Smoke tested other context menus in Teams such as the menu on the tab control and the search field.
Known issues with pull request:
This more broad approach may mean there could mean that NVDA won't announce "menu" for a menu that appears that does not focus one of its items until the user presses an arrow key. However, having checked all menus that I know, and waying up the slight confusion of NvDA saying nothing on a menu until pressing a key verses the real focus such as for a message being interuppted all the time by an incorrect menu, I feel this approach is by far the best choice for the user.
Change log entries:
New features
Changes
Bug fixes
For Developers
Code Review Checklist: