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

Prevent Default Behavior of Gesture (Back Navigation on Mobile) #1

Closed
mjsisley opened this issue Mar 30, 2018 · 9 comments
Closed

Prevent Default Behavior of Gesture (Back Navigation on Mobile) #1

mjsisley opened this issue Mar 30, 2018 · 9 comments

Comments

@mjsisley
Copy link

On mobile browsers (at least on iOS: Safari, Chrome), I'm getting the default behavior of a swipe to the left triggering a back navigation (along with the gesture provided with this library). It would be great if this library exposed a way to prevent that default behavior.

I see you removed preventDefault on the last release, I'm not sure what the issue was... but it would be great if there was some way to configure this (I'm not seeing one atm).

@drcmda
Copy link
Member

drcmda commented Apr 5, 2018

@mjsisley i think i got some console warnings in chrome, didn't they change the way events worked? I'm a bit out of the loop, do you know what would be the right way to do it? Would you go for an option, or use a reasonable default.

@zvictor
Copy link
Contributor

zvictor commented Dec 31, 2018

I am having a similar issue with Chrome reloading the page when I swipe down. Having the ability to stopPropagation/preventDefault is a must 🤳

At first I tried using onAction to achieve this goal, expecting it to send down the event, but that's not possible. So, my proposal is that we change the api to expose event somehow.

// proposal A
//  send `event` as extra argument in `onAction`
useGesture({ onAction: (props, event) => event.preventDefault() })


// proposal B
//  provide exclusive handler for `event`
useGesture({ onEvent: event => event.preventDefault() })

@drcmda
Copy link
Member

drcmda commented Jan 1, 2019

Makes sense, the event is now included in 3.0.3. I've switched it to passive mode, though, so in order to be able to do e.preventDefault it would be something like this:

useEvent({ transient: true, onAction: props => props.event.preventDefault(), passive: false })

@niwsa
Copy link

niwsa commented Jan 2, 2019

@drcmda Is it possible to preventDefault in render props pattern? I don't quite understand transient mode, how are things different in transient mode will something like below work ?

code

Getting event.preventDefault is not a function in mobile.

@drcmda
Copy link
Member

drcmda commented Jan 2, 2019

Yeah, I noticed, there’s no way currently to pass props. I think it would be a breaking change at least for the HOC. Should we do it?

@niwsa
Copy link

niwsa commented Jan 2, 2019

Seems yes from my side.I am facing the same issue that Victor mentioned i.e the browser refreshing the page on swiping down.Hopefully the breaking change you mentioned won't cause issues somewhere else or do they (I don't have the whole picture)?

@zvictor
Copy link
Contributor

zvictor commented Jan 2, 2019

thanks for the great and quick work, @drcmda!
I pushed a little as #18 and I would like to also make a comment about the passive prop added by you.

I think the code you posted above contains some imprecisions. Wouldn't the correct way be as below?

useGesture({ transient: true, onAction: props => props.event.preventDefault(), passive: {passive: false} })

If my understanding of the code is correct we are sending the prop passive, as it is, down as the third argument of addEventListener, which is called options there. I would then recommend calling this prop something that relates to its original name, e.g. listenerOptions or just options.

useGesture({ transient: true, onAction: props => props.event.preventDefault(), options: {passive: false} })

@drcmda
Copy link
Member

drcmda commented Jan 3, 2019

I've made a rough draft here, it fulfils all options and does some extras (like calculating directions, velocity, local deltas, etc.)

https://codesandbox.io/embed/n10mvnxmmm

You can exchange the demos in the render function (Drag/Pull/Decay)

/** [bind, set] = useGesture(eventHandler: {
      event       base event
      down        mouse down / touch start
      first       initial event?
      target      dom node
      time        time tag
      initial     first click/touch position (vec2)
      xy          page position (vec2)
      delta       delta (xy - initial) (vec2)
      previous    previous delta (vec2)
      direction   direction normal (vec2)
      local       delta bookkeeping (stores drop-point) (vec2)
      lastLocal   internal, used by local
      velocity    drag speed/momentuum
      args        args passed to bind(...)
      temp        args (optionally) returned by eventHandler
    })

    or ...

    bind = useGesture({ onAction, passive, mouse, touch })
    [bind, props] = useGesture({ passive, mouse, touch })
    <Gesture onAction passive mouse touch />
    @withGesture({ onAction, passive, mouse, touch })
      class extends React.Component {}
 */

If you have additional ideas or comments about the new api, please go ahead, i'm all ears.

@zvictor in the draft above it's similar to your PR. Though transient is out, onAction is enough to signal transient mode i guess. and passive is an option itself. can take these props as they are, and withGesture now wants options first and then the component. useGesture(eventHandler) is shortform for useGesture({ onAction, passive, etc })

@drcmda
Copy link
Member

drcmda commented Jan 4, 2019

Released it today: https://twitter.com/0xca0a/status/1081185260208078849

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

5 participants