Uh oh!
There was an error while loading. Please reload this page.
Fix infinite loop in ContextualMenu - #2297
Conversation
Harrie Shin (harrieshin)
commented
Nov 2, 2022
Isnt it still valid in some cases the tab should infact take u outside of focuszone? |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Harrie Shin (harrieshin)
left a comment
There was a problem hiding this comment.
I think tab should work outside focuszone
chiuam
commented
Nov 2, 2022
Yes, I made sure tabbing through FocusZones still works as expected for all test cases in the test page. (tabbing into/out of disabled FZ is buggy but not a regression from this change) |
chiuam
commented
Nov 2, 2022
It does work? |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Saad Najmi (Saadnajmi)
left a comment
There was a problem hiding this comment.
Talked offline, approval with 2 comments:
Let's document with comments
while([nextViewToFocus isDescendantOf:focusZoneAncestor])means "there are no views left in the key view loopLet's not introduce a new
noOpboolean, and just leave the check as "nextViewToFocus === nil` with the same comment / logic
Uh oh!
There was an error while loading. Please reload this page.
Harrie Shin (harrieshin)
commented
Nov 4, 2022
before and after video please |
Harrie Shin (harrieshin)
commented
Nov 4, 2022
update the rest of the pr description as well please? |
Uh oh!
There was an error while loading. Please reload this page.
chiuam
commented
Nov 4, 2022
I'll be updating the PR description with before and after videos. Anything else? |
might be worth testing arrow key as well after tab? |
chiuam
commented
Nov 7, 2022
Tab shouldn't move focus, so that's expected? |
chiuam
commented
Nov 7, 2022
Untitled.movUp/down works after tab :) |
This reverts commit 6f086d9.
Platforms Impacted
Description of changes
This PR fixes a hang on tabbing inside a contextual menu. The hang happens in this while loop:
while([nextViewToFocus isDescendantOf:focusZoneAncestor])in FocusZone where it tries to get to the next key view that's outside of the current FocusZone view. In the context of ContextualMenu, the while loop tries to search for the next valid key view (outside the menu view) but it becomes infinite due to the fact thatisDescendantOf: selfreturns true. The expected outcome for tabbing inside a contextual menu should be no op. Let's catch this edge case for the tab and shift-tab scenarios.Verification
Before:
before.mov
After:
after.mov
Pull request checklist
This PR has considered (when applicable):