RoamingDialog stays open on rotation
Moved the dialog into a fragment instead of using
the AlertBuilder directly in the activity. Fixed jank that
would occur when the device was rotated and the toggle changed
state before actually changing the preference. Wrote integration
test to make sure dialog stays visible on device rotation. The
test is located in the tests folder for the project and can be
run in the same way that is already described in the manifest
file.
Test: Espresso Test
Bug: 32441370
Change-Id: I2880bb719f41f1765bf548e7a3ae363f7108a50d
diff --git a/src/com/android/phone/MobileNetworkSettings.java b/src/com/android/phone/MobileNetworkSettings.java
index 56b6390..6593e3d 100644
--- a/src/com/android/phone/MobileNetworkSettings.java
+++ b/src/com/android/phone/MobileNetworkSettings.java
@@ -16,12 +16,14 @@
package com.android.phone;
+import android.app.DialogFragment;
import com.android.ims.ImsManager;
import com.android.internal.telephony.Phone;
import com.android.internal.telephony.PhoneConstants;
import com.android.internal.telephony.PhoneFactory;
import com.android.internal.telephony.TelephonyIntents;
import com.android.internal.telephony.TelephonyProperties;
+import com.android.phone.RoamingDialogFragment.RoamingDialogListener;
import com.android.settingslib.RestrictedLockUtils;
import java.util.ArrayList;
@@ -29,10 +31,8 @@
import java.util.List;
import android.app.ActionBar;
-import android.app.AlertDialog;
import android.content.BroadcastReceiver;
import android.content.Context;
-import android.content.DialogInterface;
import android.content.Intent;
import android.content.IntentFilter;
import android.content.pm.PackageManager.NameNotFoundException;
@@ -76,8 +76,7 @@
* available from the Phone app (see CallFeaturesSetting for that.)
*/
public class MobileNetworkSettings extends PreferenceActivity
- implements DialogInterface.OnClickListener,
- DialogInterface.OnDismissListener, Preference.OnPreferenceChangeListener{
+ implements Preference.OnPreferenceChangeListener, RoamingDialogListener {
// debug data
private static final String LOG_TAG = "NetworkSettings";
@@ -87,6 +86,9 @@
// Number of active Subscriptions to show tabs
private static final int TAB_THRESHOLD = 2;
+ // fragment tag for roaming data dialog
+ private static final String ROAMING_TAG = "RoamingDialogFragment";
+
//String keys for preference lookup
private static final String BUTTON_PREFERED_NETWORK_MODE = "preferred_network_mode_key";
private static final String BUTTON_ROAMING_KEY = "button_roaming_key";
@@ -154,6 +156,12 @@
private final BroadcastReceiver mPhoneChangeReceiver = new PhoneChangeReceiver();
+ @Override
+ public void onPositiveButtonClick(DialogFragment dialog) {
+ mPhone.setDataRoamingEnabled(true);
+ mButtonDataRoam.setChecked(true);
+ }
+
private class PhoneChangeReceiver extends BroadcastReceiver {
@Override
public void onReceive(Context context, Intent intent) {
@@ -165,24 +173,6 @@
}
}
- //This is a method implemented for DialogInterface.OnClickListener.
- // Used to dismiss the dialogs when they come up.
- public void onClick(DialogInterface dialog, int which) {
- if (which == DialogInterface.BUTTON_POSITIVE) {
- mPhone.setDataRoamingEnabled(true);
- mOkClicked = true;
- } else {
- // Reset the toggle
- mButtonDataRoam.setChecked(false);
- }
- }
-
- @Override
- public void onDismiss(DialogInterface dialog) {
- // Assuming that onClick gets called first
- mButtonDataRoam.setChecked(mOkClicked);
- }
-
/**
* Invoked on each preference click in this hierarchy, overrides
* PreferenceActivity's implementation. Used to make sure we track the
@@ -925,14 +915,10 @@
if (!mButtonDataRoam.isChecked()) {
// First confirm with a warning dialog about charges
mOkClicked = false;
- new AlertDialog.Builder(this).setMessage(
- getResources().getString(R.string.roaming_warning))
- .setTitle(R.string.roaming_alert_title)
- .setIconAttribute(android.R.attr.alertDialogIcon)
- .setPositiveButton(android.R.string.yes, this)
- .setNegativeButton(android.R.string.no, this)
- .show()
- .setOnDismissListener(this);
+ RoamingDialogFragment fragment = new RoamingDialogFragment();
+ fragment.show(getFragmentManager(), ROAMING_TAG);
+ // Don't update the toggle unless the confirm button is actually pressed.
+ return false;
} else {
mPhone.setDataRoamingEnabled(false);
}
diff --git a/src/com/android/phone/RoamingDialogFragment.java b/src/com/android/phone/RoamingDialogFragment.java
new file mode 100644
index 0000000..ec2967f
--- /dev/null
+++ b/src/com/android/phone/RoamingDialogFragment.java
@@ -0,0 +1,73 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.android.phone;
+
+import android.app.AlertDialog;
+import android.app.Dialog;
+import android.app.DialogFragment;
+import android.content.Context;
+import android.content.DialogInterface;
+import android.content.DialogInterface.OnClickListener;
+import android.os.Bundle;
+
+/**
+ * A dialog fragment that asks the user if they are sure they want to turn on data roaming
+ * to avoid accidental charges.
+ */
+public class RoamingDialogFragment extends DialogFragment implements OnClickListener {
+
+ /**
+ * The interface we expect a host activity to implement.
+ */
+ public interface RoamingDialogListener {
+ void onPositiveButtonClick(DialogFragment dialog);
+ }
+
+ // the host activity which implements the listening interface
+ private RoamingDialogListener mListener;
+
+
+ @Override
+ public void onAttach(Context context) {
+ super.onAttach(context);
+ // Verify host activity implemented callback interface
+ try {
+ mListener = (RoamingDialogListener) getActivity();
+ } catch (ClassCastException e) {
+ throw new ClassCastException(getActivity().toString() +
+ "must implement RoamingDialogListener");
+ }
+ }
+
+ @Override
+ public Dialog onCreateDialog(Bundle savedInstanceState) {
+ AlertDialog.Builder builder = new AlertDialog.Builder(getActivity());
+ builder.setMessage(getResources().getString(R.string.roaming_warning))
+ .setTitle(R.string.roaming_alert_title)
+ .setIconAttribute(android.R.attr.alertDialogIcon)
+ .setPositiveButton(android.R.string.yes, this)
+ .setNegativeButton(android.R.string.no, this);
+ return builder.create();
+ }
+
+ @Override
+ public void onClick(DialogInterface dialog, int which) {
+ // let the host know that the positive button has been clicked
+ if (which == dialog.BUTTON_POSITIVE) {
+ mListener.onPositiveButtonClick(this);
+ }
+ }
+}
diff --git a/tests/Android.mk b/tests/Android.mk
index 66df1a8..2feb55a 100644
--- a/tests/Android.mk
+++ b/tests/Android.mk
@@ -32,6 +32,8 @@
LOCAL_STATIC_JAVA_LIBRARIES := \
android-support-test \
mockito-target \
- legacy-android-test
-
+ legacy-android-test \
+ mockito-target-minus-junit4 \
+ espresso-core \
+ truth-prebuilt
include $(BUILD_PACKAGE)
diff --git a/tests/src/com/android/phone/RoamingDialogFragmentTest.java b/tests/src/com/android/phone/RoamingDialogFragmentTest.java
new file mode 100644
index 0000000..96a3fda
--- /dev/null
+++ b/tests/src/com/android/phone/RoamingDialogFragmentTest.java
@@ -0,0 +1,123 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.android.phone;
+
+import android.app.Activity;
+import android.content.pm.ActivityInfo;
+import android.provider.Settings.Global;
+import android.provider.Settings.SettingNotFoundException;
+import android.support.test.espresso.matcher.PreferenceMatchers;
+import android.support.test.rule.ActivityTestRule;
+import com.google.common.truth.Truth;
+import junit.framework.AssertionFailedError;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import static android.support.test.espresso.Espresso.onData;
+import static android.support.test.espresso.Espresso.onView;
+import static android.support.test.espresso.action.ViewActions.click;
+import static android.support.test.espresso.assertion.ViewAssertions.doesNotExist;
+import static android.support.test.espresso.assertion.ViewAssertions.matches;
+import static android.support.test.espresso.matcher.ViewMatchers.hasDescendant;
+import static android.support.test.espresso.matcher.ViewMatchers.isChecked;
+import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed;
+import static android.support.test.espresso.matcher.ViewMatchers.withId;
+import static android.support.test.espresso.matcher.ViewMatchers.withText;
+import static com.google.common.truth.Truth.assertThat;
+
+/**
+ * Espresso tests to check some properties of the dialog that appears when a user
+ * tries to turn on data roaming.
+ */
+public class RoamingDialogFragmentTest {
+
+ @Rule
+ public ActivityTestRule<MobileNetworkSettings> mRule =
+ new ActivityTestRule<>(MobileNetworkSettings.class);
+ private Activity mActivity;
+
+ /**
+ * Make sure roaming is off before we start a test since this checks the dialog that only
+ * shows up when we try to turn it on.
+ */
+ @Before
+ public void disableRoaming() {
+ mActivity = mRule.getActivity();
+
+ // turn off data roaming if it is on
+ try {
+ onData(PreferenceMatchers.withTitle(R.string.roaming))
+ .check(matches(hasDescendant(isChecked())))
+ .perform(click());
+ } catch (AssertionFailedError e) {
+ // don't click the switch if it is already off.
+ }
+ }
+
+ @Test
+ public void dataRoamingDialogPersistsOnRotation() {
+ // click on the data roaming preference to trigger warning dialog
+ onData(PreferenceMatchers.withTitle(R.string.roaming)).perform(click());
+
+ // request both orientations to ensure at least one rotation occurs
+ mActivity.setRequestedOrientation(ActivityInfo.SCREEN_ORIENTATION_LANDSCAPE);
+ mActivity.setRequestedOrientation(ActivityInfo.SCREEN_ORIENTATION_PORTRAIT);
+
+ // verify the title of the dialog is visible
+ onView(withText(R.string.roaming_alert_title)).check(matches(isDisplayed()));
+
+ }
+
+ @Test
+ public void dataRoamingEnabledWhenPositiveButtonClicked() throws SettingNotFoundException {
+ // click on the data roaming preference to trigger warning dialog
+ onData(PreferenceMatchers.withTitle(R.string.roaming)).perform(click());
+
+ // click to confirm we want to turn on data roaming
+ onView(withId(android.R.id.button1)).perform(click());
+
+ // verify that the the setting has actually been changed
+ assertThat(Global.getInt(mActivity.getApplicationContext().getContentResolver(),
+ Global.DATA_ROAMING)).isEqualTo(1);
+ }
+
+ @Test
+ public void dialogDismissedOnNegativeButtonClicked() {
+ // click on the data roaming preference to trigger warning dialog
+ onData(PreferenceMatchers.withTitle(R.string.roaming)).perform(click());
+
+ // click to cancel turning on data roaming
+ onView(withId(android.R.id.button2)).perform(click());
+
+ // verify the title of the dialog is gone
+ onView(withText(R.string.roaming_alert_title)).check(doesNotExist());
+ }
+
+ @Test
+ public void dataRoamingStaysDisabledWhenDialogCanceled() throws SettingNotFoundException {
+ // click on the data roaming preference to trigger warning dialog
+ onData(PreferenceMatchers.withTitle(R.string.roaming)).perform(click());
+
+ // click to cancel turning on data roaming
+ onView(withId(android.R.id.button2)).perform(click());
+
+ // verify that the the setting has not been changed
+ assertThat(Global.getInt(mActivity.getApplicationContext().getContentResolver(),
+ Global.DATA_ROAMING)).isEqualTo(0);
+
+ }
+}