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

Implement useIntersection hook. #652

Merged
merged 7 commits into from
Oct 12, 2019

Conversation

kevinnorris
Copy link
Contributor

As described in #604.

@wardoost wardoost self-requested a review October 9, 2019 08:05
Copy link
Contributor

@wardoost wardoost left a comment

Choose a reason for hiding this comment

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

Looks great 👏

I have a few small comments + would you be able to add tests?

README.md Outdated Show resolved Hide resolved
docs/useIntersection.md Outdated Show resolved Hide resolved
Kevin Norris added 2 commits October 10, 2019 20:48
@kevinnorris
Copy link
Contributor Author

I'm not sure how to test the main functionality of the hook, as it requires a ref and elements intersecting via scrolling. It's likely possible by using a library to fully render the DOM and then force scroll events. Any thoughts on a test approach would be appreciated. 😃

@peter-mouland
Copy link

@kevinnorris I recently created a hook very similar to this and used shopify mocks for the testing

import { intersectionObserver } from '@shopify/jest-dom-mocks';

@wardoost
Copy link
Contributor

@kevinnorris I think you could either

I'm not sure what is best in this case, @Belco90 might have a suggestion on this.

@Belco90
Copy link
Contributor

Belco90 commented Oct 10, 2019

Never done this before for intersectionObserver but I'd go for mocking it and just assume it will do its thing in the real world. How to do it is up to you I guess: mocking it manually or using a third party library for it (that @shopify/jest-dom-mocks that @peter-mouland mentioned looks really interesting).

src/useIntersection.ts Outdated Show resolved Hide resolved
@wardoost wardoost merged commit d5f359f into streamich:master Oct 12, 2019
streamich pushed a commit that referenced this pull request Oct 12, 2019
# [12.4.0](v12.3.2...v12.4.0) (2019-10-12)

### Features

* useIntersection ([#652](#652)) ([d5f359f](d5f359f))
@streamich
Copy link
Owner

🎉 This PR is included in version 12.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants