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

cancel swipe depending on the return of onSwiping* callbacks #28

Closed
MoOx opened this issue Jan 8, 2016 · 5 comments · Fixed by #32
Closed

cancel swipe depending on the return of onSwiping* callbacks #28

MoOx opened this issue Jan 8, 2016 · 5 comments · Fixed by #32

Comments

@MoOx
Copy link
Contributor

MoOx commented Jan 8, 2016

That might help me to implement a pull to refresh only when I am swiping at the top of my container. I would be able to detect if the container is at its top and return true/false if not which might trigger the scroll or not. Currently I use onSwipingDown but it's blocking the scroll up :)

@MoOx
Copy link
Contributor Author

MoOx commented Jan 8, 2016

It's easy

        if (this.props.onSwipingDown) {
-          this.props.onSwipingDown(e, pos.absY)
-          cancelPageSwipe = true
+          cancelPageSwipe = this.props.onSwipingDown(e, pos.absY)
        }

//...

-    if (cancelPageSwipe) {
+    if (cancelPageSwipe === undefined || cancelPageSwipe) {
      e.preventDefault()
    }

With this diff, no breaking change and you can control from onSwipingDown if you want to prevent default or not.
Can be make for all onSwiping* too.
Are you open for this change?

@MoOx
Copy link
Contributor Author

MoOx commented Jan 20, 2016

Are you open for this (tiny but very cool) change?

@hartzis
Copy link
Collaborator

hartzis commented Jan 20, 2016

Personally I do think it is interesting and neat hack. Just wondering if it could be made more explicit because if (cancelPageSwipe === undefined || cancelPageSwipe) doesn't look/feel right.

It also seems it would be a major version bump. Users could randomly be returning values from their callbacks of onSwiping's.

@goatslacker thoughts on this?

@MoOx
Copy link
Contributor Author

MoOx commented Jan 20, 2016

The (cancelPageSwipe === undefined || cancelPageSwipe) was to avoid breaking changes, but you are right, that might introduce some changes anyway.
So if that's a breaking change, we can assume that onSwiping* must return a boolean.
Or we can reverse the thing: noCancelPageSwipe = this.props.onSwipingDown(e, pos.absY) (return undefined or false => no preventDefault, return true: preventDefault.
Hum so, noCancelPageSwipe will be a preventDefault flag.

@hartzis
Copy link
Collaborator

hartzis commented Jan 23, 2016

this seems very much related to a bigger issue with #21, and androids and old ios. any ideas?

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

Successfully merging a pull request may close this issue.

2 participants