search

wix / react-native-navigation

A complete native navigation solution for React Native
https://wix.github.io/react-native-navigation/
MIT License
13.04k stars 2.67k forks source link

TopBar button throttling #3110

Open guyca opened 6 years ago

guyca commented 6 years ago

The problem

Clicking on TopBar buttons multiple times, emits multiple click events which is undesirable. Ideally we would want the button to be disabled until the onClick logic (both native and in Js) completes.

Proposed solutions

  1. Time based throttling The naive approach is to throttle based on some arbitrary time frame, 300 ms for example. This approach is flawed due to the asynchronous nature of RN as some screens might take much longer to render then 300 ms.

  2. Throttling based on button props (options) #256 Again, this approach suffers from the same drawbacks, making it more of a workaround then a solution.

  3. Disable buttons until action completes

    • When declaring a button, we can assign it a navigationCommand (push, showModal, mergeOptions etc). Since commands return promises, we can disable the button until the promise is resolved. As a bonus, this method lets us avoid the round trip over the bridge when clicking buttons.

I'm opening the issue with the hope someone can propose a better solution, Until then I think we should go with option 3. This task hasn't been prioritised yet, but if you'd like to contribute please comment here or reach out on Discord.

sorodrigo commented 6 years ago

Happy to help! I just realised this is tagged as v2, will there be a v1 fix too?

guyca commented 6 years ago

Hey @sorodrigo 👋 We've stopped maintaining v1, so the sort answer is no. Meanwhile, If you're interested, you can migrate to v2 easily using this library.

sorodrigo commented 6 years ago

Didn't know about that lib, thanks! Will come back when we've migrated to v2 😛

pribeh commented 6 years ago

Sorry for the dumb question, but any update on this?

guyca commented 6 years ago

@pribeh This issue will be prioritized in the coming weeks. We're going to draw a roadmap for the upcoming quarter soon and this issue will be definitely be high priority.

pribeh commented 6 years ago

Wicked. We implemented a debounce which is working for now, but, as you pointed out, it's not ideal.

meznaric commented 5 years ago

Any updates?

zabojad commented 5 years ago

I've been using lodash.throttle directly on the navigationButtonPressed method and it works like a charm. Why did you decide to support throttling in rnn for the topbar buttons?

nitzandayzz commented 5 years ago

Any updates regarding this issue? Still happens on Android with RNNv2

gaguirre commented 5 years ago

I've experienced some issues related to this but just on Android. For instance, if you push a screen that has the back button in the top bar and during the animation you press that button, then the push is cancelled but the options are not reset. In iOS this is prevented disabling the back button during the animation.

I'd love to help on this, but don't know if v2 is still supported. @guyca

marcmoo commented 4 years ago

I created an HOC including a debounce function to stop multiple clicks for them, the issue still existing in V4 (4.8.1 what i am using now) - BTW, this is Android only problem

akemys commented 4 years ago

can you share your HOC code? @markmoomoo

keeprock commented 4 years ago

can you share your HOC code? @markmoomoo

https://medium.com/@devmrin/debouncing-touch-events-in-react-native-prevent-navigating-twice-or-more-times-when-button-is-90687e4a8113

khuonghieu commented 2 years ago

Any update on this? I have the exact same issue as @gaguirre mentioned above https://github.com/wix/react-native-navigation/issues/3110#issuecomment-515821442

I am using android, RNN version 7.26.0