Android datepicker mode configurations added #10932

pandiaraj44 · 2016-11-14T14:02:00Z

Motivation

Currently, The DatePickerAndroid component opens the native date picker with default mode. We can't able to change the mode via configurations. To support android date-picker mode, existing component needs to be changed.

Solution

For Android >= 5.0, The DatePickerDialog doesn't provide the default method to change the mode of a date picker. So I have added custom theme which will support two kinds of mode('spinner','calendar') ref:https://developer.android.com/reference/android/R.attr.html#datePickerStyle.

For Android < 5.0, The DatePickerDialog provides the default method to change the mode of a date picker. ref:https://developer.android.com/reference/android/widget/DatePicker.html#setCalendarViewShown(boolean).

With the help of Build.VERSION.SDK_INT I have done the above functionality with limited lines of code changes and also I have added the example to UIExplorer.

Test Plan

When opening a date picker on Android, We need to pass the option value(mode: 'calendar/spinner/default').

Example1:

DatePickerAndroid.open({ date: Date.now(), mode: 'calendar'}) // It will open datepicker with calendar mode

Android < 5.0

Android >= 5.0

Example2:
DatePickerAndroid.open({ date: Date.now(), mode: 'spinner'}) // It will open datepicker with spinner mode

Android < 5.0

Android >= 5.0

Example3:

DatePickerAndroid.open({ date: Date.now()})
(or)
DatePickerAndroid.open({ date: Date.now(), mode: 'default'})
// It will open datepicker with default native mode based on android versions.

Android < 5.0

Android >= 5.0

So If the user doesn't give mode option then default mode will be shown.

For real-time testing, you can run UIExplorer android project. I have added example code.

facebook-github-bot · 2016-11-14T14:02:35Z

By analyzing the blame information on this pull request, we identified @spicyj and @AaaChiuuu to be potential reviewers.

ericvicenti · 2016-11-14T15:48:23Z

This looks reasonable. Why is it tagged "[URGENT]"? Can you verify that the test failures are unrelated to this change?

pandiaraj44 · 2016-11-14T16:05:26Z

@ericvicenti removed the urgent tag from title. The build issues are related to Xcode8, I have changed only the DatePickerDialogModule for android.

ericvicenti · 2016-11-14T17:24:07Z

Ok, looks great!

@facebook-github-bot shipit

facebook-github-bot · 2016-11-14T17:24:25Z

@ericvicenti has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

ericvicenti · 2016-11-15T00:13:10Z

We had some test failures internally. Do you know why this would happen?

/data/react-native-github/ReactAndroid/src/main/java/com/facebook/react/modules/datepicker/DatePickerDialogFragment.java:28: error: cannot find symbol
import com.facebook.react.R;
                         ^
/data/react-native-github/ReactAndroid/src/main/java/com/facebook/react/modules/datepicker/DatePickerDialogFragment.java:63: error: package R does not exist
        dialog = new DismissableDatePickerDialog(activityContext, R.style.CalendarDatePickerDialog, onDateSetListener, year, month, day);
                                                                   ^
/data/react-native-github/ReactAndroid/src/main/java/com/facebook/react/modules/datepicker/DatePickerDialogFragment.java:65: error: package R does not exist
        dialog = new DismissableDatePickerDialog(activityContext, R.style.SpinnerDatePickerDialog, onDateSetListener, year, month, day);
stderr: /data/react-native-github/ReactAndroid/src/main/java/com/facebook/react/modules/datepicker/DatePickerDialogFragment.java:28: error: cannot find symbol

pandiaraj44 · 2016-11-15T01:54:31Z

@ericvicenti I have added custom styles in "react-native/ReactAndroid/src/main/res//shell/values/styles.xml". For reflecting those styles you have to run a clean build on android projects. You need to build the "ReactAndroid" android project.

ericvicenti · 2016-11-15T03:15:32Z

Hmm. Maybe @mkonicek can help land this, because I have no idea what I'm doing when it comes to best practices around these things.

facebook-github-bot · 2016-11-15T06:48:38Z

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

pandiaraj44 · 2016-11-15T09:19:29Z

@ericvicenti , @mkonicek I have run the "npm test" command inside react-native folder. The test cases are executed successfully.
FYI...

Can someone from android core team have a look at on this.

leeight · 2016-11-16T01:48:27Z

Try to run scripts/run-android-local-unit-tests.sh

pandiaraj44 · 2016-11-16T07:14:56Z

Hi @ericvicenti , @mkonicek , @facebook-github-bot , @leeight Finally, I have found the root cause of the issue("error: cannot find symbol import com.facebook.react.R;"). I have missed the resource dependency in date picker buck file. Now I have added the dependencies to "ReactAndroid/src/main/java/com/facebook/react/modules/datepicker/BUCK" file. I run the android unit test cases and it was passed and In an android integration test, date picker modal test cases were passed.

FYI..
sh scripts/run-android-local-unit-tests.sh- Result

sh scripts/run-android-local-integration-tests.sh - Result

I have re-base the code from master and updated the pull request.

sophiebits · 2016-11-16T17:38:08Z

@pandiaraj44 Please stop tagging everyone. It sounds like @mkonicek will look into why the import failed and tell you if anything needs to be changed.

ericvicenti · 2016-11-16T17:44:11Z

I am re-running internal tests and will land this afternoon if everything passes.

leeight · 2016-11-14T14:06:44Z

Libraries/Components/DatePickerAndroid/DatePickerAndroid.android.js

   *   * `date` (`Date` object or timestamp in milliseconds) - date to show by default
   *   * `minDate` (`Date` or timestamp in milliseconds) - minimum date that can be selected
   *   * `maxDate` (`Date` object or timestamp in milliseconds) - minimum date that can be selected
+   *   * `calendarMode` (`Boolean` true or false) - To set the date-picker mode to spinner/calendar.


Why not use enum type?

As per the android API documentation, Android date-picker supports only two modes. https://developer.android.com/reference/android/R.styleable.html#DatePicker_datePickerMode

leeight · 2016-11-17T03:18:53Z

ReactAndroid/src/main/java/com/facebook/react/modules/datepicker/BUCK

    react_native_target('java/com/facebook/react/bridge:bridge'),
    react_native_target('java/com/facebook/react/common:common'),
    react_native_target('java/com/facebook/react/module/annotations:annotations'),
+    react_native_target('res:shell'),


Actually i'm not sure it's right or not add res:shell dependency here, but i guess it's a bad smell...

Inside ReactAndroid project, The shell resource visibility is in PUBLIC mode. So I have reused that one and added some list of styles to shell resource folder.

I don't think it is necessary to create a resource folder for each of the package in ReactAndroid project.

Maybe if the visibility of the shell is in private mode then I would create separate resource folder.

Ref: https://developer.android.com/guide/topics/ui/themes.html

How about remove react_native_target('res:shell'), dependency and use context.getResources().getIdentifier instead of R...., for example:

cc @satya164

ReactAndroid/src/main/java/com/facebook/react/modules/i18nmanager/I18nManagerModule.java 44: final Locale locale = context.getResources().getConfiguration().locale; ReactAndroid/src/main/java/com/facebook/react/modules/statusbar/StatusBarModule.java 58: final int heightResId = context.getResources() 61: PixelUtil.toDIPFromPixel(context.getResources().getDimensionPixelSize(heightResId)) : ReactAndroid/src/main/java/com/facebook/react/uimanager/DisplayMetricsHolder.java 55: DisplayMetrics displayMetrics = context.getResources().getDisplayMetrics(); ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java 601: Resources resources = v.getContext().getResources(); ReactAndroid/src/main/java/com/facebook/react/views/imagehelper/ResourceDrawableIdHelper.java 54: int id = context.getResources().getIdentifier( 64: return resId > 0 ? context.getResources().getDrawable(resId) : null; ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageView.java 167: return new GenericDraweeHierarchyBuilder(context.getResources()) ReactAndroid/src/main/java/com/facebook/react/views/text/frescosupport/FrescoBasedReactTextInlineImageShadowNode.java 80: int resId = context.getResources().getIdentifier( 97: Resources resources = getThemedContext().getResources(); ReactAndroid/src/main/java/com/facebook/react/views/toolbar/ReactToolbar.java 301: return new GenericDraweeHierarchyBuilder(getResources()) 308: return getResources().getIdentifier( 317: return getResources().getDrawable(getDrawableResourceByName(name)); ReactAndroid/src/main/java/com/facebook/react/views/view/ReactDrawableHelper.java 41: int attrID = context.getResources().getIdentifier(attr, "attr", "android"); 49: return context.getResources() 52: return context.getResources().getDrawable(sResolveOutValue.resourceId); 72: color = context.getResources().getColor(sResolveOutValue.resourceId);

pandiaraj44 · 2016-11-17T17:31:58Z

Hi @ericvicenti , FYI... All git hub checks have passed.

pandiaraj44 · 2016-11-18T11:20:54Z

Hi @ericvicenti , Again I have re-base my code from upstream and tests have passed.

pandiaraj44 · 2016-11-22T04:27:41Z

cc @leeight

pandiaraj44 · 2016-11-22T08:57:30Z

cc @satya164 @ericvicenti @facebook-github-bot

satya164 · 2016-11-22T13:47:41Z

@pandiaraj44 Can you address @leeight 's nits regarding whitespace please?

pandiaraj44 · 2016-11-22T15:44:16Z

@satya164 Updated the pull request.
Can you please let me know who is the potential reviewer for this pull request and when they will accept/reject this pull request?

cc @leeight

satya164 · 2016-11-22T16:25:11Z

ReactAndroid/src/androidTest/assets/AndroidTestBundle.js.meta

@@ -0,0 +1 @@
+5	 4f:}&�		dl�8mH


Why is this file used?

@satya164 While running test cases that file get created. I removed that file and updated the pull request.

satya164 · 2016-11-22T16:27:34Z

Can you please let me know who is the potential reviewer

Me, @leeight and @mkonicek are reviewing your PR, so I guess we're the reviewers

when they will accept/reject this pull request?

We'll merge when all review comments have been addressed

pandiaraj44 · 2016-11-22T17:30:01Z

@satya164 Thank you very much for your information.

mkonicek · 2016-11-23T01:34:49Z

ReactAndroid/src/main/java/com/facebook/react/modules/datepicker/DatePickerDialogFragment.java


-    final DatePickerDialog dialog =
-        new DismissableDatePickerDialog(activityContext, onDateSetListener, year, month, day);
+    DatePickerMode mode = DatePickerMode.DEFAULT;  //To set mode to default


nit: Please remove comment that repeats what the code does without adding information.

Comment has been removed.

mkonicek · 2016-11-23T01:36:31Z

ReactAndroid/src/main/java/com/facebook/react/modules/datepicker/DatePickerDialogFragment.java

+            onDateSetListener, year, month, day);
+          break;
+        case DEFAULT:
+          //To Show a default native date picker


Please remove redundant comment.

Comment has been removed.

mkonicek · 2016-11-23T01:37:59Z

ReactAndroid/src/main/java/com/facebook/react/modules/datepicker/DatePickerDialogFragment.java

+          break;
+        case SPINNER:
+          dialog.getDatePicker().setCalendarViewShown(false);
+          break;


The code in this switch statement should be moved up to the branches where the dialogs are created.

mkonicek · 2016-11-23T01:41:35Z

ReactAndroid/src/main/java/com/facebook/react/modules/datepicker/DatePickerDialogFragment.java

+          break;
+      }
+    } else {
+      dialog = new DismissableDatePickerDialog(activityContext, onDateSetListener, year, month, day);


It's possible to write the code above without repeating this line twice:

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { if (mode == DatePickerMode.CALENDAR) { // Create picker with calendar } else (if mode == DatePickerMode.SPINNER) { // Create picker with spinner } } // Default case

setCalendarViewShown has been deprecated in latest android API version. So build version "if condition" is mandatory.

We can remove the dialog creation line but it requires extra two if condition

DatePickerDialog dialog = null; if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { switch (mode) { case CALENDAR: dialog = new DismissableDatePickerDialog(activityContext, activityContext.getResources().getIdentifier("CalendarDatePickerDialog", "style", activityContext.getPackageName()), onDateSetListener, year, month, day); break; case SPINNER: dialog = new DismissableDatePickerDialog(activityContext, activityContext.getResources().getIdentifier("SpinnerDatePickerDialog", "style", activityContext.getPackageName()), onDateSetListener, year, month, day); break; } } if (dialog == null) { dialog = new DismissableDatePickerDialog(activityContext, onDateSetListener, year, month, day); } if (Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) { switch (mode) { case CALENDAR: dialog.getDatePicker().setCalendarViewShown(true); dialog.getDatePicker().setSpinnersShown(false); break; case SPINNER: dialog.getDatePicker().setCalendarViewShown(false); break; } }

For supporting future cases, I have used a switch statement.

Makes sense, thanks!

mkonicek · 2016-11-23T01:42:33Z

ReactAndroid/src/main/java/com/facebook/react/modules/datepicker/DatePickerMode.java

+    mValue = value;
+  }
+
+  public static DatePickerMode fromString(String value) {


Would valueOf work instead?
http://stackoverflow.com/questions/604424/lookup-enum-by-string-value

ValueOf has been implemented. For that, we need to match the user values with enum constants.

mkonicek · 2016-11-23T01:44:32Z

ReactAndroid/src/main/res/shell/values/styles.xml

+
+    <style name="CalendarDatePickerStyle" parent="android:Widget.Material.Light.DatePicker" tools:targetApi="lollipop">
+        <item name="android:datePickerMode">calendar</item>
+    </style>


Is it needed for these styles to be defined in resources? Is it possible to set the android:datePickerMode programmatically instead?

Latest Android API doesn't provide the default methods to change the mode.
Ref: https://developer.android.com/reference/android/widget/DatePicker.html#attr_android:calendarViewShown ,
https://developer.android.com/reference/android/widget/DatePicker.html#attr_android:datePickerMode

mkonicek · 2016-11-23T11:51:23Z

Thanks!

@facebook-github-bot shipit

mkonicek · 2016-11-23T11:45:11Z

ReactAndroid/src/main/java/com/facebook/react/modules/datepicker/DatePickerDialogFragment.java

+          break;
+      }
+    } else {
+      dialog = new DismissableDatePickerDialog(activityContext, onDateSetListener, year, month, day);


Makes sense, thanks!

Summary: Currently, The **DatePickerAndroid** component opens the native date picker with default mode. We can't able to change the mode via configurations. To support android **[date-picker mode](https://developer.android.com/reference/android/widget/DatePicker.html)**, existing component needs to be changed. **For Android >= 5.0**, The DatePickerDialog doesn't provide the default method to change the mode of a date picker. So I have added custom theme which will support two kinds of **mode('spinner','calendar')** ref:https://developer.android.com/reference/android/R.attr.html#datePickerStyle. **For Android < 5.0,** The DatePickerDialog provides the default method to change the mode of a date picker. ref:https://developer.android.com/reference/android/widget/DatePicker.html#setCalendarViewShown(boolean). With the help of **Build.VERSION.SDK_INT** I have done the above functionality with limited lines of code changes and also I have added the example to UIExplorer. Closes facebook/react-native#10932 Differential Revision: D4176089 Pulled By: ericvicenti fbshipit-source-id: 7dfa9101214501ac2124bda7ee273a538f04e7cf

Summary: Currently, The **DatePickerAndroid** component opens the native date picker with default mode. We can't able to change the mode via configurations. To support android **[date-picker mode](https://developer.android.com/reference/android/widget/DatePicker.html)**, existing component needs to be changed. **For Android >= 5.0**, The DatePickerDialog doesn't provide the default method to change the mode of a date picker. So I have added custom theme which will support two kinds of **mode('spinner','calendar')** ref:https://developer.android.com/reference/android/R.attr.html#datePickerStyle. **For Android < 5.0,** The DatePickerDialog provides the default method to change the mode of a date picker. ref:https://developer.android.com/reference/android/widget/DatePicker.html#setCalendarViewShown(boolean). With the help of **Build.VERSION.SDK_INT** I have done the above functionality with limited lines of code changes and also I have added the example to UIExplorer. Closes facebook#10932 Differential Revision: D4176089 Pulled By: ericvicenti fbshipit-source-id: 7dfa9101214501ac2124bda7ee273a538f04e7cf

Summary: In the spirit of #10932, I added the `mode` option to the `TimePicker` Android API. There is only one mode available for **Android < 5**, the `spinner` one. If we are on **Android >= 5** we can choose between `spinner` or `clock`. If we specify `default` it will use the default of the current Android version. On **Android < 5**, whatever we choose it will be this: ![screen shot 2017-02-14 at 17 05 44](https://cloud.githubusercontent.com/assets/5436545/22937805/024ec67e-f2da-11e6-8b32-a680d9bc2247.png) On **Android >= 5**, with the `spinner` mode: ![screen shot 2017-02-14 at 16 51 17](https://cloud.githubusercontent.com/assets/5436545/22937803/024e0bbc-f2da-11e6-9f4b-26102ff2eeac.png) And with the `clock` mode, the default: ![screen shot 2017-02-14 at 16 51 02](https://cloud.githubusercontent.com/assets/5436545/22937804/024e64e0-f2da-11e6-9911-4135049f4726.png) Closes #12384 Differential Revision: D6006689 Pulled By: hramos fbshipit-source-id: fcd37c867c4061b9982b1687f2c10211e54df7cf

facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Nov 14, 2016

pandiaraj44 changed the title ~~Android datepicker mode configurations added [URGENT]~~ Android datepicker mode configurations added Nov 14, 2016

facebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Nov 14, 2016

facebook-github-bot added Import Failed and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Nov 15, 2016

pandiaraj44 force-pushed the master branch 2 times, most recently from a3cc751 to 7bcf865 Compare November 16, 2016 06:57

leeight reviewed Nov 17, 2016

View reviewed changes

pandiaraj44 force-pushed the master branch from 7bcf865 to 27e722f Compare November 17, 2016 14:06

pandiaraj44 force-pushed the master branch 2 times, most recently from c3e250d to aa514dc Compare November 18, 2016 08:53

pandiaraj44 force-pushed the master branch from f2e4244 to 1454da0 Compare November 22, 2016 06:07

pandiaraj44 force-pushed the master branch from 1454da0 to 69fb493 Compare November 22, 2016 15:35

satya164 reviewed Nov 22, 2016

View reviewed changes

pandiaraj44 force-pushed the master branch from 69fb493 to 34d736c Compare November 22, 2016 17:24

mkonicek suggested changes Nov 23, 2016

View reviewed changes

mkonicek added GH Review: needs-revision and removed GH Review: review-needed labels Nov 23, 2016

Android datepicker mode configurations added

fcc63b5

pandiaraj44 force-pushed the master branch from 34d736c to fcc63b5 Compare November 23, 2016 05:32

facebook-github-bot added GH Review: review-needed and removed GH Review: needs-revision labels Nov 23, 2016

mkonicek approved these changes Nov 23, 2016

View reviewed changes

facebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Nov 23, 2016

facebook-github-bot closed this in eaccd7e Nov 23, 2016

mkonicek mentioned this pull request Nov 23, 2016

Support of spinnerMode property to allow spinner datepicker on Android #10339

Closed

Kerumen mentioned this pull request Feb 14, 2017

[Android] Add TimePicker modes #12384

Closed

pandiaraj44 mentioned this pull request Jun 28, 2017

Not bring up datepicker pandiaraj44/react-native-datepicker-dialog#5

Closed

sidferreira mentioned this pull request Apr 10, 2018

[Android][Question] DatePicker / TimePicker without dialog #18783

Closed

3 tasks

Android datepicker mode configurations added #10932

Android datepicker mode configurations added #10932

Uh oh!

Conversation

pandiaraj44 commented Nov 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Solution

Test Plan

Example1:

Example3:

Uh oh!

facebook-github-bot commented Nov 14, 2016

Uh oh!

ericvicenti commented Nov 14, 2016

Uh oh!

pandiaraj44 commented Nov 14, 2016

Uh oh!

ericvicenti commented Nov 14, 2016

Uh oh!

facebook-github-bot commented Nov 14, 2016

Uh oh!

ericvicenti commented Nov 15, 2016

Uh oh!

pandiaraj44 commented Nov 15, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ericvicenti commented Nov 15, 2016

Uh oh!

facebook-github-bot commented Nov 15, 2016

Uh oh!

pandiaraj44 commented Nov 15, 2016

Uh oh!

leeight commented Nov 16, 2016

Uh oh!

pandiaraj44 commented Nov 16, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sophiebits commented Nov 16, 2016

Uh oh!

ericvicenti commented Nov 16, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pandiaraj44 Nov 17, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leeight Nov 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pandiaraj44 commented Nov 17, 2016

Uh oh!

pandiaraj44 commented Nov 18, 2016

Uh oh!

pandiaraj44 commented Nov 22, 2016

Uh oh!

pandiaraj44 commented Nov 22, 2016

Uh oh!

satya164 commented Nov 22, 2016

Uh oh!

pandiaraj44 commented Nov 22, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

satya164 commented Nov 22, 2016

Uh oh!

pandiaraj44 commented Nov 22, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pandiaraj44 commented Nov 14, 2016 •

edited

Loading

pandiaraj44 commented Nov 15, 2016 •

edited

Loading

pandiaraj44 commented Nov 16, 2016 •

edited

Loading

pandiaraj44 Nov 17, 2016 •

edited

Loading

leeight Nov 19, 2016 •

edited

Loading