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 Android timezone issues #431

Conversation

anton-patrushev
Copy link

Summary

I have find out issue while using datetime picker for RN App with timezone handling.
I need to use fixed timezone through whole app. As I found out, from last releases datetime picker should not be device timezone dependent if you've passed timeZoneOffsetInMinutes prop into DateTimePicker component (as the docs says). iOS works really well, but as it turns out, Android is not so good. (To be correct it doesn't apply timezone in some cases at all 😄). This proposal should fix that issue for Android. (At least it works for us). (I will describe steps to reproduce and technical source of the issue below)

  • It's impact Android only
  • It's related to timeZoneOffsetInMinutes feature

Test Plan

Display initially passed date (input)

  • (timezones: device: GMT +3, app: GMT -7)
  • pass 2021-04-09T17:00:00.000Z date (for UTC it stands for 17:00, but for app timezone -7 it should be treated like 10:00)
  • pass timeZoneOffsetInMinutes={-420} prop (-420 = -7 * 60)
  • Expected: TimePicker should display 10:00 as initial selected date
  • Actual: TimePicker displays 17:00 as initial selected date
    According to this fact we can suppose what timeZoneOffsetInMinutes prop is ignored and UTC hours & minutes are displayed instead.

What's required for testing (prerequisites)?

  • App uses fixed timezone for date handling/management (for example GMT -7)
  • Device timezone should differ from app timezone (device: GMT +3, app: GMT -7)
  • Android 🙂

What are the steps to reproduce (after prerequisites)?

Compatibility

OS Affected
iOS
Android

Changes Motivation

RNDate.java

  • I have changed Custom Date initialization logic (set date in milliseconds at first, and only after that apply timezone)
  • For some unknown reasons timeZoneOffsetInMinutes type acts randomly. In one case it's Integer type, in other case it's Long type. So I applied Long Java type compatibility (fallback for getInt).

datetimepicker.android.js

  • I have refactored if statement for timeZoneOffsetDateSetter function which is called for time settings after promise from native Android method is resolved. I really don't get any reasons for ignoring negative timeZoneOffsetInMinutes value. (even docs showing example with GMT -7 = - 420 minutes)

Checklist

  • I have tested this on my Android device and a emulator from Android Studio

@vonovak
Copy link
Member

vonovak commented Apr 9, 2021

hello and thank you for investigating this. There is an issue with e2e tests failing:

Expected: (with text: is "22:30" and view has effective visibility=VISIBLE)

Got: "ReactTextView{ text=23:30 ...}

Can you please look into that? Thank you! :)

@anton-patrushev
Copy link
Author

hello and thank you for investigating this. There is an issue with e2e tests failing:

Expected: (with text: is "22:30" and view has effective visibility=VISIBLE)

Got: "ReactTextView{ text=23:30 ...}

Can you please look into that? Thank you! :)

I will look into this in the near future.
Maybe my fix is not perfect. I need to see e2e log and expected test results. Anyway, it works for us :)
I will back with answer!

@lukebrandonfarrell
Copy link

Any update on this? @anton-patrushev

@anton-patrushev
Copy link
Author

I would close it, since @manojmalik20 move this changes into another PR and fix E2E test. Thanks @manojmalik20 for helping!

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.

3 participants