react-onclickoutside icon indicating copy to clipboard operation
react-onclickoutside copied to clipboard

Problem when using hooks + multiple components

Open thismarcoantonio opened this issue 5 years ago • 14 comments

Hi everyone,

I'm having a problem, I've been using this library for a while working 100%. Today I tried to implement it in another part of my code, and it just don't work properly. I've tried this in version 6.8.0 and 6.9.0.

Here's my code:

import React from "react"
import onClickOutside from "react-onclickoutside"

function FocusHandler({ children }) {
  const [hasFocus, setFocus] = React.useState(false)
  FocusHandler.handleClickOutside = () => setFocus(false)

  return (
    <div style={{ width: "100%" }} onFocus={() => setFocus(true)}>
      {children({ hasFocus, setFocus })}
    </div>
  )
}

const clickOutsideConfig = {
  handleClickOutside: () => FocusHandler.handleClickOutside
}

export default onClickOutside(FocusHandler, clickOutsideConfig)

If I only use this in one component, everything works properly. If I two or more components, it breaks.

EDIT: Also made a simple example: https://codesandbox.io/embed/react-onclickoutside-bug-report-jc963

thismarcoantonio avatar Aug 27 '19 14:08 thismarcoantonio

This HOC works based on the assumption that it wraps true instances, so if you use a functional component, and you only use one of them on a page, it will likely work, but if you use more than one, the last one "wins" and is the only one that'll work because the FocusHandler gets treated as an instance, which as singleton function object, will definitely go wrong.

Pomax avatar Aug 27 '19 16:08 Pomax

I see... there is any way to get around that? Should I use a new HOC for every component? Why the functional difference matters here?

thismarcoantonio avatar Aug 27 '19 17:08 thismarcoantonio

I have this issue as well, will there be a fix for this?

elya158-zz avatar Sep 01 '19 13:09 elya158-zz

can we use class component to fix it ?

Quocnamit avatar Sep 10 '19 07:09 Quocnamit

https://codesandbox.io/s/loving-newton-754t8 I try to use the class component. so the event can apply to multiples components

Quocnamit avatar Sep 10 '19 07:09 Quocnamit

FocusHandler.handleClickOutside = () => setFocus(false)

This line is the problem. This ties a static method to a single instance, which is a serious anti-pattern in React.

The class-based usage is fine, because it uses an instance method instead of a static method.

cypherfunc avatar Sep 17 '19 18:09 cypherfunc

Makes sense, don't know why I didn't realised about it before...

thismarcoantonio avatar Sep 18 '19 05:09 thismarcoantonio

The linked example in this section (https://github.com/Pomax/react-onclickoutside#functional-component-with-usestate-hook) will demonstrate this bug if you put 2 Menu components in index.js Would be nice if this got fixed internally so that we can use this w/ functional components.

abominab avatar Sep 18 '19 15:09 abominab

Is this something that will be addressed in a future release?

strongui avatar Sep 25 '19 14:09 strongui

Managed to resolve it with introducing unique name (id) for the component. So, imagine we have name prop which value is unique. We'd need to adjust static component property assignment in order to set unique property:

Menu['handleClickOutside_' + name] = () => setIsOpen(false);

And config:

const clickOutsideConfig = {
    handleClickOutside: ({ props }) => Menu['handleClickOutside_' + props.name]
};

Though, this is a dirty workaround, hope it would help someone.

metalass avatar Apr 14 '20 13:04 metalass

@metalass you sir deserve a medal! Worked like a charm in "react": "^16.13.1" and "react-onclickoutside": "^6.9.0".

FranciscoMateusVG avatar May 26 '20 19:05 FranciscoMateusVG

OMG. Thank you for this @metalass. It's a workaround but it definitely works and makes sense!!

Makwe-O avatar Nov 02 '20 14:11 Makwe-O

Ty @metalass , worked fine on "react": "^17.0.1" and "react-onclickoutside": "^6.9.0"

iqor avatar Nov 19 '20 12:11 iqor

Using this hook also works https://usehooks.com/useOnClickOutside/.

antal-bukos avatar Feb 08 '21 21:02 antal-bukos