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

Problem when using hooks + multiple components #329

Open
thismarcoantonio opened this issue Aug 27, 2019 · 14 comments
Open

Problem when using hooks + multiple components #329

thismarcoantonio opened this issue Aug 27, 2019 · 14 comments

Comments

@thismarcoantonio
Copy link

thismarcoantonio commented Aug 27, 2019

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

@Pomax
Copy link
Owner

Pomax commented Aug 27, 2019

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.

@thismarcoantonio
Copy link
Author

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?

@elya158-zz
Copy link

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

@Quocnamit
Copy link

can we use class component to fix it ?

@Quocnamit
Copy link

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

@cypherfunc
Copy link

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.

@thismarcoantonio
Copy link
Author

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

@abominab
Copy link

abominab commented Sep 18, 2019

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.

@strongui
Copy link

strongui commented Sep 25, 2019

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

vlbee added a commit to PrideInLondon/pride-london-web that referenced this issue Jan 26, 2020
egmcdonald pushed a commit to PrideInLondon/pride-london-web that referenced this issue Feb 16, 2020
SonyaMoisset pushed a commit to PrideInLondon/pride-london-web that referenced this issue Feb 16, 2020
* WEBREF-21 Refactor EventDropdownFilter to use hooks

* WEBREF-21 Fix broken onClickOutside functionality in EventDropDownFilter

The previously used library was broken in the hooks implementation:
Pomax/react-onclickoutside#329

Replaced it with https://www.npmjs.com/package/use-onclickoutside

* WEBREF-21: Refactor to use context hook

* WEBREF-21: Replace shouldComponentUpdate with React.memo

* WEBREF-21: Fix useContext

* Add comment about broken react-onclickoutside lib

* Destructure AppContext state

* WEBREF-21 Remove useEffect and refactor context
@metalass
Copy link

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.

@FranciscoMateusVG
Copy link

FranciscoMateusVG commented May 26, 2020

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

@Makwe-O
Copy link

Makwe-O commented Nov 2, 2020

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

@iqor
Copy link

iqor commented Nov 19, 2020

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

@antal-bukos
Copy link

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

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