Open megvalcour opened 2 years ago
Here is a refactor I did while just working through the review, in case it helps at all. There's more we could clean up/refactor here too, but just an example of what I'm suggesting.
Thanks for the review, @khamer !
My thoughts on your thoughts:
once
-- I think in some instances, like lazyloading or entrance effects, you'll want it to fire once and then stop observing that element both performance and UX reasons. But for different, continual scroll effects like the one in your playground example, you'd want to continue observing the element until the component is unmounted. So, once
gives the dev that option in an easy, simple way, but like with my point above, the flexibility to work as they'd like.As always, @tspears1 , feel free to jump in if you disagree or want to add anything else!
First - yes, let's definitely get a concrete example of this working in BP. If Megan can't get to it before she leaves, and Tim is under water maybe I can give it a go when I have time.
Second - I have to agree with Megan's 2nd and 3rd points here. Seems like the standard is using callbacks so we should go that route, and the "once" is definitely handy and a lot of libs offer an option like that for different scenarios, too.
And lastly let's just appreciate this gem "instead of just having an intersection observer observing the element and calling the callback(s) as needed, you have the observer observing and then a watcher watching the results of the observer".
I think next steps are to get a working example up and merge it in!
A better comparison would be useMouse() or useFetch() further down the same page in the docs.
By returning the reactive isIntersecting
, you've already given everyone a way to run code when the value changes; callback and outCallback are redundant and shouldn't be required to construct useIntersectionObserver(). In a way, the value you're adding with UseIntersectionObserver is you're providing a reactive wrapper to something native that uses callbacks. The reactive wrapper shouldn't also use callbacks.
Most components should just use isIntersecting
; something like
<div class="animate__animated" :class="{ animate__fadeInUp: isIntersecting }">
As far as as once
- should once
only fire once no matter what the change is? what should it do if the element is already visible? What if you want to have one animation run only the first time but another run every time?
Having a watcher only run once is something like
const unwatch = watch(isIntersecting, () => {
...
unwatch()
})
@khamer @TristanMNorton okay looked at a few examples of this in the wild and thought about how to make it hopefully the most useful. The current setup that Megan and I proposed I think is helpful but its almost too restrictive and I now think unnecessary.
Here's my new thinking: demo
This still allows the engineer full access to all of the tools inside of IntersectionObserver and makes it work well with Vue 3 (that unref is key for sure). Its very easy if you're just trying to see if something is intersecting or not OR you do something more complicated if you needed to. The example above would be one pattern to use if you were still after the ONCE option we first laid out. Thoughts?
Sure, heading the right direction - the next thing I'd think about is whether we want this composable to work with a single or multiple targets. I'd guess optimizing for a single target makes the most sense, and given that, we can probably refactor to completely avoid passing a callback.
If you want it to support multiple targets - which seems overkill for this to me - then maybe a callback makes sense, but we need to change how the composable is used so you can pass in multiple elements.
Per https://workshop.imarc.com/projects/1486/tasks/216631, @tspears1 and I created a composable that leverages the Intersection Observer API and the onMounted/onUnMounted Vue hooks.
Import it into your Vue components so you can easily execute a callback (animation, lazyloading, pulling data from an API, etc.) based on an element entering or exiting the viewport.