Skip to content
This repository was archived by the owner on Jul 5, 2021. It is now read-only.
Merged
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@nearform/react-browser-hooks",
"version": "2.1.0",
"version": "2.2.0",
"description": "react-browser-hooks React component",
"main": "lib/index.js",
"module": "es/index.js",
Expand Down
32 changes: 32 additions & 0 deletions src/hooks/click-outside.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { useEffect } from 'react'

export function useClickOutside(el, options = {}, onClick) {
const els = [].concat(el)
let active = true

if (!onClick && typeof options === 'function') {
onClick = options
} else {
active = options.active
}

const handler = (ev) => {
const target = ev.target

if (els.every((ref) => !ref.current || !ref.current.contains(target))) {
onClick(ev)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add the window.removeEventListener('click', handler) inside here also to clean it up on off click but not necessarily unmount 👍

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this remove the handler for that component on the first off-click? I think we want to keep it there until it's unmounted.

I guess we do need to check each ref exists though, as each may be unmounted, and the cleanup handler of useEffect might not be called it may not be a ref to the component that is consuming the hook. (If that makes sense).

I guess the handler should also be removed if all els are falsey.

Copy link
Author

@nathanpower nathanpower Feb 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the handler should also be removed if all els are falsey.

Actually, on second thoughts, probably shouldn't do that. There is nothing stopping a component passing an array of refs to components other than itself, and this component will expect the handler to stay around for it's own lifetime, even if all the targets are unmounted and remounted again. I'll go with the truthy check for now.

It's a bit of an edge case I guess, I imagine most usage will be the component passing its own ref.

Copy link
Contributor

@jh3y jh3y Feb 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will remove it you're right. And that's because the effect only runs when el changes.
This does bring up something I think we've overlooked though 🤦‍♂️
You don't want to bind the handler on every render. Because that means the off click callback is firing even when it doesn't need to.
Think of the case where you have a menu that slides in and out. You don't want the handler firing every time the user clicks the page even if the menu is closed. So maybe it requires a parameter that tells it if it can bind, then we can conditionally bind the event in the useEffect function.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't want to bind the handler on every render

I thought that it bound only when el changed, (as well as mount/unmount) as el is passed in the array of dependencies (last argument to useEffect). Am I wrong there?

In any case, I think we only want to bind the handler on mount, and remove on unmount, so I think I can pass an empty array as last argument to achieve that.

Passing in an empty array [] of inputs tells React that your effect doesn’t depend on any values from the component, so that effect would run only on mount and clean up on unmount; it won’t run on updates. - https://reactjs.org/docs/hooks-reference.html#useeffect

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Sorry I probably didn't word that right.

Currently, that handler will fire on every click as soon as the component is mounted. You don't want that.

You only want to bind when some condition is met. For example, a menu in an open state.

Copy link
Author

@nathanpower nathanpower Feb 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an interesting one... should be enforce that this condition be supplied, or would you be OK with opt-in, like 09b7b01?

Here we are back to running the hook on every render, but adding/removing the listener depending on an optional predicate.

You would use this like e.g useClickOutside(ref, optionsVisible, hideOptions)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think that makes sense 👍

}
}

const cleanup = () => window.removeEventListener('click', handler)

useEffect(() => {
if (active) {
window.addEventListener('click', handler)
} else {
cleanup()
}

return cleanup
})
}
1 change: 1 addition & 0 deletions src/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from './hooks/click-outside'
export * from './hooks/fullscreen'
export * from './hooks/geolocation'
export * from './hooks/mouse-position'
Expand Down
35 changes: 35 additions & 0 deletions storybook/stories/click-outside/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
## Click Outside Hook

The Click Outside Hook attaches a listener which will callback the target component with the event object on any click which is not on the target component, or a child of the target component.

Import as follows:

```javascript
import { useClickOutside } from '@nearform/react-browser-hooks'
```

Example of usage:

```javascript
useClickOutside(ref, onClick)
```

Callback is invoked with the original event as the only argument

Also supported is passing an array of refs, where `onClick` will only be called if the click target is outside _all_ of the components referenced.

```javascript
useClickOutside([ref, siblingRef], onClick)
```

Avoiding unnecessary callbacks:

If you have a large app with many components using this hook, you may wish to avoid calling the callback when not necessary. In this situation you can pass options as the second argument, and include an `active` property. The callback will not be invoked if this is falsey. For example, if you you had a dropdown where you are only interested in receiving a callback if the options are visible, you might use like:

```javascript
const [optionsVisible, setOptionsVisible] = useState(false)
const hideOptions = () => setOptionsVisible(false)
useClickOutside(ref, { active: optionsVisible }, hideOptions)
```

In this example, `hideOptions` will never be called if `optionsVisible` is already `false`.
46 changes: 46 additions & 0 deletions storybook/stories/click-outside/click-outside.stories.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import React, { useRef, useState } from 'react'
import { storiesOf } from '@storybook/react'
import { withReadme } from 'storybook-readme'
import { useClickOutside } from '../../../src'
import readme from './README.md'

