-
Notifications
You must be signed in to change notification settings - Fork 129
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
Fix useScrollLock instances issue #2860
Conversation
🦋 Changeset detectedLatest commit: 417925a The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #2860 +/- ##
=======================================
Coverage 87.83% 87.84%
=======================================
Files 225 225
Lines 13005 13012 +7
Branches 1776 1777 +1
=======================================
+ Hits 11423 11430 +7
Misses 1528 1528
Partials 54 54
|
Size Change: +53 B (+0.01%) Total Size: 679 kB
ℹ️ View Unchanged
|
Addresses #DSYS-883
Purpose
When testing nested modals, I noticed that two different instances of the
useScrollLock
collided and scroll lock no longer functioned as expected. On a single web page, we only need to disable scroll on the body element once. If a second instance of the hook is introduced by a second component, its cleanup fn will restore the scroll thus canceling the effect of the first instance.Approach and changes
Keep track of the number instances of the
useScrollLock
hook and only execute cleanup fn for the first instance.Definition of done