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.

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

Menu does not close when using Preact #240

Closed
christianjuth opened this issue Aug 23, 2021 · 1 comment
Closed

Menu does not close when using Preact #240

christianjuth opened this issue Aug 23, 2021 · 1 comment

Comments

@christianjuth
Copy link

The issue

When using preact, the onBlur event does not seem to fire. I think this is related to the fact that React bubbles blur events up to the parent element, but Preact doesn't (see this issue). I think this can be easily fixed by using focusout instead of onBlur.

Example fix

This is just a rough example to demon straight my point. Obviously, I'm leaving some things out. I already implemented this change in my own project, but it would be better if it was done at the library level.

function Menu() {
  const ref = useRef(null)

  useEffect(() => {
    const container = ref.current

    const handleClose = (e) => {
      // Same logic as https://github.com/szhsin/react-menu/blob/6ef8d95a82af27a15023d3f5a5f035a387e9a2df/src/components/ControlledMenu.js#L109
      // ...
    }

    if (container) {
      container.addEventListener('focusout', handleClose)
      return () => {
        container.removeEventListener('focusout', handleClose)
      }
    }
  }, [])

  return (
    <ControlledMenu ref={ref}/>
  )
}
@szhsin
Copy link
Owner

szhsin commented Aug 24, 2021

Hi @christianjuth , thank for the feedback.
It's very interesting to see this little library is being used with Preact. I don't know much about Preact, but I think the issue here is that React has it's own synthetic event system, where as Preact uses native browser events directly.
So in React the onBlur has some special handling which is built on top of the onFocusOut facebook/react#19186 and allows it to bubble up.

I had a look of latest React 16/17 and they don't actually expose onFocusOut as part of the event system; you get this warning when trying to use onFocusOut:

Warning: React uses onFocus and onBlur instead of onFocusIn and onFocusOut. All React events are normalized to bubble, so onFocusIn and onFocusOut are not needed/supported by React.

I know we might around it by using addEventListener/removeEventListener. However, the library is built for React and it works well with React. Sticking to React's event system keeps codebase consistent and offers benefits which might include state update batching or better compatibility with future versions of React.
There is no real incentive to implement this for the compatibility with Preact, as you might get other problems in Preact.

@szhsin szhsin closed this as completed Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants