search

atomiks / tippyjs

Tooltip, popover, dropdown, and menu library
https://atomiks.github.io/tippyjs/
MIT License
11.86k stars 520 forks source link

Calling hide breaks focus restoration when window is blurred #1064

Open zhouzi opened 2 years ago

zhouzi commented 2 years ago

Bug description

Tippy manually blurs the active element when the window is blurred, if a tooltip is attached to it and if the tooltip is hidden:

https://github.com/atomiks/tippyjs/blob/ad85f6feb79cf6c5853c43bf1b2a50c4fa98e7a1/src/bindGlobalEventListeners.ts#L42-L58

This is breaking the browser's focus restoration when switching back to the window.

Reproduction

StackBlitz link: https://stackblitz.com/edit/web-platform-briuf1?file=index.html

  1. Click in the input with a tooltip, the tooltip is shown
  2. Press alt+tab to switch to another window
  3. Press alt+tab again to switch back to the window with the input

The input should be focused but it's not the case. The reproduction includes an input without a tooltip so you can compare the behaviour.

It seems that this was initially implemented as a fix to #72. It would argue that it goes against the browser's behaviour and what's reported in #72 is not an issue but what is expected. But there might be other valid reasons I am not aware of.

Let me know what you think and I'd be happy to submit a PR.

zhouzi commented 2 years ago

A solution that wouldn't run the risk of breaking current users' expectations would be to add an option that defaults to the current behaviour. With a way to turn it off.

atomiks commented 2 years ago

I agree it's expected in this case, what it was trying to fix was the case where the ref has focus but the tippy is hidden (due to the hideOnClick: true behavior), coming back to the window would show it unexpectedly. Probably instead it should be checking to see if it's visible at the time the window is blurred.

EDIT: okay, it checks isVisible but the ref gets blurred when the window is blurred so that's why the check is failing I guess? I wonder if this behavior has changed in browsers somewhat recently? Checking isMounted instead would work, provided it has an animation/transition. If not it would also fail in this case.

zhouzi commented 2 years ago

okay, it checks isVisible but the ref gets blurred when the window is blurred so that's why the check is failing I guess?

What do you mean by the check is failing? The input receives a blur event, isVisible is false so it blurs the focused element. Seems fine.

When switching window, the input receives a blur event but it's still the activeElement. We could leverage that to set a ignoreNextFocusEvent to true. Alternatively we could store a reference of the new activeElement on blur and then compare it with the reference on focus. If it's the same then ignore the focus event. But that's more work and data to store in memory.

atomiks commented 2 years ago

What do you mean by the check is failing? The input receives a blur event, isVisible is false so it blurs the focused element. Seems fine.

If the tippy was visible at the time the window blurred and the input lost focus, it shouldn't close. But the check fails here because the trigger blur event is firing right before the window blur event (I think), so it thinks it's not visible at the time the window blurred even though it was.

But yeah open to a PR with a better solution 👍

zhouzi commented 2 years ago

I wanted to help with this and submit a PR but I can't install tippy locally. I have a MacBook with an Apple M1 processor and ran into these issues: