fix: dismiss android datepicker dialog upon unmount (#337)

vonovak · aleksanb · stianjensen · web-flow · commit 62abaffa2e56 · 2020-11-17T10:26:34.000-03:00
* Dismiss android pickers on react component unmount

This commit makes the behavior of the android datetimepickers more in
line with other controlled components in react, where unmounting the
component actually dismisses the ui widget as well.

Co-authored-by: Stian Jensen &lt;me@stianj.com&gt;

* remove duplication, rename close() to dismiss()

* cover android dialog dismiss by e2e test

* fix lint

* fix e2e test

Co-authored-by: Aleksander Vognild Burkow &lt;aleksanderburkow@gmail.com&gt;
Co-authored-by: Stian Jensen &lt;me@stianj.com&gt;
diff --git a/android/src/main/java/com/reactcommunity/rndatetimepicker/Common.java b/android/src/main/java/com/reactcommunity/rndatetimepicker/Common.java
@@ -0,0 +1,29 @@
+package com.reactcommunity.rndatetimepicker;
+
+import androidx.fragment.app.DialogFragment;
+import androidx.fragment.app.FragmentActivity;
+import androidx.fragment.app.FragmentManager;
+
+import com.facebook.react.bridge.Promise;
+
+public class Common {
+
+  public static void dismissDialog(FragmentActivity activity, String fragmentTag, Promise promise) {
+    if (activity == null) {
+      promise.reject(
+              RNConstants.ERROR_NO_ACTIVITY,
+              "Tried to close a " + fragmentTag + " dialog while not attached to an Activity");
+      return;
+    }
+
+    FragmentManager fragmentManager = activity.getSupportFragmentManager();
+    final DialogFragment oldFragment = (DialogFragment) fragmentManager.findFragmentByTag(fragmentTag);
+
+    boolean fragmentFound = oldFragment != null;
+    if (fragmentFound) {
+      oldFragment.dismiss();
+    }
+
+    promise.resolve(fragmentFound);
+  }
+}
diff --git a/android/src/main/java/com/reactcommunity/rndatetimepicker/RNDatePickerDialogModule.java b/android/src/main/java/com/reactcommunity/rndatetimepicker/RNDatePickerDialogModule.java
@@ -22,6 +22,8 @@
 import com.facebook.react.common.annotations.VisibleForTesting;
 import com.facebook.react.module.annotations.ReactModule;
 
+import static com.reactcommunity.rndatetimepicker.Common.dismissDialog;
+
 /**
  * {@link NativeModule} that allows JS to show a native date picker dialog and get called back when
  * the user selects a date.
@@ -84,6 +86,11 @@ public void onClick(DialogInterface dialog, int which) {
     }
   }
 
+  @ReactMethod
+  public void dismiss(Promise promise) {
+    FragmentActivity activity = (FragmentActivity) getCurrentActivity();
+    dismissDialog(activity, FRAGMENT_TAG, promise);
+  }
   /**
    * Show a date picker dialog.
    *
diff --git a/android/src/main/java/com/reactcommunity/rndatetimepicker/RNTimePickerDialogModule.java b/android/src/main/java/com/reactcommunity/rndatetimepicker/RNTimePickerDialogModule.java
@@ -23,6 +23,8 @@
 import androidx.fragment.app.FragmentActivity;
 import androidx.fragment.app.FragmentManager;
 
+import static com.reactcommunity.rndatetimepicker.Common.dismissDialog;
+
 /**
  * {@link NativeModule} that allows JS to show a native time picker dialog and get called back when
  * the user selects a time.
@@ -83,6 +85,12 @@ public void onClick(DialogInterface dialog, int which) {
     }
   }
 
+  @ReactMethod
+  public void dismiss(Promise promise) {
+    FragmentActivity activity = (FragmentActivity) getCurrentActivity();
+    dismissDialog(activity, FRAGMENT_TAG, promise);
+  }
+
   @ReactMethod
   public void open(@Nullable final ReadableMap options, Promise promise) {
 
diff --git a/example/App.js b/example/App.js
@@ -180,6 +180,25 @@ export const App = () => {
                 testID="neutralButtonLabelTextInput"
               />
             </View>
+
+            <View style={styles.header}>
+              <ThemedText style={{margin: 10, flex: 1}}>
+                [android] show and dismiss picker after 3 secs
+              </ThemedText>
+            </View>
+            <View style={styles.button}>
+              <Button
+                testID="showAndDismissPickerButton"
+                onPress={() => {
+                  setShow(true);
+                  setTimeout(() => {
+                    setShow(false);
+                  }, 3000);
+                }}
+                title="Show and dismiss picker!"
+              />
+            </View>
+
             <View style={styles.button}>
               <Button
                 testID="showPickerButton"
diff --git a/example/e2e/detoxTest.spec.js b/example/e2e/detoxTest.spec.js
@@ -3,14 +3,16 @@ const {
   getDateText,
   elementById,
   elementByText,
+  getDateTimePickerIOS,
+  getDatePickerAndroid,
 } = require('./utils/matchers');
 const {
   userChangesMinuteValue,
   userOpensPicker,
   userTapsCancelButtonAndroid,
   userTapsOkButtonAndroid,
 } = require('./utils/actions');
-const {isAndroid, isIOS} = require('./utils/utils');
+const {isAndroid, isIOS, wait} = require('./utils/utils');
 
 describe('Example', () => {
   beforeEach(async () => {
@@ -35,21 +37,17 @@ describe('Example', () => {
     await userOpensPicker({mode: 'date', display: 'default'});
 
     if (isIOS()) {
-      await expect(
-        element(by.type('UIPickerView').withAncestor(by.id('dateTimePicker'))),
-      ).toBeVisible();
+      await expect(getDateTimePickerIOS()).toBeVisible();
     } else {
-      await expect(element(by.type('android.widget.DatePicker'))).toBeVisible();
+      await expect(getDatePickerAndroid()).toBeVisible();
     }
   });
 
   it('nothing should happen if date does not change', async () => {
     await userOpensPicker({mode: 'date', display: 'default'});
 
     if (isIOS()) {
-      await expect(
-        element(by.type('UIPickerView').withAncestor(by.id('dateTimePicker'))),
-      ).toBeVisible();
+      await expect(getDateTimePickerIOS()).toBeVisible();
     } else {
       const testElement = element(
         by
@@ -70,9 +68,7 @@ describe('Example', () => {
     const dateText = getDateText();
 
     if (isIOS()) {
-      const testElement = element(
-        by.type('UIPickerView').withAncestor(by.id('dateTimePicker')),
-      );
+      const testElement = getDateTimePickerIOS();
       await testElement.setColumnToValue(0, 'November');
       await testElement.setColumnToValue(1, '3');
       await testElement.setColumnToValue(2, '1800');
@@ -96,9 +92,7 @@ describe('Example', () => {
     await userOpensPicker({mode: 'time', display: 'default'});
 
     if (isIOS()) {
-      await expect(
-        element(by.type('UIPickerView').withAncestor(by.id('dateTimePicker'))),
-      ).toBeVisible();
+      await expect(getDateTimePickerIOS()).toBeVisible();
     } else {
       await expect(element(by.type('android.widget.TimePicker'))).toBeVisible();
     }
@@ -108,9 +102,7 @@ describe('Example', () => {
     await userOpensPicker({mode: 'time', display: 'default'});
 
     if (isIOS()) {
-      await expect(
-        element(by.type('UIPickerView').withAncestor(by.id('dateTimePicker'))),
-      ).toBeVisible();
+      await expect(getDateTimePickerIOS()).toBeVisible();
     } else {
       await userChangesMinuteValue();
       await userTapsCancelButtonAndroid();
@@ -124,9 +116,7 @@ describe('Example', () => {
     const timeText = getTimeText();
 
     if (isIOS()) {
-      const testElement = element(
-        by.type('UIPickerView').withAncestor(by.id('dateTimePicker')),
-      );
+      const testElement = getDateTimePickerIOS();
       await testElement.setColumnToValue(0, '2');
       await testElement.setColumnToValue(1, '44');
       await testElement.setColumnToValue(2, 'PM');
@@ -149,6 +139,14 @@ describe('Example', () => {
     await expect(dateText).toHaveText('01/01/1970');
   });
 
+  it(':android: when component unmounts, dialog is dismissed', async () => {
+    await elementById('showAndDismissPickerButton').tap();
+    await expect(getDatePickerAndroid()).toBeVisible();
+    await wait(3500);
+
+    await expect(getDatePickerAndroid()).toNotExist();
+  });
+
   describe('given 5-minute interval', () => {
     it(':android: clock picker should correct 18-minute selection to 20-minute one', async () => {
       try {
@@ -195,9 +193,7 @@ describe('Example', () => {
     it(':ios: picker should offer only options divisible by 5 (0, 5, 10,...)', async () => {
       await userOpensPicker({mode: 'time', display: 'spinner', interval: 5});
 
-      const testElement = element(
-        by.type('UIPickerView').withAncestor(by.id('dateTimePicker')),
-      );
+      const testElement = getDateTimePickerIOS();
       await testElement.setColumnToValue(0, '2');
       await testElement.setColumnToValue(2, 'PM');
       const timeText = getTimeText();
diff --git a/example/e2e/utils/matchers.js b/example/e2e/utils/matchers.js
@@ -2,10 +2,16 @@ const getTimeText = () => element(by.id('timeText'));
 const getDateText = () => element(by.id('dateText'));
 const elementById = (id) => element(by.id(id));
 const elementByText = (text) => element(by.text(text));
+const getDateTimePickerIOS = () =>
+  element(by.type('UIPickerView').withAncestor(by.id('dateTimePicker')));
+const getDatePickerAndroid = () =>
+  element(by.type('android.widget.DatePicker'));
 
 module.exports = {
   getTimeText,
   getDateText,
   elementById,
   elementByText,
+  getDateTimePickerIOS,
+  getDatePickerAndroid,
 };
diff --git a/example/e2e/utils/utils.js b/example/e2e/utils/utils.js
@@ -1,7 +1,10 @@
 const isAndroid = () => device.getPlatform() === 'android';
 const isIOS = () => device.getPlatform() === 'ios';
+const wait = async (time = 1000) =>
+  new Promise((resolve) => setTimeout(resolve, time));
 
 module.exports = {
   isAndroid,
   isIOS,
+  wait,
 };
diff --git a/src/datepicker.android.js b/src/datepicker.android.js
@@ -42,6 +42,10 @@ export default class DatePickerAndroid {
     return NativeModules.RNDatePickerAndroid.open(options);
   }
 
+  static async dismiss(): Promise<boolean> {
+    return NativeModules.RNDatePickerAndroid.dismiss();
+  }
+
   /**
    * A date has been selected.
    */
diff --git a/src/datetimepicker.android.js b/src/datetimepicker.android.js
@@ -15,6 +15,7 @@ import {
 } from './constants';
 import pickers from './picker';
 import invariant from 'invariant';
+import {useEffect} from 'react';
 
 import type {AndroidEvent, AndroidNativeProps} from './types';
 
@@ -66,6 +67,12 @@ export default function RNDateTimePicker(props: AndroidNativeProps) {
       break;
   }
 
+  useEffect(() => {
+    // This effect runs on unmount, and will ensure the picker is closed.
+    // This allows for controlling the opening state of the picker through declarative logic in jsx.
+    return () => (pickers[mode] ?? pickers[MODE_DATE]).dismiss();
+  }, [mode]);
+
   picker.then(
     function resolve({action, day, month, year, minute, hour}) {
       const date = new Date(value);
diff --git a/src/timepicker.android.js b/src/timepicker.android.js
@@ -40,6 +40,10 @@ export default class TimePickerAndroid {
     return NativeModules.RNTimePickerAndroid.open(options);
   }
 
+  static async dismiss(): Promise<boolean> {
+    return NativeModules.RNDatePickerAndroid.dismiss();
+  }
+
   /**
    * A time has been selected.
    */

Original file line number	Diff line number	Diff line change
`@@ -22,6 +22,8 @@`
`22`	`22`	`import com.facebook.react.common.annotations.VisibleForTesting;`
`23`	`23`	`import com.facebook.react.module.annotations.ReactModule;`
`24`	`24`
	`25`	`+import static com.reactcommunity.rndatetimepicker.Common.dismissDialog;`
	`26`	`+`
`25`	`27`	`/**`
`26`	`28`	`* {@link NativeModule} that allows JS to show a native date picker dialog and get called back when`
`27`	`29`	`* the user selects a date.`
`@@ -84,6 +86,11 @@ public void onClick(DialogInterface dialog, int which) {`
`84`	`86`	`}`
`85`	`87`	`}`
`86`	`88`
	`89`	`+ @ReactMethod`
	`90`	`+ public void dismiss(Promise promise) {`
	`91`	`+ FragmentActivity activity = (FragmentActivity) getCurrentActivity();`
	`92`	`+ dismissDialog(activity, FRAGMENT_TAG, promise);`
	`93`	`+ }`
`87`	`94`	`/**`
`88`	`95`	`* Show a date picker dialog.`
`89`	`96`	`*`