Prevent navigating twice when clicking a button quickly

[nihgwu]

When we double-click(or multi-click) a link quickly, it will navigate to the same screen twice, so when we want to goBack, we need click the back button twice to go back to parent screen, that's insane.

I use ExperimentalNavigation before, and if I double-click a link quickly, the app crashes because we are pushing a route with the same key, which is not allowed by ExperimentalNavigation, you can try the UIExplorer to reproduce, so I make a check for it:

1. newKey == latestKey, ignore this action
2. routes.contains(newKey), jumpTo(newKey)

Now in react-navigation it won't crash because every time we dispatch a new action, we get a different key

So my propose:

1. don't generate key if provided in action when dispatching and check for existing
2. generate key according to route params, but I'm afraid that will be buggy because the screen relies on more params expect routeParams

Any thoughts on this?

[nihgwu]

export function routerReducer(state, action) {
  if (action.type.startsWith('Navigation/')) {
    const { type, routeName } = action
    const lastRoute = state.routes[state.routes.length - 1]
    if (type == lastRoute.type && routeName == lastRoute.routeName) return state
  }
  return AppNavigator.router.getStateForAction(action, state)
}

workaround for my case, I know we can custom nearly everything, but IMO this issue should be fixed in core

[satya164]

Another option is to just debounce the function you're dispatching from.

cc @ericvicenti

[nihgwu]

but debounce would make transition a bit laggy and we should let user to dispatch them own keys
dispatch(NavigationActions.navigate({routeName: 'Detail', key: 'user-123'}))

[grabbou]

Now in react-navigation it won't crash because every time we dispatch a new action, we get a different key

To me it's a feature. Imagine a screen, like userProfile that is generic and accepts user details to display as a JSON. It can be in the stack as many times as I want.

[nihgwu]

@grabbou But I don't think we should override the provided key, at least show a warn about overriding or key generation strategy

[satya164]

@nihgwu : but debounce would make transition a bit laggy

Maybe I'm missing something, but why would it?

cc @ericvicenti What do you think about this? This is has been a common issue I have faced in all of the apps I have worked on so far.

Mostly I used NavigationExperimental or ExNavigation. In case of former I did changes in reducer to not push same routes with same params twice

ExNavigation had a debouncing mechanism to prevent navigating twice in quick succession. cc @skevy

[nihgwu]

@satya164 my mistake, a debounce would be fine, but we should stop overriding custom keys

[arnsa]

@satya164 @nihgwu care to explain how can I implement this with a TabNavigator? Because state.routes[state.routes.length - 1] is always going to be last tab bar item for me.

[nihgwu]

I take the control of the default action flow of react-navigation, and apply a debounce, here is an example using dva:
https://github.com/nihgwu/react-native-dva-starter/blob/master/app/models/router.js#L24-L33

[joonhocho]

@nihgwu @satya164 @grabbou
I am facing the same issus, and I believe everyone who uses this library will eventually have to address this issue.

I created #768, but closed as duplicate as I found this thread.
Here's my original post and a possible idea to fix this.

---

Pressing on the navigation header right button quickly for multiple times causes navigating to the new route multiple times.

I think it should be easy to prevent this behavior as it can be very common behavior that we do not want.

Possible solutions would be

Add transitioning property to navigation prop so that header button can disable itself if there is another navigation transition taking in place.

navigationOptions: {
  headers: ({navigate, transitioning}) => ({
      right:
        <Button
          icon="ios-settings-outline"
          disabled={transitioning}
          onPress={() => navigate('Settings')}
        />,
    })
},

[satya164]

Yes, it's in the v1 roadmap #723

[joonhocho]

@satya164 What do you think about adding a new property, transitioning?
It can be useful when you want to disable multiple buttons as transition occurs.

[satya164]

A prop will cause a re-render which will make transition slower. You can use InteractionManager.runAfterInteractions instead

[evisong]

I guess a lodash.debounce() can solve this already?

[dzuncoi]

Currently I create a function to override navigate from addNavigationHelpers as below:

_addNavigationHelpers(navigation) {
    const original = addNavigationHelpers(navigation);
    let debounce;
    return {
      ...original,
      navigateWithDebounce: (routeName, params, action) => {
        let func = () => {
          clearTimeout(debounce);
          debounce = setTimeout(() => {
            navigation.dispatch(NavigationActions.navigate({
              routeName,
              params,
              action
            }));
          }, 200)
        }
        return func();
      }
    }
  }

And

<VocabTabs navigation={this._addNavigationHelpers({
            dispatch: this.props.dispatch,
            state: this.props.nav,
          })}/>

Then you can use this.props.navigation.navigateWithDebounce instead of this.props.navigation.navigate

This, of course is a temporary approach. It'd better to have a life-cycle hook to detect when screen is in transition progress then prevent pushing another route.

[microwavesafe]

