feat(android): support setDatePickerDate #3790

mauricedoepke · 2022-12-29T04:05:48Z

Description

This pull request addresses the issue described here: (WIP) Support setDatePickerDate for android as well #3789

In this pull request, I have extended the setDatePickerDate action to work on android as well.

setDatePickerDate is already working for android in this pr and I also updated one relevant test.
I have yet to investigate whether more tests need changes.
I also have yet to update the docs.

Feel free to comment on how to improve this implementation.

For features/enhancements:

I have added/updated the relevant references in the documentation files.

For API changes:

I have made the necessary changes in the types index file.

noomorph · 2022-12-29T08:48:31Z

Looks good to me, at least at the first glance!

There is some missing unit test coverage, but I hope it is trivial enough to add it.

Could you post here a screenshot of the updated DatePicker screen?

If you need some help or have questions, let us know.

detox/index.d.ts

detox/src/android/AndroidExpect.test.js

docs/api/actions.md

detox/test/e2e/17.datePicker.test.js

detox/test/src/Screens/DatePickerScreen.js

noomorph · 2022-12-30T15:55:25Z

detox/test/e2e/17.datePicker.test.js

+
+      //rn-datepicker does not support testId's on android, so by.type is the only way to match the datepicker right now
+      //@see https://github.com/react-native-datetimepicker/datetimepicker#view-props-optional-ios-only
+      await element(by.type('android.widget.DatePicker')).setDatePickerDate('2019-02-06T05:10:00-08:00', "ISO8601");


I see an error like that on CI:
https://buildkite.com/wix-mobile-oss/detox/builds/1871#018563a8-6f16-438e-ac69-4f1785fe8ec3/6-4104

Weird. It worked on my machine. One possible explanation I can think of is, that detox's auto waiting might not work well together with how android updates the datepicker value.
This might cause the test to work on some systems, but not on others due to timing issues.

I will try to use a waitFor to verify this hypothesis.

…n ios

noomorph · 2023-01-03T17:40:12Z

@mauricedoepke sure I'll resume tomorrow.

asafkorem

@mauricedoepke Thank you for your contribution to the project! Your code will be a valuable addition. We appreciate your support and for taking the time to open a pull request.

detox/index.d.ts

detox/android/detox/src/full/java/com/wix/detox/espresso/DetoxAction.java

noomorph · 2023-01-06T17:39:55Z

detox/android/detox/src/full/java/com/wix/detox/espresso/DetoxAction.java

-            ZonedDateTime date = ZonedDateTime.parse(dateString);
-            return PickerActions.setDate(date.getYear(), date.getMonthValue(), date.getDayOfMonth());
-        } 
+            date = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ssZ").parse(dateString);


Does it work with custom time zones? +10:00 etc?

Oh, I see you changed it again . Probably this is not relevant.

Yes, the Z seems to support this case.
But now that I saw your commit that removes ms in the ios implementation, I think the android implementation does not support ms either. So maybe we can generalize removing the millis in a way that it does it for both ios and android?

I think the android implementation does not support ms either.

Well, it does not throw errors and it passes the tests. Do you think we need any extra action items in such case? 😕 ❓

mauricedoepke · 2023-01-06T19:56:43Z

detox/test/e2e/17.datePicker.test.js

