Fix UX problems in time zone pickers

- Remove emoji region flag in the region picker.
  It's more consistent with locale picker which shows no flag in region
  picker
- Remove redundant information in the summary field
  e.g. same GMT offset in primary and secondary field in fixed offset
  picker
- Better mode switching flow. Switch region/fixed offset mode
only when the user confirms their selection in the picker.

Bug: 73952488
Test: m RunSettingsRoboTests ROBOTEST_FILTER=com.android.settings.datetime.timezone
Change-Id: Id5da8a2516acd10c9a3d71181e94bc617d797d20

src/com/android/settings/datetime/timezone/BaseTimeZoneInfoPicker.java

@@ -31,6 +31,7 @@ import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
 import java.util.Locale;
+import java.util.Objects;
 
 /**
  * Render a list of {@class TimeZoneInfo} into the list view in {@class BaseTimeZonePicker}
@@ -135,8 +136,11 @@ public abstract class BaseTimeZoneInfoPicker extends BaseTimeZonePicker {
                     name = mTimeZoneInfo.getStandardName();
                 }
             }
-            if (name == null) {
+
-                return mTimeZoneInfo.getGmtOffset();
+            // Ignore name / GMT offset if the title shows the same information
+            if (name == null || name.equals(mTitle)) {
+                CharSequence gmtOffset = mTimeZoneInfo.getGmtOffset();
+                return gmtOffset == null || gmtOffset.toString().equals(mTitle) ? "" : gmtOffset;
             } else {
                 return SpannableUtil.getResourcesText(mResources,
                         R.string.zone_info_offset_and_name, mTimeZoneInfo.getGmtOffset(), name);

src/com/android/settings/datetime/timezone/RegionSearchPicker.java

@@ -110,56 +110,25 @@ public class RegionSearchPicker extends BaseTimeZonePicker {
     private List<RegionItem> createAdapterItem(Set<String> regionIds) {
         final Collator collator = Collator.getInstance(getLocale());
         final TreeSet<RegionItem> items = new TreeSet<>(new RegionInfoComparator(collator));
-        final Paint paint = new Paint();
         final LocaleDisplayNames localeDisplayNames = LocaleDisplayNames.getInstance(getLocale());
         long i = 0;
         for (String regionId : regionIds) {
             String name = localeDisplayNames.regionDisplayName(regionId);
-            String regionalIndicator = createRegionalIndicator(regionId, paint);
+            items.add(new RegionItem(i++, regionId, name));
-            items.add(new RegionItem(i++, regionId, name, regionalIndicator));
         }
         return new ArrayList<>(items);
     }
 
-    /**
-     * Create a Unicode Region Indicator Symbol for a given region id (a.k.a flag emoji). If the
-     * system can't render a flag for this region or the input is not a region id, this returns
-     * {@code null}.
-     *
-     * @param id the two-character region id.
-     * @param paint Paint contains the glyph
-     * @return a String representing the flag of the region or {@code null}.
-     */
-    private static String createRegionalIndicator(String id, Paint paint) {
-        if (id.length() != 2) {
-            return null;
-        }
-        final char c1 = id.charAt(0);
-        final char c2 = id.charAt(1);
-        if ('A' > c1 || c1 > 'Z' || 'A' > c2 || c2 > 'Z') {
-            return null;
-        }
-        // Regional Indicator A is U+1F1E6 which is 0xD83C 0xDDE6 in UTF-16.
-        final String regionalIndicator = new String(
-            new char[]{0xd83c, (char) (0xdde6 - 'A' + c1), 0xd83c, (char) (0xdde6 - 'A' + c2)});
-        if (!paint.hasGlyph(regionalIndicator)) {
-            return null;
-        }
-        return regionalIndicator;
-    }
-
     private static class RegionItem implements BaseTimeZoneAdapter.AdapterItem {
 
         private final String mId;
         private final String mName;
-        private final String mRegionalIndicator;
         private final long mItemId;
         private final String[] mSearchKeys;
 
-        RegionItem(long itemId, String id, String name, String regionalIndicator) {
+        RegionItem(long itemId, String id, String name) {
             mId = id;
             mName = name;
-            mRegionalIndicator = regionalIndicator;
             mItemId = itemId;
             // Allow to search with ISO_3166-1 alpha-2 code. It's handy for english users in some
             // countries, e.g. US for United States. It's not best search keys for users, but
@@ -183,7 +152,7 @@ public class RegionSearchPicker extends BaseTimeZonePicker {
 
         @Override
         public String getIconText() {
-            return mRegionalIndicator;
+            return null;
         }
 
         @Override

src/com/android/settings/datetime/timezone/TimeZoneSettings.java

@@ -98,7 +98,7 @@ public class TimeZoneSettings extends DashboardFragment {
         final List<AbstractPreferenceController> controllers = new ArrayList<>();
         RegionPreferenceController regionPreferenceController =
                 new RegionPreferenceController(context);
-        regionPreferenceController.setOnClickListener(this::onRegionPreferenceClicked);
+        regionPreferenceController.setOnClickListener(this::startRegionPicker);
         RegionZonePreferenceController regionZonePreferenceController =
                 new RegionZonePreferenceController(context);
         regionZonePreferenceController.setOnClickListener(this::onRegionZonePreferenceClicked);
@@ -106,7 +106,7 @@ public class TimeZoneSettings extends DashboardFragment {
                 new TimeZoneInfoPreferenceController(context);
         FixedOffsetPreferenceController fixedOffsetPreferenceController =
                 new FixedOffsetPreferenceController(context);
-        fixedOffsetPreferenceController.setOnClickListener(this::onFixedOffsetPreferenceClicked);
+        fixedOffsetPreferenceController.setOnClickListener(this::startFixedOffsetPicker);
 
         controllers.add(regionPreferenceController);
         controllers.add(regionZonePreferenceController);
@@ -172,7 +172,7 @@ public class TimeZoneSettings extends DashboardFragment {
 
     }
 
-    private void onRegionPreferenceClicked() {
+    private void startRegionPicker() {
         startPickerFragment(RegionSearchPicker.class, new Bundle(), REQUEST_CODE_REGION_PICKER);
     }
 
@@ -183,7 +183,7 @@ public class TimeZoneSettings extends DashboardFragment {
         startPickerFragment(RegionZonePicker.class, args, REQUEST_CODE_ZONE_PICKER);
     }
 
-    private void onFixedOffsetPreferenceClicked() {
+    private void startFixedOffsetPicker() {
         startPickerFragment(FixedOffsetPicker.class, new Bundle(),
                 REQUEST_CODE_FIXED_OFFSET_ZONE_PICKER);
     }
@@ -240,12 +240,18 @@ public class TimeZoneSettings extends DashboardFragment {
         setDisplayedRegion(regionId);
         setDisplayedTimeZoneInfo(regionId, mSelectedTimeZoneId);
         saveTimeZone(regionId, mSelectedTimeZoneId);
+
+        // Switch to the region mode if the user switching from the fixed offset
+        setSelectByRegion(true);
     }
 
     private void onFixedOffsetZoneChanged(String tzId) {
         mSelectedTimeZoneId = tzId;
         setDisplayedFixedOffsetTimeZoneInfo(tzId);
         saveTimeZone(null, mSelectedTimeZoneId);
+
+        // Switch to the fixed offset mode if the user switching from the region mode
+        setSelectByRegion(false);
     }
 
     private void saveTimeZone(String regionId, String tzId) {
@@ -277,11 +283,11 @@ public class TimeZoneSettings extends DashboardFragment {
     public boolean onOptionsItemSelected(MenuItem item) {
         switch (item.getItemId()) {
             case MENU_BY_REGION:
-                setSelectByRegion(true);
+                startRegionPicker();
                 return true;
 
             case MENU_BY_OFFSET:
-                setSelectByRegion(false);
+                startFixedOffsetPicker();
                 return true;
 
             default:

tests/robotests/src/com/android/settings/datetime/timezone/FixedOffsetPickerTest.java

@@ -16,41 +16,78 @@
 
 package com.android.settings.datetime.timezone;
 
+import android.content.Context;
+
+import com.android.settings.datetime.timezone.BaseTimeZoneAdapter.AdapterItem;
 import com.android.settings.datetime.timezone.model.TimeZoneData;
 import com.android.settings.testutils.SettingsRobolectricTestRunner;
 
 import libcore.util.CountryZonesFinder;
 
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.robolectric.RuntimeEnvironment;
 
 import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
 import java.util.stream.Collectors;
 
+import static com.google.common.truth.Truth.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 @RunWith(SettingsRobolectricTestRunner.class)
 public class FixedOffsetPickerTest {
 
+    private CountryZonesFinder mFinder;
+
+    @Before
+    public void setUp() {
+        List regionList = Collections.emptyList();
+        mFinder = mock(CountryZonesFinder.class);
+        when(mFinder.lookupAllCountryIsoCodes()).thenReturn(regionList);
+    }
+
     @Test
     public void getAllTimeZoneInfos_containsUtcAndGmtZones() {
-        List regionList = Collections.emptyList();
+        TestFixedOffsetPicker picker = new TestFixedOffsetPicker();
-        CountryZonesFinder finder = mock(CountryZonesFinder.class);
+        List<TimeZoneInfo> infos = picker.getAllTimeZoneInfos(new TimeZoneData(mFinder));
-        when(finder.lookupAllCountryIsoCodes()).thenReturn(regionList);
-
-        FixedOffsetPicker picker = new FixedOffsetPicker() {
-            @Override
-            protected Locale getLocale() {
-                return Locale.US;
-            }
-        };
-        List<TimeZoneInfo> infos = picker.getAllTimeZoneInfos(new TimeZoneData(finder));
         List<String> tzIds = infos.stream().map(info -> info.getId()).collect(Collectors.toList());
-        tzIds.contains("Etc/Utc");
+        assertThat(tzIds).contains("Etc/UTC");
-        tzIds.contains("Etc/GMT-12");
+        assertThat(tzIds).contains("Etc/GMT-14"); // Etc/GMT-14 means GMT+14:00
-        tzIds.contains("Etc/GMT+14");
+        assertThat(tzIds).contains("Etc/GMT+12"); // Etc/GMT+14 means GMT-12:00
+    }
+
+    @Test
+    public void createAdapter_verifyTitleAndOffset() {
+        TestFixedOffsetPicker picker = new TestFixedOffsetPicker();
+        BaseTimeZoneAdapter adapter = picker.createAdapter(new TimeZoneData(mFinder));
+        assertThat(adapter.getItemCount()).isEqualTo(12 + 1 + 14); // 27 GMT offsets from -12 to +14
+        AdapterItem utc = adapter.getItem(0);
+        assertThat(utc.getTitle().toString()).isEqualTo("Coordinated Universal Time");
+        assertThat(utc.getSummary().toString()).isEqualTo("GMT+00:00");
+        AdapterItem gmtMinus12 = adapter.getItem(1);
+        assertThat(gmtMinus12.getTitle().toString()).isEqualTo("GMT-12:00");
+        assertThat(gmtMinus12.getSummary().toString()).isEmpty();
+    }
+
+    public static class TestFixedOffsetPicker extends FixedOffsetPicker {
+        // Make the method public
+        @Override
+        public BaseTimeZoneAdapter createAdapter(TimeZoneData timeZoneData) {
+            return super.createAdapter(timeZoneData);
+        }
+
+        @Override
+        protected Locale getLocale() {
+            return Locale.US;
+        }
+
+        @Override
+        public Context getContext() {
+            return RuntimeEnvironment.application;
+        }
     }
 }