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.

Sign up for GitHub

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

Jump to bottom

inconsistancy between onDateChange, onChange in ts and js. #611

Closed
punksta opened this issue Oct 22, 2021 · 6 comments · Fixed by #612
Closed

inconsistancy between onDateChange, onChange in ts and js. #611

punksta opened this issue Oct 22, 2021 · 6 comments · Fixed by #612
Labels
bug released

Comments

@punksta
Copy link
Contributor

punksta commented Oct 22, 2021

onDateChange is never get called on ios.

onChange type is the following:onChange?: (event: Event, date?: Date) => void;

https://github.com/react-native-datetimepicker/datetimepicker/blob/1fb6a2e1c4aca448172e240c4450eb1b48bf16f5/src/index.d.ts#L36

however this library is calling it like this:


 if (this.props.onChange) {
      this.props.onChange(date);
    }

react-native-modal-datetime-picker/src/DateTimePickerModal.ios.js

Line 95 in eebaaae

handleChange = (event, date) => {

Versions

"react-native-modal-datetime-picker": "12.0.0",

Description

Reproducible Demo

pass

  onChange={(event, date) => {
          console.log('onChange', event, date)
        }}

and see what happens on ios :)
@punksta punksta added the bug label Oct 22, 2021
@mmazzarolo
Copy link
Owner

👋

Yeah, we should definitely update the type definitions. Not sure if we want to expose the original event in the signature though.
May I ask you what you're using onChange for? (instead of onConfirm/onCancel)

@punksta
Copy link
Contributor Author

punksta commented Oct 22, 2021

@mmazzarolo thank you for you question. I am using it to react on date change asap. In my case submit button is not used. That is my UX team decision. User selects date and clicks outside of popup.

@punksta
Copy link
Contributor Author

punksta commented Oct 22, 2021

Perhaps you want to keep type definition of props in your project?
If types changes in the original project, typescript in your project can detect type mismatch.

@mmazzarolo
Copy link
Owner

@punksta not sure I'm following your suggestion 🤔

We're already extending the original project definitions here.
The problem is that we're using onChange in our own project, so we must define it in our typedefs since we're overriding it — it's just the typedefs that are outdated.

@mmazzarolo
Copy link
Owner

@punksta mind reviewing #612 and letting me know if it makes sense?

@mmazzarolo mmazzarolo mentioned this issue Oct 23, 2021
Merged
@github-actions
Copy link

🎉 This issue has been resolved in version 13.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug released
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix (types): onDateChange → onChange mmazzarolo/react-native-modal-datetime-picker
2 participants
@mmazzarolo @punksta