fix(worldclock): name an offset-set local row by its real offset
A device set by UTC offset rather than by city reports an id like Etc/GMT-9, which is UTC+9; the local row took its last segment as the city and read "GMT-9". It is now named by its actual offset. LocalClock takes the already-resolved home zone rather than resolving it a second time, the tab reads both of its settings from one subscription, and the local row falls back to the unknown-zone sentence for TalkBack instead of an empty time.
This commit is contained in:
1 parent
755150881b
commit
1333001aa9
9 files changed
+76
-29
No files matched your search
@@ -60,24 +60,25 @@ object LocalClock {
|
||||
* winter while home is Lisbon. It is asked again on every tick, so a DST
|
||||
* change on either side moves it with the clock.
|
||||
*
|
||||
* Null when [enabled] is off, when [deviceZone] is not a listed zone id, when no home zone is stored (home then *is*
|
||||
* the device's zone, so it can never differ), and when [savedZoneIds]
|
||||
* already holds a known zone at the device's offset: that row already
|
||||
* shows the local time, and a second one would be a duplicate.
|
||||
* [homeZone] is `HomeZone.resolve`'s answer. With no home stored it *is*
|
||||
* [deviceZone], so a home that follows the device never shows a row.
|
||||
*
|
||||
* Null when [enabled] is off, when [deviceZone] is not a zone id the
|
||||
* tzdata lists (a bare offset, which no row could be read against), and
|
||||
* when [savedZoneIds] already holds a known zone at the device's offset:
|
||||
* that row already shows the local time, and a second one would be a
|
||||
* duplicate.
|
||||
*/
|
||||
fun zoneToShow(
|
||||
enabled: Boolean,
|
||||
storedHomeZoneId: String?,
|
||||
homeZone: ZoneId,
|
||||
deviceZone: ZoneId,
|
||||
savedZoneIds: List<String>,
|
||||
at: Instant,
|
||||
): ZoneId? {
|
||||
// A device zone that is not an id the tzdata lists (a bare offset, say)
|
||||
// has no city to name the row after.
|
||||
if (!enabled || !Zones.isValid(deviceZone.id)) return null
|
||||
val home = HomeZone.resolve(storedHomeZoneId, deviceZone)
|
||||
val deviceOffset = ZoneComparisons.utcOffsetMinutes(deviceZone, at)
|
||||
if (ZoneComparisons.utcOffsetMinutes(home, at) == deviceOffset) return null
|
||||
if (ZoneComparisons.utcOffsetMinutes(homeZone, at) == deviceOffset) return null
|
||||
val alreadyShown = savedZoneIds.any { id ->
|
||||
Zones.isValid(id) && ZoneComparisons.utcOffsetMinutes(ZoneId.of(id), at) == deviceOffset
|
||||
}
|
||||
|
||||
@@ -68,6 +68,13 @@ object ZoneCatalog {
|
||||
fun sorted(entries: List<ZoneEntry>): List<ZoneEntry> =
|
||||
entries.sortedWith(compareBy({ ZoneSearch.fold(it.city) }, { it.zoneId }))
|
||||
|
||||
/**
|
||||
* True for an id named after a place (`Asia/Tokyo`) or for [UTC]; false for
|
||||
* the offset and abbreviation ids tzdata also lists (`Etc/GMT-9`, `CET`),
|
||||
* whose last segment is no city and, for `Etc/GMT±n`, has the sign reversed.
|
||||
*/
|
||||
fun isCityId(zoneId: String): Boolean = zoneId == UTC || isRegionId(zoneId)
|
||||
|
||||
private fun isRegionId(zoneId: String): Boolean =
|
||||
zoneId.contains('/') && zoneId.substringBefore('/') in REGIONS
|
||||
}
|
||||
@@ -147,9 +147,14 @@ internal fun LocalClockRow(
|
||||
// "Local time · 7 hours ahead": the label is what tells it apart from a
|
||||
// saved city, so it leads the summary rather than hiding in a glyph.
|
||||
val summary = stringResource(R.string.world_clock_summary, stringResource(R.string.world_clock_local), summaryFor(row))
|
||||
// Only built for a zone the device lists, so the time is always there.
|
||||
val time = row.time?.let(formatTime).orEmpty()
|
||||
val description = stringResource(R.string.world_clock_row_description, row.name, time, summary)
|
||||
val time = row.time?.let(formatTime)
|
||||
// `LocalClock` only offers a zone the device lists, so the time is there;
|
||||
// the fallback keeps TalkBack from reading an empty slot if that changes.
|
||||
val description = if (time != null) {
|
||||
stringResource(R.string.world_clock_row_description, row.name, time, summary)
|
||||
} else {
|
||||
stringResource(R.string.world_clock_row_description_unknown, row.name, summary)
|
||||
}
|
||||
|
||||
GroupedSurface(
|
||||
position = Position.Alone,
|
||||
|
||||
@@ -100,7 +100,11 @@ internal object WorldClockRows {
|
||||
/**
|
||||
* The automatic local clock's row for [deviceZone], read against home like
|
||||
* any other row. Its [WorldClockRowState.id] is [LOCAL_ROW_ID], which no
|
||||
* stored row can have, and it never carries a user label.
|
||||
* stored row can have.
|
||||
*
|
||||
* A device set by UTC offset rather than by city (Android 13+ offers it)
|
||||
* reports an id like `Etc/GMT-9`, whose last segment is no city and has
|
||||
* its sign reversed, so such a row is named by its offset ("GMT+9").
|
||||
*/
|
||||
fun local(
|
||||
deviceZone: ZoneId,
|
||||
@@ -108,13 +112,20 @@ internal object WorldClockRows {
|
||||
now: Instant,
|
||||
entries: Map<String, ZoneEntry>,
|
||||
zoneNames: Map<String, String?>,
|
||||
): WorldClockRowState = from(
|
||||
clocks = listOf(WorldClock(id = LOCAL_ROW_ID, zoneId = deviceZone.id, label = null, sortOrder = 0)),
|
||||
homeZone = homeZone,
|
||||
now = now,
|
||||
entries = entries,
|
||||
zoneNames = zoneNames,
|
||||
).single()
|
||||
): WorldClockRowState {
|
||||
val name = if (ZoneCatalog.isCityId(deviceZone.id)) {
|
||||
null
|
||||
} else {
|
||||
ZoneOffsetFormat.gmtLabel(ZoneComparisons.utcOffsetMinutes(deviceZone, now))
|
||||
}
|
||||
return from(
|
||||
clocks = listOf(WorldClock(id = LOCAL_ROW_ID, zoneId = deviceZone.id, label = name, sortOrder = 0)),
|
||||
homeZone = homeZone,
|
||||
now = now,
|
||||
entries = entries,
|
||||
zoneNames = zoneNames,
|
||||
).single()
|
||||
}
|
||||
|
||||
/** Room ids start at 1, so a row with this id is never a stored one. */
|
||||
const val LOCAL_ROW_ID: Long = -1L
|
||||
|
||||
@@ -38,8 +38,9 @@ internal class WorldClockSource @Inject constructor(
|
||||
private val storedHomeZoneId: Flow<String?> =
|
||||
settings.defaults.map { it.homeZoneId }.distinctUntilChanged()
|
||||
|
||||
private val automaticLocalClock: Flow<Boolean> =
|
||||
settings.defaults.map { it.automaticLocalClock }.distinctUntilChanged()
|
||||
/** The two settings the tab reads, from one subscription to the defaults. */
|
||||
private val tabSettings: Flow<Pair<String?, Boolean>> =
|
||||
settings.defaults.map { it.homeZoneId to it.automaticLocalClock }.distinctUntilChanged()
|
||||
|
||||
/**
|
||||
* The tab's state. Both the instant and the device zone are re-read on
|
||||
@@ -55,8 +56,9 @@ internal class WorldClockSource @Inject constructor(
|
||||
*/
|
||||
@OptIn(ExperimentalCoroutinesApi::class)
|
||||
fun state(): Flow<WorldClockUiState.Loaded> =
|
||||
combine(worldClocks.worldClocks(), storedHomeZoneId, automaticLocalClock, ::Triple)
|
||||
.flatMapLatest { (clocks, storedHome, localClock) ->
|
||||
combine(worldClocks.worldClocks(), tabSettings, ::Pair)
|
||||
.flatMapLatest { (clocks, tab) ->
|
||||
val (storedHome, localClock) = tab
|
||||
ticker.ticks(WorldClockDefaults.Tick).map { build(clocks, storedHome, localClock) }
|
||||
}
|
||||
.distinctUntilChanged()
|
||||
@@ -111,14 +113,15 @@ internal class WorldClockSource @Inject constructor(
|
||||
val now = wallClock.now()
|
||||
val deviceZone = zones.current()
|
||||
val homeZone = HomeZone.resolve(storedHome, deviceZone)
|
||||
val savedZoneIds = clocks.map { it.zoneId }
|
||||
val localZone = LocalClock.zoneToShow(
|
||||
enabled = localClock,
|
||||
storedHomeZoneId = storedHome,
|
||||
homeZone = homeZone,
|
||||
deviceZone = deviceZone,
|
||||
savedZoneIds = clocks.map { it.zoneId },
|
||||
savedZoneIds = savedZoneIds,
|
||||
at = now,
|
||||
)
|
||||
val zoneIds = (clocks.map { it.zoneId } + homeZone.id + listOfNotNull(localZone?.id)).distinct()
|
||||
val zoneIds = (savedZoneIds + homeZone.id + listOfNotNull(localZone?.id)).distinct()
|
||||
// Both memos live inside the directory, keyed on the locale it reports:
|
||||
// a cache here would survive a per-app language change and keep every
|
||||
// city in the old language while the ICU zone name switched. Both are
|
||||
|
||||
@@ -25,7 +25,7 @@ class LocalClockTest {
|
||||
saved: List<String> = emptyList(),
|
||||
enabled: Boolean = true,
|
||||
at: Instant = july,
|
||||
): ZoneId? = LocalClock.zoneToShow(enabled, storedHome, device, saved, at)
|
||||
): ZoneId? = LocalClock.zoneToShow(enabled, HomeZone.resolve(storedHome, device), device, saved, at)
|
||||
|
||||
@Test
|
||||
fun `a device away from a stored home is shown`() {
|
||||
|
||||
@@ -224,4 +224,12 @@ class ZoneCatalogTest {
|
||||
|
||||
assertThat(picked).containsExactly("Europe/Kyiv")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a city id is a region id or UTC, never an offset or abbreviation`() {
|
||||
assertThat(ZoneCatalog.isCityId("Asia/Tokyo")).isTrue()
|
||||
assertThat(ZoneCatalog.isCityId("UTC")).isTrue()
|
||||
assertThat(ZoneCatalog.isCityId("Etc/GMT-9")).isFalse()
|
||||
assertThat(ZoneCatalog.isCityId("CET")).isFalse()
|
||||
}
|
||||
}
|
||||
@@ -256,4 +256,13 @@ class WorldClockRowsTest {
|
||||
assertThat(local.time).isEqualTo(LocalTime.of(7, 0))
|
||||
assertThat(local.known).isTrue()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a device set by UTC offset names the local row by its real offset`() {
|
||||
// Etc/GMT-9 is UTC+9: the id's sign is reversed, so its last segment is no name.
|
||||
val local = WorldClockRows.local(ZoneId.of("Etc/GMT-9"), BERLIN, january, entries, emptyMap())
|
||||
|
||||
assertThat(local.name).isEqualTo("GMT+9")
|
||||
assertThat(local.offset).isEqualTo(OffsetLabel.Ahead(8, 0))
|
||||
}
|
||||
}
|
||||
@@ -2404,7 +2404,10 @@ shows nothing, and Phoenix while home is Denver shows a row only in summer.
|
||||
The row is hidden when a saved city is already on the device's offset, since
|
||||
that city already shows the local time. Home that follows the device can never
|
||||
differ from it, so with no stored home the rule is always null, and Settings ›
|
||||
Clock says so under the switch rather than hiding it.
|
||||
Clock says so under the switch rather than hiding it. A device set by UTC
|
||||
offset rather than by city reports an id like `Etc/GMT-9`, whose last segment
|
||||
has its sign reversed, so that row is named by its real offset ("GMT+9")
|
||||
instead (`ZoneCatalog.isCityId`).
|
||||
|
||||
The row is not stored. It is `WorldClockUiState.Loaded.local`, built per tick
|
||||
by the source from the same `zones.current()` read the face uses, so a
|
||||
|
||||
Reference in new issue
Block a user