+    test.each([
+      ['ISO8601', new Date(2019, 1, 6, 14, 10, 0).toISOString()],
+      ['yyyy/MM/dd HH:mm', '2019/02/06 14:10'],
+    ])('can select dates in %j format', async (formatString, dateString) => {
+      if (platform === 'ios') {
+        await element(by.id('datePicker')).setDatePickerDate(dateString, formatString);
+        await expect(element(by.id('localDateLabel'))).toHaveText('Date (Local): Feb 6th, 2019');
+        await expect(element(by.id('localTimeLabel'))).toHaveText('Time (Local): 2:10 PM');
+      } else {
+        await element(by.id('openDatePicker')).tap();
+        //rn-datepicker does not support testId's on android, so by.type is the only way to match the datepicker right now
+        //@see https://github.com/react-native-datetimepicker/datetimepicker#view-props-optional-ios-only
+        await element(by.type('android.widget.DatePicker')).setDatePickerDate('2019-02-06T05:10:00-08:00', 'ISO8601');
+        await element(by.text('OK')).tap();

-      if(failed === false) {
-        throw new Error('Test should have thrown an error, but did not');
+        await waitFor(element(by.id('utcDateLabel'))).toHaveText('Date (UTC): Feb 6th, 2019').withTimeout(3000);


@noomorph
This does not test the custom pattern feature on android, does it?

Also what I learned during my local testing, when I ran the tests between 00:00 and 01:00 is that I think you have to check against:

utc for iso datestrings or custom patterns that contain explicit timezones

local time for patterns that don't contain patterns.
Otherwise tests can fail due to timezone errors.

The reason is when:

using a iso string, you let the device parse the date with an explicit format. When you then display it in the local format and the local format differs from utc, you can have a day off error around midnight. When you hoever display it in utc, the results will be stable.

using a pattern without timezone you parse it in local time. When you now display it in utc, the result can vary when the device has a non utc timezone. So in this case the result will only be stable when you also output it as local time.

As this is quite complex, I think it is easier to remove the loop, even though the resulting code duplication is not so nice.

This was my suggestion, you may have not seen it because we worked on similar times on this:

it(':ios: can select dates on a UIDatePicker, format: ISO8601', async () => { await element(by.id('datePicker')).setDatePickerDate('2019-02-06T05:10:00-08:00', 'ISO8601'); await expect(element(by.id('utcDateLabel'))).toHaveText('Date (UTC): Feb 6th, 2019'); await expect(element(by.id('utcTimeLabel'))).toHaveText('Time (UTC): 1:10 PM'); }); it(':ios: can select dates on a UIDatePicker, format: yyyy/MM/dd HH:mm', async () => { await element(by.id('datePicker')).setDatePickerDate('2019/02/06 13:10', 'yyyy/MM/dd HH:mm'); await expect(element(by.id('localDateLabel'))).toHaveText('Date (Local): Feb 6th, 2019'); await expect(element(by.id('localTimeLabel'))).toHaveText('Time (Local): 1:10 PM'); }); it(':android: can select dates on a UIDatePicker, format: ISO8601', async () => { //rn-datepicker does not support testId's on android, so by.type is the only way to match the datepicker right now //@see https://github.com/react-native-datetimepicker/datetimepicker#view-props-optional-ios-only await element(by.type('android.widget.DatePicker')).setDatePickerDate('2019-02-06T05:10:00-08:00', 'ISO8601'); await element(by.text('OK')).tap(); await waitFor(element(by.id('utcDateLabel'))).toHaveText('Date (UTC): Feb 6th, 2019').withTimeout(3000); }); it(':android: can select dates on a UIDatePicker, format: yyyy/MM/dd', async () => { await element(by.type('android.widget.DatePicker')).setDatePickerDate('2019/02/06', 'yyyy/MM/dd'); await element(by.text('OK')).tap(); await waitFor(element(by.id('localDateLabel'))).toHaveText('Date (Local): Feb 6th, 2019').withTimeout(3000); });

mauricedoepke · 2023-01-06T20:06:49Z

@jonathanmos @asafkorem @noomorph

Can someone with deeper android knowledge have a look at this error?:
https://buildkite.com/wix-mobile-oss/detox/builds/1889#01858831-2d65-4c42-a6b1-8ecbace27c29

I have no idea what this means, how to debug this or if this is even related to this pr.

jonathanmos · 2023-01-11T14:21:09Z

@jonathanmos @asafkorem @noomorph

Can someone with deeper android knowledge have a look at this error?: https://buildkite.com/wix-mobile-oss/detox/builds/1889#01858831-2d65-4c42-a6b1-8ecbace27c29

I have no idea what this means, how to debug this or if this is even related to this pr.

As discussed on Discord, could you remove the two androidTestImplementations of espresso-contrib and com.google.truth from examples/demo-pure-native-android/app/build.gradle? This will fix the error on buildkite.

detox/test/e2e/17.datePicker.test.js

noomorph

So, from my side, I see only one remaining question:

How much work is left to add time support (hh:mm:ss) for Android? Maybe we could do it now and land the complete feature? 🙏

As for the actual issues, it looks like the simple date formatter cannot handle 2023-01-11T10:41:26Z for some reason, while it perfectly handles 2019-02-06T05:10:00-08:00. The test fails with Unparseable date: "2023-01-11T10:41:26Z". The build is red 🔴 .

noomorph · 2023-01-13T07:22:36Z

@jonathanmos, I'm removing myself as an assignee for now – all JavaScript-related comments from my side are resolved at the moment.

Let's come up with the final resolution for that Zoned/Simple date formatter, and maybe let's cover that logic as a unit test in Java/Kotlin. Interesting that 2023-01-11T10:41:26Z is parsed alright on my local machine, but it fails on CI. Maybe it is an API level issue, maybe not – let's locate it.

noomorph

Approving it all except for the Test Failed: Unparseable date: "2023-01-11T10:41:26Z" issue.

d4vidi · 2023-01-16T13:43:10Z

Hi, I will try to also get into the loop of things here, sometime this week.

d4vidi · 2023-01-29T20:27:50Z

I'm finally back into the loop of things - planning to high-prioritize this for the upcoming 2-week iteration.

@mauricedoepke It's starting to seem that by getting the Test Failed: Unparseable date: "2023-01-11T10:41:26Z" problem that @noomorph has mentioned you could be crossing the last mile for getting this thing merged ✅ 🍾
Do you think you could get around to it soon? I am likely to be here to help as needed, as I mentioned.

d4vidi · 2023-02-02T21:45:37Z

Well hopefully I've fixed the case of ISO-8601 with no time-zone. I'm nonetheless not 100% satisfied with the documentation, so I'm inclined to hold back the merge a bit longer till this gets sorted out too, even if all tests pass.

noomorph · 2023-02-03T06:38:44Z

detox/src/android/espressoapi/DetoxAction.js

@@ -233,6 +233,18 @@ class DetoxAction {
    };
  }

+  static parseDateISO8601(dateString) {


@d4vidi looks like we need to remove private methods generation, right?

No, we have to switch to invoke2 😄

d4vidi · 2023-02-05T17:50:34Z

Suggestion for a future rearchitecture: JS (node) should take on the single responsibility of converting the various formats to the unified time/date representation - which is in fact a unix time stamp. The JS will then deliver the various formats into the various native environments (android / iOS) in a unified way, and the native-side handling code will then be rendered super-simple.

d4vidi · 2023-02-06T09:01:56Z

Buildkite is green after a retry --> merging.

Maurice Döpke added 3 commits December 29, 2022 03:23

install community datepicker

be76d6e

add setDatePickerDate functionality for android

e0cbcb5

adjust date picker test to support android as well

d142ed1

mauricedoepke requested review from d4vidi, jonathanmos and noomorph as code owners December 29, 2022 04:05

Maurice Döpke added 5 commits December 29, 2022 14:22

add simple unit test for setDatePickerDate

c7a2b5f

add missing unit test case for setDatePickerDate

8216a44

use button to open android datepicker instead of implicit click on text

eaad43d

add setDatePickerDate working on android to docs

47908ac

improve documentation to reflect android changes

9326e1c

mauricedoepke changed the title ~~DRAFT: Add setDatePickerDate support to android~~ Add setDatePickerDate support to android Dec 29, 2022

noomorph reviewed Dec 29, 2022

View reviewed changes

detox/index.d.ts Outdated Show resolved Hide resolved

noomorph reviewed Dec 29, 2022

View reviewed changes

detox/src/android/AndroidExpect.test.js Outdated Show resolved Hide resolved

noomorph reviewed Dec 29, 2022

View reviewed changes

docs/api/actions.md Outdated Show resolved Hide resolved

remove correct usage wrong test expecting wrong usage

b168ea6

mauricedoepke mentioned this pull request Dec 30, 2022

Add testID support to android datepicker react-native-datetimepicker/datetimepicker#705

Merged

5 tasks

noomorph reviewed Dec 30, 2022

View reviewed changes

detox/test/e2e/17.datePicker.test.js Outdated Show resolved Hide resolved

noomorph added 2 commits December 30, 2022 13:42

docs(api): reword setDatePicker description

f7c8eec

chore: migrate iOS date picker

f74c7d0

noomorph mentioned this pull request Dec 30, 2022

test(unit): fix AndroidExpect #3791

Closed

3 tasks

noomorph reviewed Dec 30, 2022

View reviewed changes

detox/test/src/Screens/DatePickerScreen.js Outdated Show resolved Hide resolved

noomorph reviewed Dec 30, 2022

View reviewed changes

Maurice Döpke added 3 commits December 30, 2022 19:55

give iso examples first and add simple ios only example

a9b3145

unify datepickerscreen for android and ios

da63e6d

handle date parsing in java instead of js to use device timezone as o…

16ae8e0

…n ios

noomorph self-assigned this Jan 3, 2023

asafkorem requested changes Jan 3, 2023

View reviewed changes

detox/index.d.ts Outdated Show resolved Hide resolved

detox/android/detox/src/full/java/com/wix/detox/espresso/DetoxAction.java Outdated Show resolved Hide resolved

noomorph reviewed Jan 6, 2023

View reviewed changes

noomorph added 2 commits January 6, 2023 19:45

lint: fix error

10138f1

test(e2e): fix padding in the app

f62e6cd

mauricedoepke commented Jan 6, 2023

View reviewed changes

test(e2e): rewrite UTC and local assertions

d9cbb5b

noomorph added 2 commits January 12, 2023 16:09

test: add resilience on Android

4ab4f07

chore: cherry-pick @jonathanmos fixes

fc493bc

d4vidi reviewed Jan 12, 2023

View reviewed changes

detox/test/e2e/17.datePicker.test.js Outdated Show resolved Hide resolved

noomorph added 2 commits January 12, 2023 19:31

fix: trim milliseconds for Android too

e6fd633

test(e2e): remove statefulness from date picker test

c30bb5e

noomorph requested changes Jan 12, 2023

View reviewed changes

fix: action description

02f3e86

noomorph removed their assignment Jan 13, 2023

noomorph approved these changes Jan 13, 2023

View reviewed changes

noomorph changed the title ~~Add setDatePickerDate support to android~~ feat(android): support setDatePickerDate Jan 13, 2023

fix(android): Support non-TZ ISO8601 format

59129ff

noomorph reviewed Feb 3, 2023

View reviewed changes

docs: Improve types and docs related to setDatePickerDate()

76df062

d4vidi enabled auto-merge (squash) February 5, 2023 17:44

d4vidi disabled auto-merge February 6, 2023 09:02

d4vidi merged commit d34546a into wix:master Feb 6, 2023

d4vidi mentioned this pull request Feb 6, 2023

(WIP) Support setDatePickerDate for android as well #3789

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(android): support setDatePickerDate #3790

feat(android): support setDatePickerDate #3790

mauricedoepke commented Dec 29, 2022 •

edited

noomorph commented Dec 29, 2022 •

edited

noomorph Dec 30, 2022

mauricedoepke Dec 30, 2022 •

edited

noomorph commented Jan 3, 2023

asafkorem left a comment •

edited

noomorph Jan 6, 2023

noomorph Jan 6, 2023

mauricedoepke Jan 6, 2023

noomorph Jan 11, 2023

mauricedoepke Jan 6, 2023

mauricedoepke commented Jan 6, 2023

jonathanmos commented Jan 11, 2023

noomorph left a comment •

edited

noomorph commented Jan 13, 2023

noomorph left a comment

d4vidi commented Jan 16, 2023

d4vidi commented Jan 29, 2023 •

edited

d4vidi commented Feb 2, 2023 •

edited

noomorph Feb 3, 2023

d4vidi Feb 5, 2023

d4vidi commented Feb 5, 2023

d4vidi commented Feb 6, 2023

feat(android): support setDatePickerDate #3790

feat(android): support setDatePickerDate #3790

Conversation

mauricedoepke commented Dec 29, 2022 • edited

Description

noomorph commented Dec 29, 2022 • edited

Choose a reason for hiding this comment

mauricedoepke Dec 30, 2022 • edited

Choose a reason for hiding this comment

noomorph commented Jan 3, 2023

asafkorem left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mauricedoepke commented Jan 6, 2023

jonathanmos commented Jan 11, 2023

noomorph left a comment • edited

Choose a reason for hiding this comment

noomorph commented Jan 13, 2023

noomorph left a comment

Choose a reason for hiding this comment

d4vidi commented Jan 16, 2023

d4vidi commented Jan 29, 2023 • edited

d4vidi commented Feb 2, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d4vidi commented Feb 5, 2023

d4vidi commented Feb 6, 2023

mauricedoepke commented Dec 29, 2022 •

edited

noomorph commented Dec 29, 2022 •

edited

mauricedoepke Dec 30, 2022 •

edited

asafkorem left a comment •

edited

noomorph left a comment •

edited

d4vidi commented Jan 29, 2023 •

edited

d4vidi commented Feb 2, 2023 •

edited