I used your method to good effect. I changed it around slightly because I needed long delays (page transitions can be quite slow) and I didn't want to delay the navigation.

    _addNavigationHelpers = (navigation) => {
        const original = addNavigationHelpers(navigation);
        let debounce;
        return {
            ...original,
            navigateWithDebounce: (routeName, params, action) => {
                let func = () => {
                    if (debounce) {
                        return;
                    }

                    navigation.dispatch(NavigationActions.navigate({
                        routeName,
                        params,
                        action
                    }));

                    debounce = setTimeout(() => {
                        debounce = 0;
                    }, 1000)
                };
                return func();
            }
        }
    };

[dzuncoi]

Nice approach @microwavesafe 👍

[cosivox]

@microwavesafe Where do I need to put that to get it working?

[dzuncoi]

@cosivox wherever you use addNavigationHelpers, replace it with the this one.

[cosivox]

@dzuncoi Do you mean inside library files? Because I don't use addNavigationHelpers anywhere in my code

[MacKentoch]

Why is it closed?

This behavior is clearly a 🐛 for a navigation library.

[Dror-Bar]

The solution @Doko-Demo-Doa suggested (where he edits the lib) doesn't seem to work for me.

Does anyone know of an updated solution to edit the lib and just add a delay / debounce for like 500 ms every time I use navigate?

Also, why was this closed if it's still an issue? or is this considered a clicking issue and not a react-navigation issue?

[wellyshen]

Any progress for this issue officially?

[ivosabev]

+1 for an official solution

[ryanab]

I am having the same issue, but was playing around on a production applications from very large unnamed company that I believe uses React Native (Think Multiple Billion in Market Cap), and was able to reproduce the issue on there (and from my understanding they are using a different nav library); so I do not think the issue is specific to solely react-navigation, and may be more broad common than we think across various libs.

However that being said I would love an official solution, but in the interim I will post what I end up deciding on for anyone's future reference, but I think most of the redux solutions above look pretty good (I am not a fan of the timers personally but that is just personal opinion I am sure it works great).

I am looking to see if I can do anything with the onTransitionStart onTransitionEnd functions for those of us who are not currently routing all our navigation actions through redux, but ultimately this looks like the redux solution may be best option.

[AugustoAleGon]

@phumaster This solution works!

[LeonidVeremchuk]

OMG. So much workaround code ...
It is necessary to add something like :

launchMode = "singleInstance"

[ericvicenti]

Hey folks, we just added a new feature: If you provide a key to the navigate action, you will always navigate to the same screen. So, if you use the following new API:

this.props.navigation.navigate({ key: 'MyScreen1', routeName: 'ProfileScreen', params: {...}})

You can call this action as many times as you like, and only one screen will be pushed! This is because the second time the action fires, it basically means "go to the existing route with key equal to 'MyScreen1'"

You can test this on the master branch, or wait for the upcoming release of beta 28

[Palisand]

Is this what we've all been waiting for?!

[ryanab]

@ericvicenti you are the man. What NPM version is it?

Edit: Ignore the last question; got too excited and didn't read to the end haha.

[ericvicenti]

Its not on NPM yet, but we will get it released soon! If you want to test it out, you can point your package.json to github:

"react-navigation": "git://github.com/react-navigation/react-navigation.git#master"

[ryanab]

@ericvicenti confirmed to be working just tested beta 27 vs git master and it works!

Just a note to others reading (and please do not confuse this as a complaint I am very pleased this has just made my weekend much better 👍 ) but you can still fire off the navigation action twice if you press very fast. However the good news is with this fix only one is actually on the stack. If you are bothered by the double animation (and you have to press extremely fast its pretty tough) you may need to use a debounce etc. as discussed by some people above.

Thanks again to everyone on react navigation team for this.

[ericvicenti]

Glad it works for you now!

If somebody wants to fix the double-tap behavior, maybe we could look into blocking the whole screen's touch events during a transition. Right now I think we block events to the screen, but the header still accepts touches

[nihgwu]

1 year passed 😅

[AlessandroAnnini]

yes @nihgwu 1 year passed, meanwhile weeks ago i lost my job because of the bugs in the app and the slowness in the fixing... the navigation was quite complex and guess what library i was using?
This library is TOXIC, heavy-pollution for the react environment..

[ericvicenti]

I'm really sorry to hear that @sun2rise.. open source isn't easy 😞 Maybe you can help us de-toxify the library? What were the biggest issues for your app, aside from this one?

[ryanab]

I think it is a little unfair to blame a library which is open-source/free. There are issues but it is a constantly changing and young project working with React Native which itself is still a very young and constabtly changing project, and should save you considerable time from rolling your own navigator. That being said I have not done much to contribute and probably should; Eric if i were to look at the double tap issue where would be a good starting point / jumping off point.

[ericvicenti]

@ryanab, thanks for offering! In this case, I'd love to see an exploration of the core problem, and post a fresh issue with your findings. Here are the questions I have:

• Are these double-taps happening on screens, or in the header?
• There is something in StackNavigator that is supposed to block touches on the last screen once transition starts (PointerEventsContainer). Is this broken, or does it not happen fast enough?
• If the double-tap is in the header, can we use a similar technique to block pointer events during the transition?
• Maybe the button should be configured to only fire one event, even if it gets a double tap? Explore this approach, and discuss the drawbacks.
• Are there any cases where two quick taps really should result in two navigations? Maybe a "next" button? How do we handle those cases, if at all?
• Maybe its ok for there to be two events, if we ignore the second one. This is the motivation for the new key field on navigate, which allows for an idempotent navigation. Maybe this could be improved to address the problem.

[Dror-Bar]

ericvicenti -
Thank you very much for this new feature, I think it is one of the most requested features.
Double-tapping quickly is sort of a general issue, which is amplified when dealing with navigations. Having a solution for this is highly appreciated.
That being said, is there any estimation for the release of beta 28?

[fnicastri]

@sun2rise @ryanab @ericvicenti Sure it's unfair but the way this project is managed is not good.
Many issues are arbitrary closed without a solution, many are unresponded, many bugs are fixed after long time (like this one) or ignored, features are dropped almost without notice and before any replacement is ready (like the lazy loading in tabs).
Thi project is huge and complex, I get that, but many developers are thinking to change navigation library because of this instead of contributing to the project. @

[MacKentoch]

@fnicastri I clearly understand you frustration. I have multiple times little troubles that get me nerve. As you said there is more than a coding issue right now, it is more about communication and visibility.

But please, keep in mind that people behind this take of their own time to try to make it better.

Wether we like or not this navigator we have to admit how hard work it is to make it working as anyone would like.

And don't forget how fast Facebook dropped old school but working Navigator in favor of community solution that were not ready for production.

I don't have time contribute to this navigator but I can at least say thank you everyone maintaining and making it grow every day.

[fnicastri]

@MacKentoch It's the attitude.
People work hard, but if you ignore serious bugs or quickly close real issues because they are not using the model or are not super clear, release new version without changenotes or even a tag...
you pass the wrong message and potential contributors fly away.

[MacKentoch]

@fnicastri definitely agree

[ericvicenti]

@fnicastri, @MacKentoch, thanks for the feedback! We're getting back up to speed, and tying to improve our practices here.

There are a ton of issues that get reported on this repo- closing the ones that don't make sense is our only way of keeping our sanity and focusing on the important problems.

ignore serious bugs

What serious bugs do you see that are going ignored? We are trying to prioritize the most important and most serious bugs. Please help us by bringing them to our attention- especially when the issue is clearly written and something badly broken.

With regard to versioning, sorry about not documenting the changes very well. One problem is that we are stuck in this "beta" state where we cannot even respect semver, so we will hopefully release 1.0 soon, even if its not perfect, so we can start documenting major/minor/patch releases properly.

Again, your feedback is appreciated, and we're trying to improve the way things like this are handled, so keep on telling Brent and me how to improve!

[ryanab]

@ericvicenti will take a look at this most likely this weekend and open a new issue, but for now:

Are these double-taps happening on screens, or in the header?
There is something in StackNavigator that is supposed to block touches on the last screen once transition starts (PointerEventsContainer). Is this broken, or does it not happen fast enough?
If the double-tap is in the header, can we use a similar technique to block pointer events during the transition?

In the screen actually - will do some testing and try to find out if event is firing off before transition starts or if perhaps there is a bug (maybe something with nested navigators?).

Maybe the button should be configured to only fire one event, even if it gets a double tap? Explore this approach, and discuss the drawbacks.
Are there any cases where two quick taps really should result in two navigations? Maybe a "next" button? How do we handle those cases, if at all?

Maybe its ok for there to be two events, if we ignore the second one. This is the motivation for the new key field on navigate, which allows for an idempotent navigation. Maybe this could be improved to address the problem.

If we are explicitly declaring our routes in a screen (i.e. not a next button in a stack), I do not really see any reason why we would want to ever allow a double tap, but perhaps there needs to be some type of setting in navigationOptions just in case?

Or, perhaps the answer is in the new key field navigate as you say. Maybe if you are explicitly navigating using a key the second matching one should not only not get added to the stack as it does now with the new update, but we prevent the second transition from starting when the event is fired (again from your first question it seems like this is the intended default behavior).

Anyway, will open a new issue at some point soon but wanted to leave my thoughts here in the interim in case anyone else is having similar issues they can track as well.

[gregblass]

EDIT: Looks like this was addressed! But maybe still possible if you really try to do it fast? I'll give it a test myself.

[pselibas]

Hi,
Was this issue not fixed in version 1.0.x? I was pretty sure I saw this on the roadmap, but I guess I was mistaken.

While I have the utmost respect and appreciation for all the hard work and progress that has been made on this project, this is the one bug that I would consider a "show stopper" when making or promoting a quality react native app.

Are there any non redux workarounds not involving a timer of some sort?
Keep up the excellent work and know that we are cheering you guys on.

[brentvatne]

react-navigation/rfcs#16 for discussion, let's turn something into a rfc. notice that we suggest idempotent pushes currently. would be nice to handle automatically. let's discuss there.