From 243070afe2873c09c1149ca3ede3386abb88a529 Mon Sep 17 00:00:00 2001 From: Dulmandakh Date: Fri, 26 Apr 2019 02:43:14 -0700 Subject: [PATCH] DialogModule supports only FragmentActivity (#23365) Summary: Now RN has only ReactActivity which extends AppCompatActivity, subclass of FragmentActivity, therefore no need to check if activity is FragmentActivity or not. This PR changes DialogModule to work only with FragmentActivity. Also DialogFragment from Android is deprecated in API 28, and recommends to use DialogFragment from Support Library. Excerpt from DialogFragment documentation. > **This class was deprecated in API level 28.** > Use the Support Library DialogFragment for consistent behavior across all devices and access to Lifecycle. **BREAKING CHANGE**: Brown field apps must extend FragmentActivity or its subclasses. [Android] [Changed] - DialogModule supports only FragmentActivity Pull Request resolved: https://github.com/facebook/react-native/pull/23365 Differential Revision: D14021986 Pulled By: cpojer fbshipit-source-id: b0ede60ef19cec48111a12701659a8bc1f66c331 --- .../react/modules/dialog/AlertFragment.java | 2 +- .../react/modules/dialog/DialogModule.java | 84 +++++-------------- .../modules/dialog/SupportAlertFragment.java | 56 ------------- .../modules/dialog/DialogModuleTest.java | 15 ++-- 4 files changed, 31 insertions(+), 126 deletions(-) delete mode 100644 ReactAndroid/src/main/java/com/facebook/react/modules/dialog/SupportAlertFragment.java diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/AlertFragment.java b/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/AlertFragment.java index 08d10e6cd88eac..a3bb7ca00d1d4a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/AlertFragment.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/AlertFragment.java @@ -12,10 +12,10 @@ import android.annotation.SuppressLint; import android.app.AlertDialog; import android.app.Dialog; -import android.app.DialogFragment; import android.content.DialogInterface; import android.content.Context; import android.os.Bundle; +import androidx.fragment.app.DialogFragment; /** * A fragment used to display the dialog. diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/DialogModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/DialogModule.java index eb05b067a71949..2f7cfc487c0b36 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/DialogModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/DialogModule.java @@ -13,6 +13,8 @@ import android.content.DialogInterface.OnDismissListener; import android.os.Bundle; import androidx.fragment.app.FragmentActivity; +import androidx.fragment.app.FragmentManager; + import com.facebook.common.logging.FLog; import com.facebook.react.bridge.Callback; import com.facebook.react.bridge.LifecycleEventListener; @@ -26,6 +28,8 @@ import com.facebook.react.common.MapBuilder; import com.facebook.react.module.annotations.ReactModule; import java.util.Map; + +import javax.annotation.Nonnull; import javax.annotation.Nullable; @ReactModule(name = DialogModule.NAME) @@ -60,35 +64,17 @@ public DialogModule(ReactApplicationContext reactContext) { } @Override - public String getName() { + public @Nonnull String getName() { return NAME; } - /** - * Helper to allow this module to work with both the standard FragmentManager - * and the Support FragmentManager (for apps that need to use it for legacy reasons). - * Since the two APIs don't share a common interface there's unfortunately some - * code duplication. - */ private class FragmentManagerHelper { - - // Exactly one of the two is null - private final @Nullable android.app.FragmentManager mFragmentManager; - private final @Nullable androidx.fragment.app.FragmentManager mSupportFragmentManager; + private final @Nonnull FragmentManager mFragmentManager; private @Nullable Object mFragmentToShow; - private boolean isUsingSupportLibrary() { - return mSupportFragmentManager != null; - } - - public FragmentManagerHelper(androidx.fragment.app.FragmentManager supportFragmentManager) { - mFragmentManager = null; - mSupportFragmentManager = supportFragmentManager; - } - public FragmentManagerHelper(android.app.FragmentManager fragmentManager) { + public FragmentManagerHelper(@Nonnull FragmentManager fragmentManager) { mFragmentManager = fragmentManager; - mSupportFragmentManager = null; } public void showPendingAlert() { @@ -99,11 +85,7 @@ public void showPendingAlert() { } dismissExisting(); - if (isUsingSupportLibrary()) { - ((SupportAlertFragment) mFragmentToShow).show(mSupportFragmentManager, FRAGMENT_TAG); - } else { - ((AlertFragment) mFragmentToShow).show(mFragmentManager, FRAGMENT_TAG); - } + ((AlertFragment) mFragmentToShow).show(mFragmentManager, FRAGMENT_TAG); mFragmentToShow = null; } @@ -111,18 +93,10 @@ private void dismissExisting() { if (!mIsInForeground) { return; } - if (isUsingSupportLibrary()) { - SupportAlertFragment oldFragment = - (SupportAlertFragment) mSupportFragmentManager.findFragmentByTag(FRAGMENT_TAG); - if (oldFragment != null && oldFragment.isResumed()) { - oldFragment.dismiss(); - } - } else { - AlertFragment oldFragment = - (AlertFragment) mFragmentManager.findFragmentByTag(FRAGMENT_TAG); - if (oldFragment != null && oldFragment.isResumed()) { - oldFragment.dismiss(); - } + AlertFragment oldFragment = + (AlertFragment) mFragmentManager.findFragmentByTag(FRAGMENT_TAG); + if (oldFragment != null && oldFragment.isResumed()) { + oldFragment.dismiss(); } } @@ -134,26 +108,14 @@ public void showNewAlert(Bundle arguments, Callback actionCallback) { AlertFragmentListener actionListener = actionCallback != null ? new AlertFragmentListener(actionCallback) : null; - if (isUsingSupportLibrary()) { - SupportAlertFragment alertFragment = new SupportAlertFragment(actionListener, arguments); - if (mIsInForeground && !mSupportFragmentManager.isStateSaved()) { - if (arguments.containsKey(KEY_CANCELABLE)) { - alertFragment.setCancelable(arguments.getBoolean(KEY_CANCELABLE)); - } - alertFragment.show(mSupportFragmentManager, FRAGMENT_TAG); - } else { - mFragmentToShow = alertFragment; + AlertFragment alertFragment = new AlertFragment(actionListener, arguments); + if (mIsInForeground && !mFragmentManager.isStateSaved()) { + if (arguments.containsKey(KEY_CANCELABLE)) { + alertFragment.setCancelable(arguments.getBoolean(KEY_CANCELABLE)); } + alertFragment.show(mFragmentManager, FRAGMENT_TAG); } else { - AlertFragment alertFragment = new AlertFragment(actionListener, arguments); - if (mIsInForeground) { - if (arguments.containsKey(KEY_CANCELABLE)) { - alertFragment.setCancelable(arguments.getBoolean(KEY_CANCELABLE)); - } - alertFragment.show(mFragmentManager, FRAGMENT_TAG); - } else { - mFragmentToShow = alertFragment; - } + mFragmentToShow = alertFragment; } } } @@ -269,8 +231,8 @@ public void run() { } /** - * Creates a new helper to work with either the FragmentManager or the legacy support - * FragmentManager transparently. Returns null if we're not attached to an Activity. + * Creates a new helper to work with FragmentManager. + * Returns null if we're not attached to an Activity. * * DO NOT HOLD LONG-LIVED REFERENCES TO THE OBJECT RETURNED BY THIS METHOD, AS THIS WILL CAUSE * MEMORY LEAKS. @@ -280,10 +242,6 @@ public void run() { if (activity == null) { return null; } - if (activity instanceof FragmentActivity) { - return new FragmentManagerHelper(((FragmentActivity) activity).getSupportFragmentManager()); - } else { - return new FragmentManagerHelper(activity.getFragmentManager()); - } + return new FragmentManagerHelper(((FragmentActivity) activity).getSupportFragmentManager()); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/SupportAlertFragment.java b/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/SupportAlertFragment.java deleted file mode 100644 index 9c3f7211ee4e5c..00000000000000 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/SupportAlertFragment.java +++ /dev/null @@ -1,56 +0,0 @@ -/** - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -package com.facebook.react.modules.dialog; - -import javax.annotation.Nullable; - -import android.annotation.SuppressLint; -import android.app.Dialog; -import android.content.DialogInterface; -import android.os.Bundle; - -import androidx.fragment.app.DialogFragment; - -/** - * {@link AlertFragment} for apps that use the Support FragmentActivity and FragmentManager - * for legacy reasons. - */ -public class SupportAlertFragment extends DialogFragment implements DialogInterface.OnClickListener { - - private final @Nullable DialogModule.AlertFragmentListener mListener; - - public SupportAlertFragment() { - mListener = null; - } - - @SuppressLint("ValidFragment") - public SupportAlertFragment(@Nullable DialogModule.AlertFragmentListener listener, Bundle arguments) { - mListener = listener; - setArguments(arguments); - } - - @Override - public Dialog onCreateDialog(Bundle savedInstanceState) { - return AlertFragment.createDialog(getActivity(), getArguments(), this); - } - - @Override - public void onClick(DialogInterface dialog, int which) { - if (mListener != null) { - mListener.onClick(dialog, which); - } - } - - @Override - public void onDismiss(DialogInterface dialog) { - super.onDismiss(dialog); - if (mListener != null) { - mListener.onDismiss(dialog); - } - } -} diff --git a/ReactAndroid/src/test/java/com/facebook/react/modules/dialog/DialogModuleTest.java b/ReactAndroid/src/test/java/com/facebook/react/modules/dialog/DialogModuleTest.java index 48acb7265b93db..7a9fe4c5e6beb4 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/modules/dialog/DialogModuleTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/modules/dialog/DialogModuleTest.java @@ -9,7 +9,7 @@ import android.app.AlertDialog; import android.content.DialogInterface; -import android.app.Activity; + import com.facebook.react.bridge.Callback; import com.facebook.react.bridge.ReactApplicationContext; @@ -25,15 +25,18 @@ import org.robolectric.RobolectricTestRunner; import org.robolectric.util.ActivityController; +import androidx.fragment.app.FragmentActivity; + import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @RunWith(RobolectricTestRunner.class) @PowerMockIgnore({"org.mockito.*", "org.robolectric.*", "androidx.*", "android.*"}) public class DialogModuleTest { - private ActivityController mActivityController; - private Activity mActivity; + private ActivityController mActivityController; + private FragmentActivity mActivity; private DialogModule mDialogModule; final static class SimpleCallback implements Callback { @@ -57,7 +60,7 @@ public Object[] getArgs() { @Before public void setUp() throws Exception { - mActivityController = Robolectric.buildActivity(Activity.class); + mActivityController = Robolectric.buildActivity(FragmentActivity.class); mActivity = mActivityController .create() .start() @@ -94,7 +97,7 @@ public void testAllOptions() { final AlertFragment fragment = getFragment(); assertNotNull("Fragment was not displayed", fragment); - assertEquals(false, fragment.isCancelable()); + assertFalse(fragment.isCancelable()); final AlertDialog dialog = (AlertDialog) fragment.getDialog(); assertEquals("OK", dialog.getButton(DialogInterface.BUTTON_POSITIVE).getText().toString()); @@ -164,7 +167,7 @@ public void testCallbackDismiss() { } private AlertFragment getFragment() { - return (AlertFragment) mActivity.getFragmentManager() + return (AlertFragment) mActivity.getSupportFragmentManager() .findFragmentByTag(DialogModule.FRAGMENT_TAG); } }