Removing focused element inside dialog or dropdown makes CPU spike in chrome.

[s-yadav]

Bug report

Current Behavior

Removing focused element inside dialog or dropdown makes CPU spike. Possibly due to focus trapping logic.

Bug

Expected behavior

CPU spike shouldn't happen

Reproducible example

https://codesandbox.io/s/confident-pasteur-30gc2q

Suggested solution

Manually handling focus trap as suggested here works https://github.com/radix-ui/primitives/issues/1498#issuecomment-1194346289

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

Additional context

Related issue: https://github.com/radix-ui/primitives/issues/1498#issuecomment-1194346289

This works fine on other OS.

Your environment

Software	Name(s)	Version
Radix Package(s)	Dialog	1.0.0
React	n/a	17
Browser	Chrome (MacOS)	103.0.5060.53
Assistive tech
Node	n/a
npm/yarn
Operating System	MacOS	all

[andy-hook]

Thanks @s-yadav , I haven't had a chance to look at this in depth but given that focus should be manually redirected upon removal I wonder if this is a practical problem at the moment?

[kuldeepkeshwar]

@andy-hook there are couple of examples we faced in our product.

• A table with virtualisation enabled having any focusable element in row, scrolling will remove the focused element
• change(content1->content2) in content of Popover based on click of actionable element in popover content.

[dsdeur]

I've also encountered this in multiple situations. Mainly for (context) menu's that have a "delete" option which deletes the parent of the menu.

I was looking into it a while back but unfortunately ran out of time. In my case I think the FocusScope might be calling .focus() on an element in a ref that is unmounted, causing the CPU spike. I don't know why it happens but can reproduce the behavior: https://codesandbox.io/s/focus-cpu-syme2v?file=/src/App.tsx

[benoitgrelard]

@dsdeur, I don't see any CPU spike on that sandbox. How am I supposed to use it/see it? A specific browser?

[kuldeepkeshwar]

Did you try chrome ?

[benoitgrelard]

Yeah I see it now. Given that sandbox from @dsdeur, it seems to be a bug in Chrome itself to be honest. We'll see if we can get around it.

[dsdeur]

Indeed, I have only seen it in Chrome, I've tested FF and Safari. It is quite problematic and I have not been able to get the workaround mentioned to work for my case.

I think what happens is a timing issue where the focusout is called before the listener is unregistered. Tracking unmount might work, an example that seems to work: https://codesandbox.io/s/focus-cpu-forked-pwizyl?file=/src/App.tsx Idk if that is also possible in FocusScope.

Alternatively what already would help a lot is to have a way to just disable this behavior. On unmount (of the parent) it's likely necessary to manually handle focus anyway.

[benoitgrelard]

Indeed, I have only seen it in Chrome, I've tested FF and Safari. It is quite problematic and I have not been able to get the workaround mentioned to work for my case.

I think what happens is a timing issue where the focusout is called before the listener is unregistered. Tracking unmount in FocusScope might work, an example that seems to work: https://codesandbox.io/s/focus-cpu-forked-pwizyl?file=/src/App.tsx Idk if that is also possible in FocusScope.

That would be if the parent component unmounts, not when the specific item gets removed. This will be impossible in FocusScope, we can't track mount state for every child inside it.

Alternatively what already would help a lot is to have a way to just disable this behavior. On unmount (of the parent) it's likely necessary to manually handle focus anyway.

I'm not following what you're saying here.

[dsdeur]

Alternatively what already would help a lot is to have a way to just disable this behavior. On unmount (of the parent) it's likely necessary to manually handle focus anyway.

I'm not following what you're saying here.

Sorry, not very clear. I was referring to the use case where I have seen it most; when you use a (context) menu to remove an item (example).

If there is a way to disable the automatic focus handling in Radix that triggers the issue it would be relatively easy to work around it and manually handle focus in my app for these cases.

[benoitgrelard]

If there is a way to disable the automatic focus handling in Radix that triggers the issue it would be relatively easy to work around it and manually handle focus in my app for these cases.

You can do this already on the Content part:

onCloseAutoFocus={event => {
  // focus what you want here, for example the previous item
  previousItemRef.current?.focus();
  event.preventDefault();
}}

[dsdeur]

Nice, thanks! Moving the focus elsewhere in the handler works as a workaround. I had tried the preventDefault, which wasn't enough.

(If anybody is interested, here is the updated example sandbox with this workaround)

[evertjr]

Any definitive fix for this issue? I created a multistep modal and when the first component unmount the CPU reaches 100%.

Edit: Managed to make it work by applying the modal styles to an wrapped div and remounting Dialog.Content when the content changes.

[benoitgrelard]

@evertjr I have just submitted a PR for it here: #2145

[lakbychance]

@benoitgrelard Thanks for the fix 😄 . We were facing this issue for dialog component. Can confirm that dialog's 1.0.4-rc.4 works. When is version 1.0.4 to be likely released for dialog ?

[benoitgrelard]

Not sure for the public release, I am trying to get some more maintenance done at the moment, so will be a little bit before that, but hopefully not too long. You can use the rc in the meantime.