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

Fixed issue #907 TypeError: Cannot read property 'dismiss' of undefined #929

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

herberthk
Copy link

@herberthk herberthk commented Sep 10, 2024

Summary

The error was described in this issue

  • What issues does the pull request solve? it solves this issue

Test Plan

What's required for testing? run the code below on android

                 <DateTimePicker
                        testID="dateTimePicker"
                        timeZoneOffsetInMinutes={0}
                        value={date}
                        mode={mode}
                        is24Hour={true}
                        display={Platform.OS === 'ios' ? 'inline' : 'default'}
                        onChange={onChange}
                        minimumDate={new Date()}
                    />

Compatibility

OS Implemented
iOS
Android

Checklist

  • 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

@@ -118,7 +118,7 @@ function open(props: AndroidNativeProps) {

function dismiss(mode: AndroidNativeProps['mode']): Promise<boolean> {
// $FlowFixMe - `AbstractComponent` [1] is not an instance type.
return pickers[mode].dismiss();
return pickers[mode]?.dismiss();
Copy link
Member

@vonovak vonovak Sep 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@herberthk hello and thanks for opening the PR.
I'm sure this fixes the error of "Cannot read property 'dismiss' of undefined" but I'd like to know first why pickers[mode] is undefined. It shouldn't be, as far as I can tell.
In the issue that you linked there's a lot of comments, but no one was able to provide a reproduction of the issue.

Right now, I see this more as a workaround rather than a solution. I'd like to know better what the underlying problem is in order to be able to assess if this is the right fix.

Can you provide more insight into what's happening?
Thank you

@herberthk
Copy link
Author

herberthk commented Sep 13, 2024

@vonovak Thanks for your response and contribution to this project. I'm not sure why pickers[mode] is undefined but this error came after migrating to react-native 0.75.2 in my case, I upgraded to the latest version of this package thinking it will resolve the issue which was not the case. The error comes after selecting or picking the date.

The error on the app looks like this
Screenshot_20240913_125231_Connect Up

The error in the terminal looks like this
Screenshot from 2024-09-13 12-54-25

This is my system information
Screenshot from 2024-09-13 13-02-20

I hope this helps
Thank you

@maiznadeem
Copy link

When can this be resolved? Or can I downgrade my version?

@herberthk
Copy link
Author

@maiznadeem The maintainer is not responding yet this PR fixes the issue

@vonovak
Copy link
Member

vonovak commented Oct 8, 2024

Hello,
I have responded in the review comment: #929 (comment)

Right now, I don't have the confidence that this fix is the correct fix. More investigation should be done to determine the cause of the issue, and then assess if this is the correct fix or not.

@sanchaz
Copy link

sanchaz commented Oct 16, 2024

@maiznadeem see #907 (comment)
The issue seems to be that when choosing 'datetime' mode on android, the picker for date still appears.
If I run only date and time (I know this is documented), it works fine.
So no fixed needed, but to avoid these comments/issues... maybe throwing without even loading the picker?

@maiznadeem
Copy link

@maiznadeem see #907 (comment)
The issue seems to be that when choosing 'datetime' mode on android, the picker for date still appears.
If I run only date and time (I know this is documented), it works fine.
So no fixed needed, but to avoid these comments/issues... maybe throwing without even loading the picker?

Just realised! thanks I might need to make my own datetime picker now.

@sanchaz
Copy link

sanchaz commented Oct 24, 2024

@maiznadeem see #907 (comment)
The issue seems to be that when choosing 'datetime' mode on android, the picker for date still appears.
If I run only date and time (I know this is documented), it works fine.
So no fixed needed, but to avoid these comments/issues... maybe throwing without even loading the picker?

Just realised! thanks I might need to make my own datetime picker now.

@maiznadeem the way it is usually done on android, if you look at google calendar for example, is that you show the date picker first and then the time picker as soon as the date picker returns. I would recommend following that pattern instead of rolling your own datetime picker.

@maiznadeem
Copy link

Just realised! thanks I might need to make my own datetime picker now.

@maiznadeem the way it is usually done on android, if you look at google calendar for example, is that you show the date picker first and then the time picker as soon as the date picker returns. I would recommend following that pattern instead of rolling your own datetime picker.

Haha, no I also meant searching for one like Tamagui or somewhere.

I could go with paper one, but it's such a pain, when handling validations, for example I want to select a date and time that is 3 hours from now. But the component returns me datetime values in UTC. I first need to extract time and date from each datetimes. And for example, in my current timezone, 3 hours from now might be the next day, but in UTC it's not, so validation also fails there.

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.

4 participants