From 297ded90aa1e905a37dba316e7b19b6779c8c804 Mon Sep 17 00:00:00 2001 From: Peter Abbondanzo Date: Wed, 12 Jun 2024 11:24:56 -0700 Subject: [PATCH] Fall back to app AlertDialog for non AppCompat themes (#44495) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/44495 ## Summary Migrates the `AlertFragment` from `android.app.AlertDialog` to `androidx.appcompat.app.AlertDialog`. This backports tons of fixes that have gone into the AlertDialog component over the years, including proper line wrapping of button text, dark mode support, alignment of buttons, etc. This change provides a fallback to the original `android.app.AlertDialog` if the current activity is not an AppCompat descendant. ## For consideration - Alert dialog themes may no longer need the `android` namespace, meaning themes can now be specified as `alertDialogTheme` rather than `android:alertDialogTheme`. ## Changelog: [Android] [Changed] - Migrated `AlertFragment` dialog builder to use `androidx.appcompat` Reviewed By: zeyap Differential Revision: D57113950 fbshipit-source-id: ba5109c9d79b6ceb042ff93eebe796a2d14ebd63 --- .../react/modules/dialog/AlertFragment.java | 67 ++++++++++++- .../react/modules/dialog/DialogModuleTest.kt | 93 +++++++++++-------- 2 files changed, 118 insertions(+), 42 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/AlertFragment.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/AlertFragment.java index 4e81931ccabaf7..b5e48ae4ec1f27 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/AlertFragment.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/AlertFragment.java @@ -11,12 +11,15 @@ import android.app.Dialog; import android.content.Context; import android.content.DialogInterface; +import android.content.res.TypedArray; import android.os.Bundle; import androidx.annotation.Nullable; import androidx.appcompat.app.AlertDialog; import androidx.fragment.app.DialogFragment; +import com.facebook.infer.annotation.Nullsafe; /** A fragment used to display the dialog. */ +@Nullsafe(Nullsafe.Mode.LOCAL) public class AlertFragment extends DialogFragment implements DialogInterface.OnClickListener { /* package */ static final String ARG_TITLE = "title"; @@ -40,6 +43,35 @@ public AlertFragment(@Nullable DialogModule.AlertFragmentListener listener, Bund public static Dialog createDialog( Context activityContext, Bundle arguments, DialogInterface.OnClickListener fragment) { + if (isAppCompatTheme(activityContext)) { + return createAppCompatDialog(activityContext, arguments, fragment); + } else { + return createAppDialog(activityContext, arguments, fragment); + } + } + + /** + * Checks if the current activity is a descendant of an AppCompat theme. This check is required to + * safely display an AppCompat dialog. If the current activity is not a descendant of an AppCompat + * theme and we attempt to render an AppCompat dialog, this will cause a crash. + * + * @returns true if the current activity is a descendant of an AppCompat theme. + */ + private static boolean isAppCompatTheme(Context activityContext) { + TypedArray attributes = + activityContext.obtainStyledAttributes(androidx.appcompat.R.styleable.AppCompatTheme); + boolean isAppCompat = + attributes.hasValue(androidx.appcompat.R.styleable.AppCompatTheme_windowActionBar); + attributes.recycle(); + return isAppCompat; + } + + /** + * Creates a dialog compatible only with AppCompat activities. This function should be kept in + * sync with {@link createAppDialog}. + */ + private static Dialog createAppCompatDialog( + Context activityContext, Bundle arguments, DialogInterface.OnClickListener fragment) { AlertDialog.Builder builder = new AlertDialog.Builder(activityContext).setTitle(arguments.getString(ARG_TITLE)); @@ -64,9 +96,42 @@ public static Dialog createDialog( return builder.create(); } + /** + * Creates a dialog compatible with non-AppCompat activities. This function should be kept in sync + * with {@link createAppCompatDialog}. + * + * @deprecated non-AppCompat dialogs are deprecated and will be removed in a future version. + */ + private static Dialog createAppDialog( + Context activityContext, Bundle arguments, DialogInterface.OnClickListener fragment) { + android.app.AlertDialog.Builder builder = + new android.app.AlertDialog.Builder(activityContext) + .setTitle(arguments.getString(ARG_TITLE)); + + if (arguments.containsKey(ARG_BUTTON_POSITIVE)) { + builder.setPositiveButton(arguments.getString(ARG_BUTTON_POSITIVE), fragment); + } + if (arguments.containsKey(ARG_BUTTON_NEGATIVE)) { + builder.setNegativeButton(arguments.getString(ARG_BUTTON_NEGATIVE), fragment); + } + if (arguments.containsKey(ARG_BUTTON_NEUTRAL)) { + builder.setNeutralButton(arguments.getString(ARG_BUTTON_NEUTRAL), fragment); + } + // if both message and items are set, Android will only show the message + // and ignore the items argument entirely + if (arguments.containsKey(ARG_MESSAGE)) { + builder.setMessage(arguments.getString(ARG_MESSAGE)); + } + if (arguments.containsKey(ARG_ITEMS)) { + builder.setItems(arguments.getCharSequenceArray(ARG_ITEMS), fragment); + } + + return builder.create(); + } + @Override public Dialog onCreateDialog(Bundle savedInstanceState) { - return createDialog(getActivity(), getArguments(), this); + return createDialog(requireActivity(), requireArguments(), this); } @Override diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/dialog/DialogModuleTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/dialog/DialogModuleTest.kt index b66d7533fbf8c8..6b12bd65d9f16a 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/dialog/DialogModuleTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/dialog/DialogModuleTest.kt @@ -47,18 +47,7 @@ class DialogModuleTest { @Before fun setUp() { - activityController = Robolectric.buildActivity(FragmentActivity::class.java) - activity = activityController.create().start().resume().get() - // We must set the theme to a descendant of AppCompat for the AlertDialog to show without - // raising an exception - activity.setTheme(APP_COMPAT_THEME) - - val context: ReactApplicationContext = mock(ReactApplicationContext::class.java) - whenever(context.hasActiveReactInstance()).thenReturn(true) - whenever(context.currentActivity).thenReturn(activity) - - dialogModule = DialogModule(context) - dialogModule.onHostResume() + setupActivity() } @After @@ -66,19 +55,6 @@ class DialogModuleTest { activityController.pause().stop().destroy() } - @Test - fun testIllegalActivityTheme() { - val options = JavaOnlyMap() - activity.setTheme(NON_APP_COMPAT_THEME) - - assertThrows(NullPointerException::class.java) { - dialogModule.showAlert(options, null, null) - shadowOf(getMainLooper()).idle() - } - - activity.setTheme(APP_COMPAT_THEME) - } - @Test fun testAllOptions() { val options = @@ -96,8 +72,7 @@ class DialogModuleTest { val fragment = getFragment() - assertNotNull("Fragment was not displayed", fragment) - assertFalse(fragment!!.isCancelable) + assertFalse(fragment.isCancelable) val dialog = fragment.dialog as AlertDialog assertEquals("OK", dialog.getButton(DialogInterface.BUTTON_POSITIVE).text.toString()) @@ -113,13 +88,13 @@ class DialogModuleTest { dialogModule.showAlert(options, null, actionCallback) shadowOf(getMainLooper()).idle() - val dialog = getFragment()!!.dialog as AlertDialog + val dialog = getFragment().dialog as AlertDialog dialog.getButton(DialogInterface.BUTTON_POSITIVE).performClick() shadowOf(getMainLooper()).idle() assertEquals(1, actionCallback.calls) - assertEquals(DialogModule.ACTION_BUTTON_CLICKED, actionCallback.args!![0]) - assertEquals(DialogInterface.BUTTON_POSITIVE, actionCallback.args!![1]) + assertEquals(DialogModule.ACTION_BUTTON_CLICKED, actionCallback.args?.get(0)) + assertEquals(DialogInterface.BUTTON_POSITIVE, actionCallback.args?.get(1)) } @Test @@ -130,13 +105,13 @@ class DialogModuleTest { dialogModule.showAlert(options, null, actionCallback) shadowOf(getMainLooper()).idle() - val dialog = getFragment()!!.dialog as AlertDialog + val dialog = getFragment().dialog as AlertDialog dialog.getButton(DialogInterface.BUTTON_NEGATIVE).performClick() shadowOf(getMainLooper()).idle() assertEquals(1, actionCallback.calls) - assertEquals(DialogModule.ACTION_BUTTON_CLICKED, actionCallback.args!![0]) - assertEquals(DialogInterface.BUTTON_NEGATIVE, actionCallback.args!![1]) + assertEquals(DialogModule.ACTION_BUTTON_CLICKED, actionCallback.args?.get(0)) + assertEquals(DialogInterface.BUTTON_NEGATIVE, actionCallback.args?.get(1)) } @Test @@ -147,13 +122,13 @@ class DialogModuleTest { dialogModule.showAlert(options, null, actionCallback) shadowOf(getMainLooper()).idle() - val dialog = getFragment()!!.dialog as AlertDialog + val dialog = getFragment().dialog as AlertDialog dialog.getButton(DialogInterface.BUTTON_NEUTRAL).performClick() shadowOf(getMainLooper()).idle() assertEquals(1, actionCallback.calls) - assertEquals(DialogModule.ACTION_BUTTON_CLICKED, actionCallback.args!![0]) - assertEquals(DialogInterface.BUTTON_NEUTRAL, actionCallback.args!![1]) + assertEquals(DialogModule.ACTION_BUTTON_CLICKED, actionCallback.args?.get(0)) + assertEquals(DialogInterface.BUTTON_NEUTRAL, actionCallback.args?.get(1)) } @Test @@ -164,16 +139,52 @@ class DialogModuleTest { dialogModule.showAlert(options, null, actionCallback) shadowOf(getMainLooper()).idle() - getFragment()!!.dialog!!.dismiss() + getFragment().dialog?.dismiss() + shadowOf(getMainLooper()).idle() + + assertEquals(1, actionCallback.calls) + assertEquals(DialogModule.ACTION_DISMISSED, actionCallback.args?.get(0)) + } + + @Test + fun testNonAppCompatActivityTheme() { + setupActivity(NON_APP_COMPAT_THEME) + + val options = JavaOnlyMap() + + val actionCallback = SimpleCallback() + dialogModule.showAlert(options, null, actionCallback) + shadowOf(getMainLooper()).idle() + + getFragment().dialog?.dismiss() shadowOf(getMainLooper()).idle() assertEquals(1, actionCallback.calls) - assertEquals(DialogModule.ACTION_DISMISSED, actionCallback.args!![0]) + assertEquals(DialogModule.ACTION_DISMISSED, actionCallback.args?.get(0)) } - private fun getFragment(): AlertFragment? { - return activity.supportFragmentManager.findFragmentByTag(DialogModule.FRAGMENT_TAG) - as? AlertFragment + private fun setupActivity(theme: Int = APP_COMPAT_THEME) { + activityController = Robolectric.buildActivity(FragmentActivity::class.java) + activity = activityController.create().start().resume().get() + + // We must set the theme to a descendant of AppCompat for the AlertDialog to show without + // raising an exception + activity.setTheme(theme) + + val context: ReactApplicationContext = mock(ReactApplicationContext::class.java) + whenever(context.hasActiveReactInstance()).thenReturn(true) + whenever(context.currentActivity).thenReturn(activity) + + dialogModule = DialogModule(context) + dialogModule.onHostResume() + } + + private fun getFragment(): AlertFragment { + val maybeFragment = activity.supportFragmentManager.findFragmentByTag(DialogModule.FRAGMENT_TAG) + if (maybeFragment == null || !(maybeFragment is AlertFragment)) { + error("Fragment was not displayed") + } + return maybeFragment } companion object {