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

We no longer allow a user to tap the confirm button unless picker = still. #40

Merged
merged 1 commit into from
May 11, 2017

Conversation

SudoPlz
Copy link
Contributor

@SudoPlz SudoPlz commented May 11, 2017

TL/DR:
We no longer allow our user to tap the confirm button unless the picker is still.

Long version:
Up until now when the user interacted with the picker, if someone tapped on the confirm button, the state was not yet changed and thus the picked value would be old and miss-leading.
The DatePickerIOS does not update on the fly, and before it even manages to dispatch an update, our component is unmounted and thus the state is lost.

We no longer allow our user to tap the confirm button unless the picker is still.

They can always tap the cancel button anyway.

@mmazzarolo mmazzarolo merged commit 988d166 into mmazzarolo:master May 11, 2017
@mmazzarolo
Copy link
Owner

I like it, thank you :)
Merged, I'll release it as soon as #39 gets merged.
Could you also provide a link to the issue on the react-native repo? I'd like to link it in the comments.

@SudoPlz
Copy link
Contributor Author

SudoPlz commented May 11, 2017

Not sure which one it was because I had a gazilion of tabs open, but it must be this one: facebook/react-native#8169

The problem with DatePickerIOS is that it doesn't expose many of it's parameters, so we have limited control over what it can do.

@mmazzarolo
Copy link
Owner

Landed on v4.6.0.
Let me know if it works correctly.
I expanded a bit the comment and I also noticed that userIsInteractingWithPicker could also be removed from the state (but I left it there anyway).

@SudoPlz
Copy link
Contributor Author

SudoPlz commented May 14, 2017

Thanks for that, but if you remove userIsInteractingWithPicker it will allow the confirm button to be pressed while the user is interacting with the picker and that's exactly what we're trying to avoid there..!
I'll try 4.6.0 on monday!

@mmazzarolo
Copy link
Owner

mmazzarolo commented May 14, 2017

@SudoPlz
I didn't explain myself properly, my bad: this.state.userIsInteractingWithPicker can be moved from the state to this.userIsInteractingWithPicker, because it is an info that is not used in the render.

edit: Ha! Just checked, it is used to compute the customConfirmButtonWhileInteractingIOS, it should be declared as part of the state (like it is right now) and not as a class variable then, my mistake :)

@SudoPlz
Copy link
Contributor Author

SudoPlz commented May 14, 2017

yep.
No problem!

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

Successfully merging this pull request may close these issues.

None yet

2 participants