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

Fix ios countdown #312

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

aceiii
Copy link

@aceiii aceiii commented Oct 17, 2020

Summary

Fixes #30

The fix requires calling the setDate twice:

  • the first time with a date that is different than what it will end up at
  • the second time with the animated flag enabled and a copy of the final date

I've tried with only a single call or different not generating a copy of the date, but was only able to resolve it with this combination. These steps are wrapped in the method resetDate which gets called from willMoveToSuperview (the view may be displayed, so we're going to perform this resetDate here) and after the onChange handler is called and if we're using the countdown mode.

Test Plan

What's required for testing (prerequisites)?

A DateTimePicker component with 'countdown' mode running on an ios device or simulator:

<DateTimePicker
    mode={'countdown' as any}
    onChange={(event, date) => console.log('Updated!')}
    value={new Date(0, 0, 0, hours, minutes, seconds)}
/>

What are the steps to reproduce (after prerequisites)?

Bug 1

  • Add logging to onChange handler to produce visible output
  • Load up component in App
  • Select the first value (notice onChange hasn't been triggered)
  • Select another value (onChange will be triggered)

Bug 2

  • Continuing from above
  • Select 0 Hours, 0 Minutes (slider will automatically change to 1 minute and onChange handler should be triggered)
  • Select another value other than 0 (notice onChange handler doesn't get triggered)
  • Select a third value other than 0 (onChange will be triggered once again)

Compatibility

OS Implemented
iOS

This is an ios only bug. It shouldn't affect the existing android implementation.

Checklist

  • [ x] I have tested this on a device and a simulator
  • I added the documentation in README.md
  • I updated the typed files (TS and Flow)
  • I added a sample use of the API in the example project (example/App.js)
  • I have added automated tests, either in JS or e2e tests, as applicable

ios/RNDateTimePicker.m Outdated Show resolved Hide resolved
Only call resetDate when datePickerMode is UIDatePickerModeCountDownTimer

Co-authored-by: Vojtech Novak <[email protected]>
@jsheffers
Copy link

jsheffers commented Nov 22, 2020

I've tested this code and it does seem to fix the issue mentioned here, but another arises. The date defaults to 0 hours 1 minute on IOS so the date is invalid if no change is fired (no valid date on initialization)

@vonovak
Copy link
Member

vonovak commented Dec 4, 2020

hello @aceiii / @jsheffers would either one of you be able to address the issue found in the comment ^ and fix the PR or open another one? Thank you!

@aceiii
Copy link
Author

aceiii commented Dec 13, 2020

I'm trying to that other issue, but fixing that causes the other bug to pop up again. I'm stumped at the moment.

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.

iOS DateTimePicker in countdown mode fails to call onChange on first update
3 participants