streamich/react-use

`useLockBodyScroll` doesn't unlock the body on unmount

Open

#858 opened on Jan 3, 2020

View on GitHub
 (5 comments) (7 reactions) (1 assignee)TypeScript (43,979 stars) (3,273 forks)batch import
bughelp wanted

Description

What is the current behavior? When a component uses the useLockBodyScroll hook, body scrolling is not unlocked when the component unmounts. This is true when passing in true as the first argument, or when omitting the first argument.

Steps to reproduce it and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have extra dependencies other than react-use. Paste the link to your JSFiddle or CodeSandbox example below: This minimal example illustrates the issue: https://codesandbox.io/s/charming-goodall-p05r9

Before clicking Show dialog, the body scrolls. <Dialog> uses the useLockBodyScroll hook, so when it renders, the body no longer scrolls (as expected). However, after the dialog is unmounted by clicking Hide dialog, body scrolling is not restored.

What is the expected behavior? I would expect that the hook would clean up after itself when it is unmounted, restoring body scroll if it had been locked by the hook.

I suspect that the original intention of this hook was that it was rendered by a persistent component that is always on the page, and just toggled on and off via the locked argument. However, we have many use-cases for a component always wanting body scrolling to be locked while it's being rendered (for example, a dialog). It seems unwieldy to require a persistent parent of that component to be in charge of turning on and off body scroll locking.

A little about versions:

  • OS: macOS Catalina 10.15.1
  • Browser (vendor and version)_: Chrome 79.0.3945.88
  • React: 16.12.0
  • react-use: 13.14.0
  • Did this worked in the previous package version? No

Contributor guide

Tech stack
reacttypescript
Domain
frontend
Issue type
bug
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
2
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
under 1 hour
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
blocked
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
clear
Prerequisites
Understanding of React useEffect cleanup
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
60
Research direction
The issue is that useLockBodyScroll does not unlock body scroll on unmount. The hook is likely in src/useLockBodyScroll.ts. The fix requires adding a cleanup function in the useEffect that sets the body overflow style back to 'visible' when the component unmounts. Refer to the linked CodeSandbox for reproduction. Ensure that the cleanup is only called if the lock was applied.