fix(react-menu): remove the tabster restorer source from MenuPopover#36314
Closed
layershifter wants to merge 1 commit into
Closed
fix(react-menu): remove the tabster restorer source from MenuPopover#36314layershifter wants to merge 1 commit into
layershifter wants to merge 1 commit into
Conversation
Follow-up to microsoft#32840, which moved Menu off the tabster Restorer (removed the restore target from MenuTrigger, restores focus manually) but kept the restorer source on MenuPopover for triggerless menus. This removes that remaining source, completing the move away from the restorer (the source of the "unintended focus switching" microsoft#32840 set out to avoid). Triggered menus are unaffected (manual restore); triggerless menus allow focus to fall to <body> on close, per microsoft#32840. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
Author
📊 Bundle size report
Unchanged fixtures
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -0,0 +1,7 @@ | |||
| { | |||
There was a problem hiding this comment.
🕵🏾♀️ visual changes to review in the Visual Change Report
vr-tests-react-components/Positioning 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Positioning.Positioning end.chromium.png | 53 | Changed |
| vr-tests-react-components/Positioning.Positioning end.updated 2 times.chromium.png | 23 | Changed |
vr-tests-react-components/ProgressBar converged 3 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/ProgressBar converged.Indeterminate + thickness - Dark Mode.default.chromium.png | 25 | Changed |
| vr-tests-react-components/ProgressBar converged.Indeterminate + thickness.default.chromium.png | 80 | Changed |
| vr-tests-react-components/ProgressBar converged.Indeterminate + thickness - High Contrast.default.chromium.png | 138 | Changed |
vr-tests-react-components/TagPicker 3 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/TagPicker.disabled - High Contrast.disabled input hover.chromium.png | 1319 | Changed |
| vr-tests-react-components/TagPicker.disabled - RTL.chromium.png | 635 | Changed |
| vr-tests-react-components/TagPicker.disabled.disabled input hover.chromium.png | 677 | Changed |
There were 3 duplicate changes discarded. Check the build logs for more information.
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
1 participant
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.
Follow-up to #32840 and #36313.
Background
#32840 moved
Menuaway from the tabsterRestorerAPI for focus restoration — it removeduseRestoreFocusTargetfromMenuTriggerand restores focus to the trigger manually instead, because "the restorer can cause unintended focus switching whenever other restore sources lose focus to body."That PR intentionally kept the restorer source on
MenuPopover— "to make sure that users can still useMenuwithout a trigger easily."Change
This removes the remaining
useRestoreFocusSource()fromMenuPopover, completing the move away from the tabster restorer forMenu:Menufocus restoration to tabster's restorer — the mechanism behind the "unintended focus switching" fix(Menu): Do not rely on tabster restorer for focus restore #32840 set out to avoid.ignoreKeydown).Tradeoff
Per #32840's note, the restorer source was kept for triggerless
Menuusage. With this change a triggerless menu no longer restores focus via tabster on close (focus is allowed to fall to<body>), consistent with #32840's stated preference: "allow lost focus to body in applications rather than unexpected focus switches."Related
MenuTrigger, kept the source onMenuPopoverignoreKeydownEscape onMenuPopover