Material-ui: [TrapFocus] Restore focus issue

[ljones-i-nexus]

• [x] The issue is present in the latest release.
• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior

Modal component when pressing escape, focus is not being correctly restored to clicked element. Clicked element is an IconButton wrapped in a Tooltip.

Focus does not trigger expected highlighting of button and tooltip popup. Moving to a different tab and moving back to the page restores focus as expected.

Expected Behavior

Focus should be restored with a highlighted button and open Tooltip.

Steps to Reproduce

https://codesandbox.io/s/modal-focus-issue-ge4pr?fontsize=14&hidenavigation=1&theme=dark

1. Open either modal
2. Press escape

Alternate:
1.Open either modal

1. Press escape
2. Change tab and change back

Your Environment

Using Typescript and Hooks but clearly exists with vanillajs

[oliviertassinari]

Confirmed, I remember looking at this issue in the past. I couldn't understand exactly what was going on. From what I recall, the focus event wasn't firing (🙈), leading to a wrong focus visible style. Introducing a setTimeout in the .focus() call was solving the problem (🙈). In any case, the DOM focus is handled correctly, the UI feedback is not.

[ljones-i-nexus]

Thanks for looking into the issue quickly. Currently switched to using the Dialog component which restores focus correctly.

[eps1lon]

We need to entangle what you mean by "focus not being returned". There are two levels of focus that are relevant here: visible focus and (standard) focus.

The standard focus is behaving as expected (otherwise please include the operating system you are using i.e. do you use a mac or not).

What I see is not behaving as expected is visible focus. Even when only operating the codesandbox with a keyboard the the visible focus is not returned to the button (it is focused though). If I switch tabs it does get visibly focused though.

It seems like an issue with the tooltip integration. Should be possible to write an integration test using Button, Modal and Tooltip and debug what's happening.

[ljones-i-nexus]

I agree this is an issue with "visible focus" rather than focus being returned to the element. However it doesn't seem to be a tooltip issue.

I have duplicated a button on the sandbox linked above and removed the Tooltip wrapper and the issue still exists.

[fyodore82]

@ljones-i-nexus , please look at screenshot below and confirm, that I understand correctly

[ljones-i-nexus]

@fyodore82 Correct, the actual element is in focus after pressing ESC, but the focus is not visible.

[fyodore82]

Seems this issue is related to this line of code

if (nodeToRestore.current && nodeToRestore.current.focus) {
    nodeToRestore.current.focus();
}

TrapFocus component tries to restore focus when a modal is still opened. But a browser will not put focus on invisible elements. (somewhat related to this)

Adding setTimeout fixes this issue, but I don't think it is a good solution, so I'll continue investigation.

if (nodeToRestore.current && nodeToRestore.current.focus) {
    const btn = nodeToRestore.current;
    setTimeout(() => { btn.focus(); }, 100);
}

[oliviertassinari]

@fyodore82 It sounds like you have reached the point where I have stopped my investigation. It would be awesome if you could crack the case :). For instance, why does it work with the Dialog and not with the Modal? Why calling .focus() doesn't trigger a focus event?

[embeddedt]

Perhaps it's related to the fact that the Dialog has an animation built-in. If the function with these lines of code is delayed by the animation, that would be similar in timing to calling the setTimeout.

[fyodore82]

@oliviertassinari Thank you for mentioning Dialog component. @embeddedt , yes Fade component makes it work. Just adding Fade as child to Modal makes Modal return focus.

<Modal open={showLocalModal} onClose={() => setShowLocalModal(false)} >
  <Fade in={showLocalModal}>
    <div/>
  </Fade>
</Modal>

[fyodore82]

Seems to be a bug in React. Filed it

[oliviertassinari]

This comment https://github.com/facebook/react/issues/17894#issuecomment-656094405 suggests that the observed bug here will be fixed in React 17.