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

[RFC] Switch mapPropsOnScroll to withWindowScroll #23

Open
wuct opened this issue Apr 28, 2016 · 1 comment
Open

[RFC] Switch mapPropsOnScroll to withWindowScroll #23

wuct opened this issue Apr 28, 2016 · 1 comment

Comments

@wuct
Copy link
Owner

wuct commented Apr 28, 2016

The propMapper of mapPropsOnScroll() currently accept two args

(nextScroll, prevScroll): Object => {}

IMHO prevScroll might be not used in some cases, so we should let consumers to save it by themselves if they need it. It is easily to achieve by using recompose/mapProps or rx-recompose/mapPropsStream and is already implemented in React componentWillReceiveProps().

If mapPropsOnScroll only provides nextScroll, it would be no difference to compose(withWindowScroll, mapProps) where withWindowScroll simply append scroll to props.

In addition, withWindowScroll would be more consistent with other HOCs in react-dom-utils.

@evenchange4 what do you think?

@wuct
Copy link
Owner Author

wuct commented Apr 28, 2016

Actually, not. I think we should refactor some of withXXXs to mapPropsOnXXXs.

To implement a withXXX, we have to be opinionated on two things:

  1. The name of the prop to append. For examples: windowSize, DOMSize or mousePosition.
  2. The events to listen to. For examples, resize, mousemove or mouseleave.

To implement a mapPropsOnXXX, we can avoid them.

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

No branches or pull requests

1 participant