function ClickOutside() {
const elRef = useRef(null)
const [clickEvent, setClickEvent] = useState(null)
useClickOutside(elRef, (ev) => {
setClickEvent(ev)
})

const message =
clickEvent === null
? ''
: `Clicked outside (x: ${clickEvent.clientX}, y: ${clickEvent.clientY})`

return (
<>
<h2>Click Outside Demo</h2>
<em>Click outside target component receives full event</em>
<div
style={{
display: 'flex',
alignItems: 'center',
justifyContent: 'center',
color: 'white',
marginTop: 15,
position: 'absolute',
width: '30%',
height: '15%',
background: 'grey'
}}
ref={elRef}
onClick={() => setClickEvent(null)}>
{message}
</div>
</>
)
}

storiesOf('Click Outside', module).add(
'Default',
withReadme(readme, () => <ClickOutside />)
)
215 changes: 215 additions & 0 deletions test/unit/hooks/click-outside.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
import React, { forwardRef, createRef } from 'react'
import { cleanup, fireEvent, render } from 'react-testing-library'
import { act } from 'react-dom/test-utils'

import { useClickOutside } from '../../../src'

let callback
let testElementRef
let childElementRef
let siblingRef
let testHook
let testHookWithSibling
let testHookWithActiveState
let active

beforeEach(() => {
testElementRef = createRef()
childElementRef = createRef()
siblingRef = createRef()
const TestChildComponent = forwardRef((props, ref) => {
return <div ref={ref} />
})

const TestHook = forwardRef(({ callback }, ref) => {
useClickOutside(ref, callback)
return (
<div ref={ref}>
<TestChildComponent ref={childElementRef} />
</div>
)
})

const TestHookWithActiveState = forwardRef(({ callback }, ref) => {
useClickOutside(ref, { active }, callback)
return (
<div ref={ref}>
<TestChildComponent ref={childElementRef} />
</div>
)
})

const TestHookWithSibling = forwardRef(({ callback }, ref) => {
useClickOutside([ref, siblingRef], callback)
return (
<>
<div ref={siblingRef} />
<div ref={ref}>
<TestChildComponent ref={childElementRef} />
</div>
</>
)
})

testHook = (callback) => {
render(<TestHook ref={testElementRef} callback={callback} />)
}

testHookWithActiveState = (callback) => {
render(<TestHookWithActiveState ref={testElementRef} callback={callback} />)
}

testHookWithSibling = (callback) => {
render(<TestHookWithSibling ref={testElementRef} callback={callback} />)
}

callback = jest.fn()
})

afterEach(cleanup)

describe('useClickOutside', () => {
it('calls callback with click event on clicking outside the component', () => {
testHook(callback)
act(() => {
fireEvent(
document.body,
new Event('click', {
bubbles: true,
cancelable: false
})
)
})

expect(callback).toBeCalledTimes(1)
expect(callback.mock.calls[0].length).toBe(1)
expect(callback.mock.calls[0][0] instanceof Event).toBe(true)
expect(callback.mock.calls[0][0].type).toBe('click')
})

it('calls callback if calling component is active', () => {
active = true
testHookWithActiveState(callback)
act(() => {
fireEvent(
document.body,
new Event('click', {
bubbles: true,
cancelable: false
})
)
})

expect(callback).toBeCalledTimes(1)
expect(callback.mock.calls[0].length).toBe(1)
expect(callback.mock.calls[0][0] instanceof Event).toBe(true)
expect(callback.mock.calls[0][0].type).toBe('click')
})

it('does not call callback if calling component is inactive', () => {
active = false
testHookWithActiveState(callback)
act(() => {
fireEvent(
document.body,
new Event('click', {
bubbles: true,
cancelable: false
})
)
})

expect(callback).toBeCalledTimes(0)
})

it('does not call callback when the component itself receives a click', () => {
testHook(callback)
act(() => {
fireEvent(
testElementRef.current,
new Event('click', {
bubbles: true,
cancelable: false
})
)
})

expect(callback).toBeCalledTimes(0)
})

it('does not call callback when a child receives a click', () => {
testHook(callback)
act(() => {
fireEvent(
childElementRef.current,
new Event('click', {
bubbles: true,
cancelable: false
})
)
})

expect(callback).toBeCalledTimes(0)
})

it('supports array of refs, and will call callback if target is not contained by any', () => {
testHookWithSibling(callback)
act(() => {
fireEvent(
document.body,
new Event('click', {
bubbles: true,
cancelable: false
})
)
})

expect(callback).toBeCalledTimes(1)
expect(callback.mock.calls[0].length).toBe(1)
expect(callback.mock.calls[0][0] instanceof Event).toBe(true)
expect(callback.mock.calls[0][0].type).toBe('click')
})

it('handles null ref.current', () => {
siblingRef.current = null
testHookWithSibling(callback)
act(() => {
fireEvent(
document.body,
new Event('click', {
bubbles: true,
cancelable: false
})
)
})

expect(callback).toBeCalledTimes(1)
expect(callback.mock.calls[0].length).toBe(1)
expect(callback.mock.calls[0][0] instanceof Event).toBe(true)
expect(callback.mock.calls[0][0].type).toBe('click')
})

it('supports array of refs, and will not call callback if target is contained by any', () => {
testHookWithSibling(callback)
act(() => {
fireEvent(
siblingRef.current,
new Event('click', {
bubbles: true,
cancelable: false
})
)
})
act(() => {
fireEvent(
testElementRef.current,
new Event('click', {
bubbles: true,
cancelable: false
})
)
})

expect(callback).toBeCalledTimes(0)
})
})