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

Double-tapping fires Actions twice #2228

Closed
abeltje1 opened this issue Aug 13, 2017 · 29 comments
Closed

Double-tapping fires Actions twice #2228

abeltje1 opened this issue Aug 13, 2017 · 29 comments

Comments

@abeltje1
Copy link

Version

Tell us which versions you are using:

  • react-native-router-flux v4.0.17 (v3 is not supported)
  • react-native v0.46.4

Expected behaviour

When an navigation action is fired through an onPress (pop or regular Actions.key) there is no possibility that the navigation action is fired twice. The first press should disable all other presses until the transition is done.

Actual behaviour

When you click a back button or another button to which you have bound a navigation action in rapid succession, the event gets fired twice. When I press the back button twice, I pop two scenes and when I press my own button with an onPress of Actions.key rapidly the scene gets mounted twice, which means I have to go back twice to my original scene.

Steps to reproduce

Really easy: just open the RNRF example and click twice like a madman. Sometimes the events fires once, sometimes twice.

@abeltje1
Copy link
Author

To further clarify this issue I made a GIF of the example project, @aksonov could you maybe take a look at this?

rnrf double click issue

@jorred
Copy link

jorred commented Aug 14, 2017

@abeltje1 did you find a way to fix this? I didn't even notice it before but after trying I noticed the same issue. Now I need to fix it because it's major UX bug.

@aksonov
Copy link
Owner

aksonov commented Aug 14, 2017

It could be react-navigation issue. I can't reproduce it with iOS 10 emulator.. Anyway, I've just added some changes, could you check latest master?

@abeltje1
Copy link
Author

@aksonov great thanks! I'll check it out in a few minutes, will let you know. You really can't reproduce? that's weird..I was using iOS 10 emulator too (XCode 8.3.3).

Cheers

@abeltje1
Copy link
Author

@aksonov I tried your latest commits but it didn't work :( Clicking in very rapid succession still opens a screen twice.

@aksonov
Copy link
Owner

aksonov commented Aug 14, 2017

Duplicate of react-navigation/react-navigation#135 ?

@aksonov
Copy link
Owner

aksonov commented Aug 14, 2017

I reproduced this issue - the problem was in refresh within componentDidMount of Login. It seems anti-pattern and should not be used - onEnter should be used instead.

@abeltje1
Copy link
Author

Hmm, I don't fully understand what you're trying to say. In my own app it happens on literally every button to which I have bound Actions.key. In the example it's also happening on every possible transition (as you can see in the GIF, it's not only on login but also on register). How can it be that it's because of the componentDidMount of login but also appears on other scenes?

I'm sorry if I don't understand it correctly, but I'm having a hard time figuring this out.

@aksonov
Copy link
Owner

aksonov commented Aug 14, 2017

Sorry, I was able to reproduce it only with Login. Maybe I have bad mouse (Apple one) that doesn't allow to click faster. Anyway, please check related issue, it seems problem of react-navigation.

@abeltje1
Copy link
Author

You're right, it's a react navigation issue. But what should we do now? wait for them until they fix it (with the current issue-handling state of React Navigation can take ages lol)? Or fix it inside RNRF? Or should all developers implement their own solutions until React Navigation fixes the issue?

What do you suggest?

Cheers

@aksonov
Copy link
Owner

aksonov commented Aug 14, 2017

I suggest that you will try suggested PR first. If it helps, you can just use forked react-navigation until they will merge it.

@abeltje1
Copy link
Author

will do, thanks for the help!

@wcmboy
Copy link

wcmboy commented Aug 16, 2017

I also have the problem

aksonov pushed a commit that referenced this issue Aug 18, 2017
@aksonov
Copy link
Owner

aksonov commented Aug 18, 2017

Please check latest master now.

@abeltje1
Copy link
Author

It works for all Actions except for pop (calling pop manually), so great progress!

@rogoja
Copy link

rogoja commented Aug 23, 2017

I still have the same problem

react-native-router-flux v4.0.0-beta.18
react-native v0.46.4

@aksonov
Copy link
Owner

aksonov commented Aug 23, 2017

@gtauniverse Read my comment above

@rogoja
Copy link

rogoja commented Aug 23, 2017

@aksonov Help. How to install the latest master?

@roots-ai
Copy link

roots-ai commented Dec 7, 2017

The issue still occurs with the latest release! How to handle this?

@antonsivogrivov
Copy link

@aksonov, I have the same issue here with rnrf 4.0.0-beta.27 and rn 0.52

@YoranRoels
Copy link

YoranRoels commented Feb 26, 2018

I've been keep tracking of this issue for a very long time and I noticed that recently react-navigation added support for idempotent navigation (react-navigation/react-navigation#3393), which would solve this problem. Is this version of react-navigation already supported in the latest react-native-router-flux beta?

@aksonov
Copy link
Owner

aksonov commented Feb 26, 2018

No. We are waiting until new 1.0 API will be stabilized

@YoranRoels
Copy link

YoranRoels commented Feb 26, 2018

Thanks for your reply, do you have a course of action to fix this problem until that time comes or an ETA for a stable 1.0?

@SofianeToumert
Copy link

SofianeToumert commented May 31, 2018

This is a workaround I use for the moment:

For every button that triggers a navigation action, you can wrap all of it process in a if condition like so:

if(Actions.currentScene == currentSceneKey) {

// Do your treatement here

}

Where currentSceneKey is the key of the scene from where the action is triggered.

This only triggers once since when you do it for the first time, it is still on the current scene, but for all the other extra taps you could perform, the stack scene is already updated (by adding the new scene you navigate to or by popping the current scene if you navigate back) therefore Actions.currentScene won't be equal to the current scene more than once.

For a cleaner code, I have created a structure that holds all my scene keys I use in Router.js and for this trick, It allows me to easily handle key scene renaming and adding new scenes:

import { sceneKeys } from './const/sceneKeys'

if(Actions.currentScene == sceneKeys.currentSceneKey) {

// Do your treatement here

}

It also work for the header buttons defined directly in the Router.js (I prefer defining them in their own scene trough static navigation).

Note: Works with tabbar scenes too, but be aware that the key generated by Actions.currentScenes for tabbar scenes is the key you specified for it in Router.js prefixed with '_' so be sure to add it in your currentSceneKey

Hope it will help you ;)

@hammooooody
Copy link

Probably the simplest workaround used for every Actions.xxx() instance

<Button onPress={() => Actions.currentScene !== "page1" ? Actions.page1() : {}} >Page 1</Button>

Enjoy

@e10a
Copy link

e10a commented Mar 6, 2019

@hammooooody this worked nicely for me.

Read through react-navigation/react-navigation#3393 but couldn't find any built-in way to get it to work - could have missed something obvious...(?)

I'm following your workaround by using a helper file to define a method that can be used globally.

Example Usage

import * as actions from './config/actions';

<TouchableOpacity
    onPress={() =>
        actions.push('venueItemView', { id, summary: this.props.venueData })
    }
>

actions.js (helper file)

import { Actions } from 'react-native-router-flux';

export const push = (destinationScene, props) => {
    // console.log('push', destinationScene, Actions.currentScene, props);
    if (Actions.currentScene === destinationScene) {
        return;
    }
    return Actions[destinationScene](props);
};

@hammooooody
Copy link

Lovely @e10a!

@nguyenvanphuc2203
Copy link

you save my life @e10a 👍 D

@syntax-e
Copy link

Is there any way this check can be built into RNRF by default?

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

No branches or pull requests