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
diff --git a/src/com/android/settings/datetime/timezone/BaseTimeZoneInfoPicker.java b/src/com/android/settings/datetime/timezone/BaseTimeZoneInfoPicker.java
index b133582..c1a3f8b 100644
--- a/src/com/android/settings/datetime/timezone/BaseTimeZoneInfoPicker.java
+++ b/src/com/android/settings/datetime/timezone/BaseTimeZoneInfoPicker.java
@@ -31,6 +31,7 @@
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 @@
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);
diff --git a/src/com/android/settings/datetime/timezone/RegionSearchPicker.java b/src/com/android/settings/datetime/timezone/RegionSearchPicker.java
index 859b851..bb87e85 100644
--- a/src/com/android/settings/datetime/timezone/RegionSearchPicker.java
+++ b/src/com/android/settings/datetime/timezone/RegionSearchPicker.java
@@ -110,56 +110,25 @@
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, regionalIndicator));
+ items.add(new RegionItem(i++, regionId, name));
}
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 @@
@Override
public String getIconText() {
- return mRegionalIndicator;
+ return null;
}
@Override
diff --git a/src/com/android/settings/datetime/timezone/TimeZoneSettings.java b/src/com/android/settings/datetime/timezone/TimeZoneSettings.java
index aeb5a80..16976c2 100644
--- a/src/com/android/settings/datetime/timezone/TimeZoneSettings.java
+++ b/src/com/android/settings/datetime/timezone/TimeZoneSettings.java
@@ -98,7 +98,7 @@
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 @@
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 @@
}
- private void onRegionPreferenceClicked() {
+ private void startRegionPicker() {
startPickerFragment(RegionSearchPicker.class, new Bundle(), REQUEST_CODE_REGION_PICKER);
}
@@ -183,7 +183,7 @@
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 @@
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 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:
diff --git a/tests/robotests/src/com/android/settings/datetime/timezone/FixedOffsetPickerTest.java b/tests/robotests/src/com/android/settings/datetime/timezone/FixedOffsetPickerTest.java
index 1c555b0..007d3c5 100644
--- a/tests/robotests/src/com/android/settings/datetime/timezone/FixedOffsetPickerTest.java
+++ b/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();
- CountryZonesFinder finder = mock(CountryZonesFinder.class);
- when(finder.lookupAllCountryIsoCodes()).thenReturn(regionList);
-
- FixedOffsetPicker picker = new FixedOffsetPicker() {
- @Override
- protected Locale getLocale() {
- return Locale.US;
- }
- };
- List<TimeZoneInfo> infos = picker.getAllTimeZoneInfos(new TimeZoneData(finder));
+ TestFixedOffsetPicker picker = new TestFixedOffsetPicker();
+ List<TimeZoneInfo> infos = picker.getAllTimeZoneInfos(new TimeZoneData(mFinder));
List<String> tzIds = infos.stream().map(info -> info.getId()).collect(Collectors.toList());
- tzIds.contains("Etc/Utc");
- tzIds.contains("Etc/GMT-12");
- tzIds.contains("Etc/GMT+14");
+ assertThat(tzIds).contains("Etc/UTC");
+ assertThat(tzIds).contains("Etc/GMT-14"); // Etc/GMT-14 means GMT+14:00
+ 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;
+ }
}
}