Focus trap lost when you click an item and remove it

[cdeutsch]

Bug report

Current Behavior

If you click an item and remove it, the focus gets set to the browser's Url bar.

When you tab back into the body content, you can tab to elements that are behind the focus trap.

Expected behavior

Focus shouldn't go to the Url bar after removing an item.

If it does, you should return to the focus trap instead of elements outside it.

Reproducible example

Code:
https://codesandbox.io/s/hardcore-stitch-ne0g0n?file=/App.js

Easier to reproduce fullscreen:
https://ne0g0n.csb.app/

Additional context

Previously I've been using focus-trap-react and it didn't have the same issue. Focus doesn't go to the Url bar when items are removed, and if you manually focus the Url bar, you will return to the tab trap straight away.

Your environment

Software	Name(s)	Version
Radix Package(s)	@radix-ui/react-dialog	0.1.8-rc.48
Radix Package(s)	@radix-ui/react-select	0.1.2-rc.50
React	n/a	latest
Browser	Chrome	102.0.5005.115
Operating System	Mac	12.4

[benoitgrelard]

This actually looks like a bug in Chrome, as Firefox and Safari don't raise a focusout event when the element that has focus gets deleted.

Not sure how they handle it, because document.activeElement shows that it's on body but a press on "tab" will continue tabbing normally to the next control (after the deleted one):

CleanShot.2022-07-07.at.17.12.34.mp4

Because no events are raised on those, we wouldn't be able to strictly control the focused element anyway.
We'll see if we can patch the Chrome bug somehow.

[cdeutsch]

Not sure how they handle it, because document.activeElement shows that it's on body but a press on "tab" will continue tabbing normally to the next control (after the deleted one):

Is that true in a Dialog as well?

[benoitgrelard]

This is an example in Dialog.

[benoitgrelard]

We handle everything when the dialog is unmounted so I think ultimately this is a product responsibility.
If in your specific app, you unmount something that has focus inside that Dialog, then you should redirect the focus to the next logical place in your dialog.

[cdeutsch]

We handle everything when the dialog is unmounted so I think ultimately this is a product responsibility.

FWIW focus-trap-react doesn't have the same issue, and doesn't expect you to do anything special.

I think the real issue with Radix is not the dialog losing focus, but that it allows focus to resume "behind" the dialog. That just shouldn't be possible if it was actually trapped.

[jjenzz]

@benoitgrelard could the focus trap do something like the following to solve this?

document.addEventListener('focusout', (event) => {
  if (event.relatedTarget === null) container.focus();
})

[benoitgrelard]

@jjenzz from my previous comment here: #1498 (comment)

This actually looks like a bug in Chrome, as Firefox and Safari don't raise a focusout event when the element that has focus gets deleted.

So looks like in theory it might work because even though focusout is only raised by Chrome in these cases (when deleting the focused element) the next tab keeps it inside magically in the other browsers (I have no idea how they do that).

However, I remember trying exactly what you did here from within FocusScope but had issues with distinguishing that case from other cases where relatedTarget is also null.

I'll give it another spin soon.

[benoitgrelard]

Reopening, will see if we can do what's above.

[benoitgrelard]

Ok I finally found a solution for this: #2145

[enkot]

Hey @benoitgrelard

Solution with MutationObserver creates another problem:
radix-vue/radix-vue#518

This is Vue example :) but it has exactly the same logic and 99% RadixUI also has this problem.

Similar issue in RadixUI - #2436

[hipstersmoothie]

I'm experiencing this exact issue

[hipstersmoothie]

I've tested the fix for the linked radix-vue` PR and it seems to work

function handleMutations(mutations) {
  const isLastFocusedElementExist = container1.contains(lastFocusedElementRef.current)

  if (!isLastFocusedElementExist) {
    $d3863c46a17e8a28$var$focus(container1)
  }
}

[hipstersmoothie]

@benoitgrelard should I make a PR?