From 3b4740801606a9784fca307e01ac92d4f56f3de7 Mon Sep 17 00:00:00 2001 From: Edgar Wang Date: Tue, 7 Jan 2020 18:19:05 +0800 Subject: [PATCH] Refine remove locale warning dialog string - If the first language is not selected, just remove the body copy else warning user the locale will change to another language. Fixes: 140723349 Test: manual & robotest Change-Id: I9fec17ae85889f2864c3f3cae744f7181e6f9b2c --- .../LocaleDragAndDropAdapter.java | 4 + .../localepicker/LocaleListEditor.java | 54 +++++----- .../localepicker/LocaleListEditorTest.java | 99 ++++++++++++++++++- 3 files changed, 132 insertions(+), 25 deletions(-) diff --git a/src/com/android/settings/localepicker/LocaleDragAndDropAdapter.java b/src/com/android/settings/localepicker/LocaleDragAndDropAdapter.java index a06c77eac11..ab9110d01ab 100644 --- a/src/com/android/settings/localepicker/LocaleDragAndDropAdapter.java +++ b/src/com/android/settings/localepicker/LocaleDragAndDropAdapter.java @@ -251,6 +251,10 @@ class LocaleDragAndDropAdapter return result; } + boolean isFirstLocaleChecked() { + return mFeedItemList != null && mFeedItemList.get(0).getChecked(); + } + void addLocale(LocaleStore.LocaleInfo li) { mFeedItemList.add(li); notifyItemInserted(mFeedItemList.size() - 1); diff --git a/src/com/android/settings/localepicker/LocaleListEditor.java b/src/com/android/settings/localepicker/LocaleListEditor.java index 0666e0bebeb..3165d09a9fe 100644 --- a/src/com/android/settings/localepicker/LocaleListEditor.java +++ b/src/com/android/settings/localepicker/LocaleListEditor.java @@ -34,6 +34,7 @@ import android.view.View; import android.view.ViewGroup; import android.widget.TextView; +import androidx.annotation.VisibleForTesting; import androidx.appcompat.app.AlertDialog; import androidx.recyclerview.widget.RecyclerView; @@ -184,7 +185,8 @@ public class LocaleListEditor extends RestrictedSettingsFragment { // Shows no warning if there is no locale checked, shows a warning // about removing all the locales if all of them are checked, and // a "regular" warning otherwise. - private void showRemoveLocaleWarningDialog() { + @VisibleForTesting + void showRemoveLocaleWarningDialog() { int checkedCount = mAdapter.getCheckedCount(); // Nothing checked, just exit remove mode without a warning dialog @@ -218,33 +220,41 @@ public class LocaleListEditor extends RestrictedSettingsFragment { final String title = getResources().getQuantityString(R.plurals.dlg_remove_locales_title, checkedCount); mShowingRemoveDialog = true; - new AlertDialog.Builder(getActivity()) - .setTitle(title) - .setMessage(R.string.dlg_remove_locales_message) + + final AlertDialog.Builder builder = new AlertDialog.Builder(getActivity()); + if (mAdapter.isFirstLocaleChecked()) { + builder.setMessage(R.string.dlg_remove_locales_message); + } + + builder.setTitle(title) .setNegativeButton(android.R.string.no, new DialogInterface.OnClickListener() { @Override public void onClick(DialogInterface dialog, int which) { setRemoveMode(false); } }) - .setPositiveButton(android.R.string.yes, new DialogInterface.OnClickListener() { - @Override - public void onClick(DialogInterface dialog, int which) { - // This is a sensitive area to change. - // removeChecked() triggers a system update and "kills" the frame. - // This means that saveState + restoreState are called before - // setRemoveMode is called. - // So we want that mRemoveMode and dialog status have the right values - // before that save. - // We can't just call setRemoveMode(false) before calling removeCheched - // because that unchecks all items and removeChecked would have nothing - // to remove. - mRemoveMode = false; - mShowingRemoveDialog = false; - mAdapter.removeChecked(); - setRemoveMode(false); - } - }) + .setPositiveButton(R.string.locale_remove_menu, + new DialogInterface.OnClickListener() { + @Override + public void onClick(DialogInterface dialog, int which) { + // This is a sensitive area to change. + // removeChecked() triggers a system update and "kills" the frame. + // This means that saveState + restoreState are called before + // setRemoveMode is called. + // So we want that mRemoveMode and dialog status have the right + // values + // before that save. + // We can't just call setRemoveMode(false) before calling + // removeCheched + // because that unchecks all items and removeChecked would have + // nothing + // to remove. + mRemoveMode = false; + mShowingRemoveDialog = false; + mAdapter.removeChecked(); + setRemoveMode(false); + } + }) .setOnDismissListener(new DialogInterface.OnDismissListener() { @Override public void onDismiss(DialogInterface dialog) { diff --git a/tests/robotests/src/com/android/settings/localepicker/LocaleListEditorTest.java b/tests/robotests/src/com/android/settings/localepicker/LocaleListEditorTest.java index d84d77923a3..f9b1543f9a7 100644 --- a/tests/robotests/src/com/android/settings/localepicker/LocaleListEditorTest.java +++ b/tests/robotests/src/com/android/settings/localepicker/LocaleListEditorTest.java @@ -18,40 +18,68 @@ package com.android.settings.localepicker; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; + import android.content.Context; import android.view.View; import android.widget.TextView; -import com.android.settings.testutils.FakeFeatureFactory; +import androidx.appcompat.app.AlertDialog; +import androidx.fragment.app.FragmentActivity; +import com.android.settings.R; +import com.android.settings.testutils.FakeFeatureFactory; +import com.android.settings.testutils.shadow.ShadowAlertDialogCompat; + +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.robolectric.Robolectric; import org.robolectric.RobolectricTestRunner; import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; import org.robolectric.util.ReflectionHelpers; @RunWith(RobolectricTestRunner.class) +@Config(shadows = ShadowAlertDialogCompat.class) public class LocaleListEditorTest { private LocaleListEditor mLocaleListEditor; - @Mock private Context mContext; + private FragmentActivity mActivity; + + @Mock + private LocaleDragAndDropAdapter mAdapter; @Before public void setUp() { - mLocaleListEditor = new LocaleListEditor(); + MockitoAnnotations.initMocks(this); + mContext = RuntimeEnvironment.application; + mLocaleListEditor = spy(new LocaleListEditor()); + when(mLocaleListEditor.getContext()).thenReturn(mContext); + mActivity = Robolectric.buildActivity(FragmentActivity.class).get(); + when(mLocaleListEditor.getActivity()).thenReturn(mActivity); ReflectionHelpers.setField(mLocaleListEditor, "mEmptyTextView", new TextView(RuntimeEnvironment.application)); ReflectionHelpers.setField(mLocaleListEditor, "mRestrictionsManager", RuntimeEnvironment.application.getSystemService(Context.RESTRICTIONS_SERVICE)); ReflectionHelpers.setField(mLocaleListEditor, "mUserManager", RuntimeEnvironment.application.getSystemService(Context.USER_SERVICE)); + ReflectionHelpers.setField(mLocaleListEditor, "mAdapter", mAdapter); FakeFeatureFactory.setupForTest(); } + @After + public void tearDown() { + ReflectionHelpers.setField(mLocaleListEditor, "mRemoveMode", false); + ReflectionHelpers.setField(mLocaleListEditor, "mShowingRemoveDialog", false); + } + @Test public void testDisallowConfigLocale_unrestrict() { ReflectionHelpers.setField(mLocaleListEditor, "mIsUiRestricted", true); @@ -67,4 +95,69 @@ public class LocaleListEditorTest { mLocaleListEditor.onResume(); assertThat(mLocaleListEditor.getEmptyTextView().getVisibility()).isEqualTo(View.VISIBLE); } + + @Test + public void showRemoveLocaleWarningDialog_allLocaleSelected_shouldShowErrorDialog() { + //pre-condition + when(mAdapter.getCheckedCount()).thenReturn(1); + when(mAdapter.getItemCount()).thenReturn(1); + when(mAdapter.isFirstLocaleChecked()).thenReturn(true); + ReflectionHelpers.setField(mLocaleListEditor, "mRemoveMode", true); + ReflectionHelpers.setField(mLocaleListEditor, "mShowingRemoveDialog", true); + + //launch dialog + mLocaleListEditor.showRemoveLocaleWarningDialog(); + + final AlertDialog dialog = ShadowAlertDialogCompat.getLatestAlertDialog(); + + assertThat(dialog).isNotNull(); + + final ShadowAlertDialogCompat shadowDialog = ShadowAlertDialogCompat.shadowOf(dialog); + + assertThat(shadowDialog.getTitle()).isEqualTo( + mContext.getString(R.string.dlg_remove_locales_error_title)); + } + + @Test + public void showRemoveLocaleWarningDialog_mainLocaleSelected_shouldShowLocaleChangeDialog() { + //pre-condition + when(mAdapter.getCheckedCount()).thenReturn(1); + when(mAdapter.getItemCount()).thenReturn(2); + when(mAdapter.isFirstLocaleChecked()).thenReturn(true); + ReflectionHelpers.setField(mLocaleListEditor, "mRemoveMode", true); + ReflectionHelpers.setField(mLocaleListEditor, "mShowingRemoveDialog", true); + + //launch dialog + mLocaleListEditor.showRemoveLocaleWarningDialog(); + + final AlertDialog dialog = ShadowAlertDialogCompat.getLatestAlertDialog(); + + assertThat(dialog).isNotNull(); + + final ShadowAlertDialogCompat shadowDialog = ShadowAlertDialogCompat.shadowOf(dialog); + + assertThat(shadowDialog.getMessage()).isEqualTo( + mContext.getString(R.string.dlg_remove_locales_message)); + } + + @Test + public void showRemoveLocaleWarningDialog_mainLocaleNotSelected_shouldShowConfirmDialog() { + //pre-condition + when(mAdapter.getCheckedCount()).thenReturn(1); + when(mAdapter.getItemCount()).thenReturn(2); + when(mAdapter.isFirstLocaleChecked()).thenReturn(false); + ReflectionHelpers.setField(mLocaleListEditor, "mRemoveMode", true); + ReflectionHelpers.setField(mLocaleListEditor, "mShowingRemoveDialog", true); + + //launch dialog + mLocaleListEditor.showRemoveLocaleWarningDialog(); + + final AlertDialog dialog = ShadowAlertDialogCompat.getLatestAlertDialog(); + + assertThat(dialog).isNotNull(); + + final ShadowAlertDialogCompat shadowDialog = ShadowAlertDialogCompat.shadowOf(dialog); + + assertThat(shadowDialog.getMessage()).isNull(); + } }