Add partialVisiblity property.

[kompot]

If partialVisibility is set then element is considered visible if only part of it is visible.

[joshwnj]

Nice idea, thanks @kompot ! Sorry I'm not merging straight away, I'd just like to take a bit of a closer look tomorrow morning and explore more how it impacts existing functionality. Probably write a new test case too. Will respond to this soon.

[kompot]

Thanks, great!

[kof]

Suggestion: rename isVisible property to visibility which can have values: 'visible', 'hidden', 'partial' or similar ...

[kof]

onChange(visibility, rect)

[joshwnj]

@kompot @kof all good suggestions.

I'm wondering whether we can find something backwards-compatible. What would you all think about leaving onChange to mean a change from "fully visible or not", and also adding a new optional onPartialChange callback which indicates a change from "at least partially visible or not visible at all".

@kompot would that work for your use-case?

[kompot]

onPartialChange would work ok for me, so any way everybody agrees on.

But there might be some confusion. Consider element is fully visible, then it becomes partially visible. Should we fire onPartialChange? According to the name — may be. But logically I guess no. In my case definitely not.

@kof 's suggestion to make it enum instead of boolean seems reasonable to me.

[joshwnj]

@kompot yeah the reason I'm thinking 2 separate callbacks instead of a single with enum value is that question of "when is the callback called?". I'm thinking that in most cases you would either care about full visibility or partial visibility, but not so much the transition from partial-to-full.

But I agree the naming I proposed could be tightened up to be clearer.

[joshwnj]

Another option that would preserve backwards-compatibility:

• add a threshold prop, which has a default value of 1.0.
• the check function will calculate what percentage of the sensor is visible and give a value between 0.0 (fully hidden) and 1.0 (fully visible).
• the onChange callback is only called when visibility crosses the threshold in either direction.

What do you think?

[kof]

I think with proper versioning and changelog, breaking changes are not an issue at all.

👎 for 2 callbacks, makes api more complex

Libs should not be opinionated. I can perfectly think of use cases where its important to know when element changes from partial to visible or hidden states.

[joshwnj]

Sure, I agree that we don't want to overly complicate things here.

@kompot I've had a closer look and I think the way you've implemented it is good as it won't change the behaviour until you opt-in with the partialVisibility prop.

I'd like to consider a more comprehensive solution that allows you to get notifications about both partial and full visibility. Possibly removing the partialVisibility prop and making this the default behaviour. But as this will possibly be a breaking change I'll merge this PR for now. We'll want to add a test-case for partial visibility moving forward as well.

Thanks!

[joshwnj]

@kompot published as v2.1.0

[kof]

@joshwnj I think @kompot agreed on my enum proposal, lets move it in this direction and release 3.0

[joshwnj]

Yes, I think it's a good plan. We can also see if there are any other breaking-changes needing to be made to roll into 3.0

Before then, it's a good opportunity to try partial visibility some more and see how it affects things. Eg. I'm thinking a "threshold" prop will definitely be useful because at the moment even 1px of visibility will trigger onChange, and in a lot of cases you'd want it to be less sensitive.

I'll create a new issue to discuss any other things people want to see in 3.0