Skip to content
This repository has been archived by the owner on Aug 21, 2023. It is now read-only.

ScrollLock: Locking fix + resolve console warning #587

Merged
merged 4 commits into from Apr 18, 2019

Conversation

ItsJonQ
Copy link
Contributor

@ItsJonQ ItsJonQ commented Apr 17, 2019

ScrollLock: Locking fix + resolve console warning

Screen Recording 2019-04-17 at 03 37 PM

This update fixes the ScrollLock mechanism to work with the newer browser +
React handling for wheel events.

The event listener is now added directly to the DOM node via addEventListener,
rather than tapping into the onWheel callback from React, which is now
considered "passive", and no longer provides us with the correct eventTarget.

@helpscout/cyan was added to help make it (much MUCH) easier to write
wheel event based tests, since these are now fired outside of React.

Fixes: #586

This update fixes the ScrollLock mechanism to work with the newer browser +
React handling for `wheel` events.

The event listener is now added directly to the DOM node via `addEventListener`,
rather than tapping into the `onWheel` callback from React, which is now
considered "passive", and no longer provides us with the correct `eventTarget`.

`@helpscout/cyan` was added to help make it (much MUCH) easier to write
`wheel` event based tests, since these are now fired outside of React.
@ItsJonQ ItsJonQ added bug 🐛 patch ☝️ To indicate patch version bumps labels Apr 17, 2019
@ItsJonQ ItsJonQ self-assigned this Apr 17, 2019

describe('Direction: Y', () => {
test('Permits scrolling within extent', () => {
const wrapper = shallow(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A lot of these tests had to be refactored since Enzyme's mount/shallow rendering can't correctly simulate an event (wheel) if bound via addEventListener.

@netlify
Copy link

netlify bot commented Apr 17, 2019

Deploy preview for hsds-react ready!

Built with commit 7ddd13d

https://deploy-preview-587--hsds-react.netlify.com

@ItsJonQ ItsJonQ requested a review from Charca April 17, 2019 19:44
@netlify
Copy link

netlify bot commented Apr 17, 2019

Deploy preview for hsds-react ready!

Built with commit 1565343

https://deploy-preview-587--hsds-react.netlify.com

@@ -183,7 +184,7 @@
"jest-cli": "^23.6.0",
"jest-watch-typeahead": "^0.2.0",
"mkdirp": "0.5.1",
"node-sass": "^4.9.2",
"node-sass": "4.11.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update from npm audit fix

@@ -10,3 +11,5 @@ beforeEach(() => {
afterEach(() => {
window.requestAnimationFrame.mockRestore()
})

setupTests()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Integrating with @helpscout/cyan

@coveralls
Copy link

coveralls commented Apr 17, 2019

Pull Request Test Coverage Report for Build 2012

  • 19 of 19 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 2008: 0.0%
Covered Lines: 7599
Relevant Lines: 7599

💛 - Coveralls

@ItsJonQ ItsJonQ merged commit 1f9af92 into master Apr 18, 2019
@ItsJonQ ItsJonQ deleted the scroll-lock-fix-error branch April 29, 2019 16:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug 🐛 patch ☝️ To indicate patch version bumps
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scrollable: Console error shows up when scrolling past the limits of the component
2 participants