joshwnj / joshwnj/react-visibility-sensor

Revamp API to cleanly separate configuration and effects

Open
#74 3 comments 2 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
2.3k
Forks
192
PR merge metrics
No merged PRs in 30d

Description

This is a more philosophical question about the API of this lib

Currently, the component depends on its effects propagating upwards by a lifecycle-ish `onChange` method, which usually triggers a change in state of the parent component / update store etc. However, this approach merges together the _configuration_ of the component (`scrollDelay`, `intervalDelay` etc), with its _effects_ (`onChange`), which makes it slightly confusing to grok IMO.

Inspired by React Router v4, I propose the following API

```jsx
const MyVisibilitySensor = createSensor({
delayInterval : 200,
// other opts
});

const App = () => (

{
({ isVisible }) => (
isVisible ? :
)
}

);
```

This separates concerns very nicely, and also avoids the overhead of having a situation where we have a hierarchy of parent component -> visibility sensor, and the `onChange` callback triggers a re-render from the parent component downwards. It helps avoid one layer of lifting state up, and is easier to read IMO.

Thoughts?

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.