peacechen / react-native-modal-selector

A cross-platform (iOS / Android), selector/picker component for React Native that is highly customizable and supports sections.
MIT License
369 stars 129 forks source link

Delayed onChange #114

Open zlanich opened 5 years ago

zlanich commented 5 years ago

The onChange event currently doesn't fire until the closing animation is 100%, done if not later. This causes the UI to not be updated to the new value (at least when using a custom selector component), until about a full second later.

Would it be possible to fire the onChange event right away instead of after the animation?

I don't see a reason that would be problematic. Thoughts?

peacechen commented 5 years ago

That should be ok. Please submit a PR and we'll merge right away.

mikaello commented 5 years ago

I agree that conceptual this should be just fine. Just be aware of an issue regarding iOS and onChange while closing a modal. The way it is done today is to prevent this issue. I guess it is possible to do this in other ways, just remember to test on iOS with alert (e.g. the SampleApp). Related issues:

zlanich commented 5 years ago

@mikaello All good points. My one thought is this: Isn't the issue of an Alert interrupting the JS thread an issue in a million different ways? I feel like this issue would pop up in every project if you're firing off an Alert when an animation or other process may be running.

Is that even something worth sacrificing UI feedback/response to fix, something where the likely better solution would be to use a non-blocking Alert alternative? I feel like it's similar to alerts in a web browser. They're kind of lame to begin with, and most mature web apps don't use them anyway.

manhthepixta commented 4 years ago

I noticed the delay too. Could it be merged anyway?

peacechen commented 4 years ago

Hopefully this doesn't cause unintended side effects. If it does, we could add a prop to control the onChange behavior -- previous work-around or none.