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

Delayed onChange #114

Open
zlanich opened this issue May 19, 2019 · 5 comments
Open

Delayed onChange #114

zlanich opened this issue May 19, 2019 · 5 comments

Comments

@zlanich
Copy link

zlanich commented May 19, 2019

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
Copy link
Owner

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

@mikaello
Copy link
Collaborator

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
Copy link
Author

zlanich commented May 21, 2019

@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.

nmantulenko added a commit to nmantulenko/react-native-modal-selector that referenced this issue Jul 19, 2019
fixed 'onChange delayed' bug peacechen#114
added 'Alert does not work in SampleApp for iOS' bug peacechen#6
@manhthepixta
Copy link

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

@peacechen
Copy link
Owner

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.

PandaQQ pushed a commit to PandaQQ/react-native-modal-selector that referenced this issue Oct 18, 2019
added 'Alert does not work in SampleApp for iOS' bug peacechen#6
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

4 participants