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.

Sign up for GitHub

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

Jump to bottom

Opening nested dialog when the parent dialog doesn't have focus closes parent dialog #3522

Open
Dremora opened this issue Feb 28, 2024 · 4 comments
Open

Opening nested dialog when the parent dialog doesn't have focus closes parent dialog #3522

Dremora opened this issue Feb 28, 2024 · 4 comments
Labels
has workaround potential bug

Comments

@Dremora
Copy link
Sponsor

Dremora commented Feb 28, 2024 •
edited by diegohaz

Current behavior

This may seem like a contrived example, but it has arisen from a real-world application. In order for the bug to happen, the following criteria need to be fulfilled:

  • have two nested dialogs (nested in the React tree)
  • outer dialog needs to have autoFocusOnShow set to false
  • outer dialog needs to have an element (e.g. button) that opens inner dialog. This element should have onMouseDown={(e) => e.preventDefault()}. The reason for this specific code is that in our application this interactive element comes from @hello-pangea/dnd, and according to their docs, a draggable handle that also serves as a clickable element will have this behaviour (to prevent the element from becoming focused)

Steps to reproduce the bug

  1. Open https://stackblitz.com/edit/cniyvy?file=dialog-nested%2Findex.tsx&theme=dark
  2. Click "View Cart"
  3. Click on any of the "Remove" buttons
  4. Watch the dialog close

Expected behavior

The outer dialog stays open, and the nested dialog appears.

Workaround

In our case, removing autoFocusOnShow={false} is probably the best solution. However, it's not a proper workaround as it changes the behaviour.

Edit by @diegohaz: Alternatively, return false on the parent Dialog's hideOnInteractOutside function prop when document.querySelector("#inner-dialog").contains(event.target).

Possible solutions

According to my understanding, this happens when the outer dialog doesn't have focus at the moment the inner dialog opens. The focus is outside the dialog (e.g. on the <body> element), and so the focus travels from outside the dialogs directly to the nested dialog. There's likely code in Ariakit that incorrectly identifies such focus event as moving focus outside of the outer dialog. Placing hideOnInteractOutside with an event handler on the outer dialog demonstrates that it's this focus event (namely, focusin) that closes the outer dialog.
@diegohaz
Copy link
Member

diegohaz commented Feb 28, 2024 •
edited

Yes, the dialog element must be focused at least once to trigger the check on nested dialogs:

ariakit/packages/ariakit-react-core/src/dialog/utils/use-hide-on-interact-outside.ts

Lines 90 to 94 in 6d29ca1

// We need to check if the content element has been focused at least once
// before checking if it's marked. This is so hovercards and tooltips
// don't stay open when new nodes are added to the DOM and focused.
const focused = focusedRef.current;
if (focused && !isElementMarked(target, contentElement.id)) return;

Note

For anyone else working on this in the future (myself included): A potential solution might be focused || modal.

Could you provide more details on why you need autoFocusOnShow={false} on the outer dialog?

@Dremora
Copy link
Sponsor Author

Dremora commented Feb 28, 2024 •
edited

Could you provide more details on why you need autoFocusOnShow={false} on the outer dialog?

The main reason: depending on what opened the dialog, the first focusable element may have :focus-visible pseudoclass applied, which results in a focus ring. We only prefer to display focus ring on buttons when keyboard interaction was involved.

Update 1: I've never given it enough thought before, but it seems that the focus will only be visible if at the moment of opening a dialog, document.activeElement is body.

Update 2: there's another reason (although not applicable to this particular dialog). We have an element with autofocus attribute within a dialog, but it doesn't work with unmountOnHide. Disabling autoFocusOnShow makes this element autofocus without any additional code.

@diegohaz
Copy link
Member

Update 2: there's another reason (although not applicable to this particular dialog). We have an element with autofocus attribute within a dialog, but it doesn't work with unmountOnHide. Disabling autoFocusOnShow makes this element autofocus without any additional code.

You may want to use Ariakit's autoFocus rather than the React autofocus prop. The former should work with any Ariakit dialog:

- <div autofocus />
+ <Focusable autoFocus render={<div />} />

@Dremora
Copy link
Sponsor Author

Dremora commented Feb 29, 2024 •
edited

Update 2: there's another reason (although not applicable to this particular dialog). We have an element with autofocus attribute within a dialog, but it doesn't work with unmountOnHide. Disabling autoFocusOnShow makes this element autofocus without any additional code.

You may want to use Ariakit's autoFocus rather than the React autofocus prop. The former should work with any Ariakit dialog:

- <div autofocus />
+ <Focusable autoFocus render={<div />} />

This works!

The issue related to body being in focus at the point of dialog being opened and resulting in a :focus-visible still persists. In our case, the element that triggers the dialog is also partially controlled by a drag-and-drop library, so likely a similar thing (focusin event prevention) occurs.

Update: it's actually not related to body being in focus in this case, but still potentially related to a D&D library blocking events.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
has workaround potential bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Dremora @diegohaz