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

Restorer steals focus from non-modal modals when trigger is inside of arrow navigation group #374

Closed
micahgodbolt opened this issue Mar 20, 2024 · 2 comments · Fixed by #389

Comments

@micahgodbolt
Copy link
Member

micahgodbolt commented Mar 20, 2024

When a non-modal modal trigger is inside of an arrow navigation group, upon opening the modal, focus doesn't properly move to the modal.

  1. Put focus on the first button.
  2. Use arrows to put focus on the non modal
  3. Hit enter, focus moves to the close button
  4. hit tab to move to the next focus target
  5. BUG: focus moves back to the non modal trigger rather than focus targets in the modal

regular modals don't exhibit the behavior, and moving the modal trigger outside of arrow navigation also negates the bug.

https://stackblitz.com/edit/5bb1gz-r6ndrd?file=src%2Fexample.tsx

@bsunderhus bsunderhus assigned bsunderhus and unassigned bsunderhus Mar 21, 2024
@bsunderhus bsunderhus assigned bsunderhus and unassigned bsunderhus May 13, 2024
@bsunderhus
Copy link
Collaborator

This seems to be a combination of @fluentui/react-tabster + tabster problem:

In @fluentui/react-tabster

Seems like no modal attribute is added once trapFocus is false (which is the case for non-modal dialogs)

https://github.com/microsoft/fluentui/blob/1ccf6c23ddc4d125940ce23ea3788d0069665f67/packages/react-components/react-tabster/src/hooks/useModalAttributes.ts#L55-L62

In tabster

It seems like even if the v9 code is modified to always include the modal attributes indicating no trap, some other issues appear

@mshoho
Copy link
Member

mshoho commented May 13, 2024

In this case, because the non-modal dialog trigger is inside arrow navigation area, it semantically belongs (via virtual parentship) to the arrow navigation area. And arrow navigation area (aka Mover) behaves almost correctly (minus the fact that arrow keys don't move focus to the inner part of the dialog). We'll discuss on a team sync what should the proper behaviour for these virtual parent cases be.

mshoho added a commit that referenced this issue May 17, 2024
… Mover. (#389)

Tabster supports custom getParent() function which allows virtual parentship. Here we make Mover to handle only real children. This fixes #374.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants