From 3f19708ae69e104e22b76dca0d17d28fe07b2885 Mon Sep 17 00:00:00 2001 From: Ellen Poe Date: Thu, 20 Nov 2025 13:25:44 -0800 Subject: [PATCH 1/3] feat: opening hours for today only --- cardinal-android/app/build.gradle.kts | 1 + .../maps/cardinal/data/LocationRepository.kt | 4 +- .../java/earth/maps/cardinal/data/Place.kt | 1 + .../maps/cardinal/data/room/AppDatabase.kt | 11 +- .../maps/cardinal/data/room/SavedPlace.kt | 3 + .../earth/maps/cardinal/ui/core/AppContent.kt | 2 +- .../maps/cardinal/ui/place/PlaceCardScreen.kt | 140 +++++++++++++++++- .../app/src/main/res/values/strings.xml | 5 + cardinal-android/gradle/libs.versions.toml | 2 + 9 files changed, 165 insertions(+), 4 deletions(-) diff --git a/cardinal-android/app/build.gradle.kts b/cardinal-android/app/build.gradle.kts index 5b83551..cc66aac 100644 --- a/cardinal-android/app/build.gradle.kts +++ b/cardinal-android/app/build.gradle.kts @@ -188,6 +188,7 @@ dependencies { implementation(libs.androidx.core.ktx) implementation(libs.androidx.lifecycle.runtime.ktx) implementation(libs.androidx.activity.compose) + implementation(libs.openinghoursparser) implementation(platform(libs.androidx.compose.bom)) implementation(libs.androidx.compose.ui) implementation(libs.androidx.compose.ui.graphics) diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt index 7679f1d..72a90e2 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/LocationRepository.kt @@ -459,6 +459,7 @@ class LocationRepository @Inject constructor( } fun createSearchResultPlace(result: GeocodeResult): Place { + val openingHours = result.properties["opening_hours"] return Place( name = result.displayName, description = mapOsmTagsToDescription(result.properties), @@ -467,7 +468,8 @@ class LocationRepository @Inject constructor( latitude = result.latitude, longitude = result.longitude, ), - address = result.address + address = result.address, + openingHours = openingHours, ) } diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/Place.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/Place.kt index 372826b..e57db4c 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/Place.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/Place.kt @@ -27,6 +27,7 @@ data class Place( val icon: String = "place", val latLng: LatLng, val address: Address? = null, + val openingHours: String? = null, val isMyLocation: Boolean = false, val isTransitStop: Boolean = false, val transitStopId: String? = null, diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/AppDatabase.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/AppDatabase.kt index 982b335..0cceb10 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/AppDatabase.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/AppDatabase.kt @@ -29,7 +29,7 @@ import earth.maps.cardinal.data.DownloadStatusConverter @Database( entities = [OfflineArea::class, RoutingProfile::class, DownloadedTile::class, SavedList::class, SavedPlace::class, ListItem::class, RecentSearch::class], - version = 11, + version = 12, exportSchema = false ) @TypeConverters(TileTypeConverter::class, DownloadStatusConverter::class, ItemTypeConverter::class) @@ -216,6 +216,14 @@ abstract class AppDatabase : RoomDatabase() { } } + private val MIGRATION_11_12 = object : Migration(11, 12) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL( + "ALTER TABLE saved_places ADD COLUMN openingHours TEXT" + ) + } + } + fun getDatabase(context: Context): AppDatabase { return INSTANCE ?: synchronized(this) { val instance = Room.databaseBuilder( @@ -230,6 +238,7 @@ abstract class AppDatabase : RoomDatabase() { MIGRATION_8_9, MIGRATION_9_10, MIGRATION_10_11, + MIGRATION_11_12, ).build() INSTANCE = instance instance diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt index e946282..6663701 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/data/room/SavedPlace.kt @@ -43,6 +43,8 @@ data class SavedPlace( val postcode: String? = null, val country: String? = null, val countryCode: String? = null, + // Misc fields + val openingHours: String? = null, val isTransitStop: Boolean = false, val transitStopId: String? = null, val createdAt: Long, @@ -70,6 +72,7 @@ data class SavedPlace( postcode = place.address?.postcode, country = place.address?.country, countryCode = place.address?.countryCode, + openingHours = place.openingHours, isTransitStop = place.isTransitStop, transitStopId = place.transitStopId, createdAt = timestamp, diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/core/AppContent.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/core/AppContent.kt index 31077ea..c9a06cc 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/core/AppContent.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/core/AppContent.kt @@ -769,7 +769,7 @@ private fun PlaceCardRoute( NavigationUtils.navigate( navController, Screen.Directions(fromPlace = null, toPlace = place) ) - }, onPeekHeightChange = { + }, appPreferences = appPreferenceRepository, onPeekHeightChange = { if (topOfBackStack == backStackEntry) { state.peekHeight = it } diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardScreen.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardScreen.kt index f9650c9..991cc1d 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardScreen.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardScreen.kt @@ -18,6 +18,7 @@ package earth.maps.cardinal.ui.place +import android.util.Log import androidx.activity.compose.BackHandler import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row @@ -38,6 +39,7 @@ import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -45,6 +47,7 @@ import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.graphics.Color import androidx.compose.ui.layout.onGloballyPositioned import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.res.dimensionResource @@ -54,25 +57,160 @@ import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import androidx.hilt.navigation.compose.hiltViewModel +import ch.poole.openinghoursparser.OpeningHoursParseException +import ch.poole.openinghoursparser.OpeningHoursParser +import ch.poole.openinghoursparser.WeekDayRange import earth.maps.cardinal.R.dimen import earth.maps.cardinal.R.drawable import earth.maps.cardinal.R.string import earth.maps.cardinal.data.AddressFormatter +import earth.maps.cardinal.data.AppPreferenceRepository +import earth.maps.cardinal.data.AppPreferences import earth.maps.cardinal.data.Place import earth.maps.cardinal.data.format +import earth.maps.cardinal.data.formatTime import kotlinx.coroutines.launch +import kotlinx.datetime.DayOfWeek +import kotlinx.datetime.LocalDateTime +import kotlinx.datetime.TimeZone +import kotlinx.datetime.atTime +import kotlinx.datetime.toInstant +import kotlinx.datetime.toLocalDateTime +import java.io.ByteArrayInputStream +import kotlin.time.Clock +import kotlin.time.Duration.Companion.minutes +import kotlin.time.ExperimentalTime + +fun weekdayRangeIncludesDay(range: WeekDayRange, day: DayOfWeek): Boolean { + if (range.startDay != null && range.startDay.ordinal == day.ordinal) { + return true + } + else if (range.startDay == null || range.endDay == null) { + return false + } + if (range.endDay < range.startDay) { + for (ord in 0..range.endDay.ordinal) { + if (ord == day.ordinal) { + return true + } + } + for (ord in range.startDay.ordinal..6) { + if (ord == day.ordinal) { + return true + } + } + } else { + for (ord in range.startDay.ordinal..range.endDay.ordinal) { + if (ord == day.ordinal) { + return true + } + } + } + return false +} + +@OptIn(ExperimentalTime::class) +@Composable +fun OpenClosed(place: Place, now: LocalDateTime, timeZone: TimeZone, use24HourFormat: Boolean) { + place.openingHours?.let { openingHours -> + val today = now.dayOfWeek + val parser = OpeningHoursParser(ByteArrayInputStream(openingHours.toByteArray(charset = Charsets.UTF_8))) + val rules = try { + parser.rules(false, false) + } catch (e: OpeningHoursParseException) { + Log.e("PlaceCardScreen", "Failed to parse opening hours", e) + return + } + + val timeOfDay = now.time + // My rationale for doing the cavewoman-brained solution instead of handling DST and other issues is that the opening hours + // are going to be outdated far more often than any other failure mode, and we can get better ROI by spending time on other + // areas of the app. + + // The opening hours parser gives us times in minutes from the beginning of the day, so we want the current time in that format. + val minutesFromMidnight = timeOfDay.minute + timeOfDay.hour * 60 + + // This logic gets complicated due to things like lunch breaks, but the user wants one of two things, depending on whether the place is open currently: + // If the place is open: + // - When does it close? Does it reopen after that, and if so, when? + // If the place is closed: + // - When does it open? + + // Whether the place is currently open. + var isOpen = false + // The first time today, not before the current time, that the POI will open. This may or may not be the first or last time the POI opens today. + var openingTimeToday: Int? = null + // The first time today, not before the current time, that the POI will close. This may or may not be the first or last time that the POI closes today. + var closingTimeToday: Int? = null + + // Iterate through the rules in this opening hours expression and find the ones that apply to today (defined above). + for (rule in rules) { + val days = rule.days + val times = rule.times + if (days == null || times == null) { + continue + } + for (dayRule in days) { + if (weekdayRangeIncludesDay(dayRule, today)) { + // Look for opening and closing times today. + for (timeRule in times) { + if (timeRule.start < minutesFromMidnight && timeRule.end > minutesFromMidnight) { + isOpen = true + if (closingTimeToday == null || timeRule.end < closingTimeToday) { + closingTimeToday = timeRule.end + } + } else if (openingTimeToday == null || timeRule.start < openingTimeToday) { + openingTimeToday = timeRule.start + } + } + } + } + } + + val openingInstantToday = openingTimeToday?.let { openingTime -> + now.date.atTime(0, 0).toInstant(timeZone = timeZone).plus(openingTime.minutes) + } + val closingInstantToday = closingTimeToday?.let { closingTime -> + now.date.atTime(0, 0).toInstant(timeZone = timeZone).plus(closingTime.minutes) + } + Row { + if (isOpen) { + Text(modifier = Modifier.padding(bottom = 8.dp), color = Color.Green, text = stringResource(string.opening_hours_open)) + closingInstantToday?.let { closingInstant -> + Spacer(modifier = Modifier.width(4.dp)) + Text(modifier = Modifier.padding(bottom = 8.dp), text = stringResource(string.opening_hours_closes_at, closingInstant.toString().formatTime(use24HourFormat))) + } + openingInstantToday?.let { openingInstant -> + Spacer(modifier = Modifier.width(4.dp)) + Text(modifier = Modifier.padding(bottom = 8.dp), text = stringResource(string.opening_hours_opens_again_at, openingInstant.toString().formatTime(use24HourFormat))) + } + } else { + Text(modifier = Modifier.padding(bottom = 8.dp), color = Color.Red, text = stringResource(string.opening_hours_closed)) + openingInstantToday?.let { openingInstant -> + Spacer(modifier = Modifier.width(4.dp)) + Text(modifier = Modifier.padding(bottom = 8.dp), text = stringResource(string.opening_hours_opens_at, openingInstant.toString().formatTime(use24HourFormat))) + } + } + } + } +} + +@OptIn(ExperimentalTime::class) @Composable fun PlaceCardScreen( place: Place, onBack: () -> Unit, onGetDirections: (Place) -> Unit, viewModel: PlaceCardViewModel, + appPreferences: AppPreferenceRepository, onPeekHeightChange: (dp: Dp) -> Unit ) { val density = LocalDensity.current val addressFormatter = remember { AddressFormatter() } + val use24HourFormat by appPreferences.use24HourFormat.collectAsState() + // Check if place is saved when screen is opened LaunchedEffect(place) { viewModel.checkIfPlaceIsSaved(place) @@ -90,7 +228,6 @@ fun PlaceCardScreen( // Place details content Column { - Column( modifier = Modifier .fillMaxWidth() @@ -106,6 +243,7 @@ fun PlaceCardScreen( ) { PlaceHeader(displayedPlace) PlaceAddress(displayedPlace, addressFormatter) + OpenClosed(displayedPlace, Clock.System.now().toLocalDateTime(timeZone = TimeZone.currentSystemDefault()), TimeZone.currentSystemDefault(), use24HourFormat) PlaceActions( displayedPlace, viewModel, diff --git a/cardinal-android/app/src/main/res/values/strings.xml b/cardinal-android/app/src/main/res/values/strings.xml index f99c83c..bf13169 100644 --- a/cardinal-android/app/src/main/res/values/strings.xml +++ b/cardinal-android/app/src/main/res/values/strings.xml @@ -265,4 +265,9 @@ Entertainment Nightlife Cannot paste a list into itself or one of its sublists + Open. + Closed. + Closes %1$s. + Opens again %1$s. + Opens %1$s. diff --git a/cardinal-android/gradle/libs.versions.toml b/cardinal-android/gradle/libs.versions.toml index 10d20c0..32d9c35 100644 --- a/cardinal-android/gradle/libs.versions.toml +++ b/cardinal-android/gradle/libs.versions.toml @@ -33,6 +33,7 @@ detekt = "2.0.0-alpha.0" mockk = "1.14.6" kotlinxCoroutinesTest = "1.10.2" hiltAndroidTesting = "2.57.1" +openinghoursparser = "0.28.2" [libraries] androidaddressformatter = { module = "com.github.woheller69:AndroidAddressFormatter", version.ref = "androidaddressformatter" } @@ -84,6 +85,7 @@ okhttp3 = { group = "com.squareup.okhttp3", name = "okhttp", version.ref = "okht mockk = { module = "io.mockk:mockk", version.ref = "mockk" } kotlinx-coroutines-test = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-test", version.ref = "kotlinxCoroutinesTest" } hilt-android-testing = { group = "com.google.dagger", name = "hilt-android-testing", version.ref = "hiltAndroidTesting" } +openinghoursparser = { module = "ch.poole:OpeningHoursParser", version.ref = "openinghoursparser" } [plugins] android-application = { id = "com.android.application", version.ref = "agp" } -- GitLab From 114031395be8a700e5c2e3ae3059ad5ddafffb0c Mon Sep 17 00:00:00 2001 From: Ellen Poe Date: Thu, 20 Nov 2025 13:48:45 -0800 Subject: [PATCH 2/3] feat: opening hours table for next 7 days --- .../maps/cardinal/ui/place/PlaceCardScreen.kt | 352 +++++++++++++++--- .../app/src/main/res/values/strings.xml | 14 + 2 files changed, 320 insertions(+), 46 deletions(-) diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardScreen.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardScreen.kt index 991cc1d..4d71c12 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardScreen.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardScreen.kt @@ -27,10 +27,15 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width +import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.height import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.verticalScroll import androidx.compose.material3.AlertDialog import androidx.compose.material3.Button +import androidx.compose.material3.Card +import androidx.compose.material3.CardDefaults import androidx.compose.material3.DividerDefaults import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.Icon @@ -65,7 +70,6 @@ import earth.maps.cardinal.R.drawable import earth.maps.cardinal.R.string import earth.maps.cardinal.data.AddressFormatter import earth.maps.cardinal.data.AppPreferenceRepository -import earth.maps.cardinal.data.AppPreferences import earth.maps.cardinal.data.Place import earth.maps.cardinal.data.format import earth.maps.cardinal.data.formatTime @@ -77,32 +81,134 @@ import kotlinx.datetime.atTime import kotlinx.datetime.toInstant import kotlinx.datetime.toLocalDateTime import java.io.ByteArrayInputStream +import java.util.Locale import kotlin.time.Clock import kotlin.time.Duration.Companion.minutes import kotlin.time.ExperimentalTime +// Data class to hold opening hours information for each day +data class DayOpeningHours( + val dayOfWeek: Int, + val dayName: String, + val timeRanges: List, + val isToday: Boolean +) -fun weekdayRangeIncludesDay(range: WeekDayRange, day: DayOfWeek): Boolean { - if (range.startDay != null && range.startDay.ordinal == day.ordinal) { - return true +@Composable +fun getDayName(dayOfWeek: Int): String { + return when (dayOfWeek) { + DayOfWeek.MONDAY.ordinal -> stringResource(string.day_monday) + DayOfWeek.TUESDAY.ordinal -> stringResource(string.day_tuesday) + DayOfWeek.WEDNESDAY.ordinal -> stringResource(string.day_wednesday) + DayOfWeek.THURSDAY.ordinal -> stringResource(string.day_thursday) + DayOfWeek.FRIDAY.ordinal -> stringResource(string.day_friday) + DayOfWeek.SATURDAY.ordinal -> stringResource(string.day_saturday) + DayOfWeek.SUNDAY.ordinal -> stringResource(string.day_sunday) + else -> "Unknown day" + } +} + +// Helper function to format minutes to time string +fun formatMinutesToTime(minutes: Int, use24HourFormat: Boolean): String { + val hours = minutes / 60 + val mins = minutes % 60 + return if (use24HourFormat) { + String.format(Locale.getDefault(), "%02d:%02d", hours, mins) + } else { + val displayHour = if (hours == 0) 12 else if (hours > 12) hours - 12 else hours + val amPm = if (hours >= 12) "PM" else "AM" + String.format(Locale.getDefault(), "%d:%02d %s", displayHour, mins, amPm) } - else if (range.startDay == null || range.endDay == null) { +} + +// Helper function to get opening hours for a specific day +fun getOpeningHoursForDay( + rules: List, + dayOfWeek: Int, + use24HourFormat: Boolean +): List { + val timeRanges = mutableListOf() + + for (rule in rules) { + val days = rule.days + val times = rule.times + if (days == null || times == null) { + continue + } + + for (dayRule in days) { + if (weekdayRangeIncludesDay(dayRule, dayOfWeek)) { + // Collect all time ranges for this day + for (timeRule in times) { + val startTime = formatMinutesToTime(timeRule.start, use24HourFormat) + val endTime = formatMinutesToTime(timeRule.end, use24HourFormat) + timeRanges.add("$startTime - $endTime") + } + } + } + } + + return timeRanges.distinct() +} + +// Helper function to get opening hours for the next 7 days +@Composable +fun getOpeningHoursForNext7Days( + openingHours: String, + now: LocalDateTime, + use24HourFormat: Boolean +): List { + val parser = + OpeningHoursParser(ByteArrayInputStream(openingHours.toByteArray(charset = Charsets.UTF_8))) + val rules = try { + parser.rules(false, false) + } catch (e: OpeningHoursParseException) { + Log.e("PlaceCardScreen", "Failed to parse opening hours", e) + return emptyList() + } + + val dayOpeningHours = mutableListOf() + val today = now.dayOfWeek + + // Get opening hours for today and next 6 days + for (i in 0..6) { + val targetDay = (today.ordinal + i) % 7 + val dayName = if (i == 0) stringResource(string.day_today) else getDayName(targetDay) + val timeRanges = getOpeningHoursForDay(rules, targetDay, use24HourFormat) + + dayOpeningHours.add( + DayOpeningHours( + dayOfWeek = targetDay, + dayName = dayName, + timeRanges = timeRanges, + isToday = i == 0 + ) + ) + } + + return dayOpeningHours +} + +fun weekdayRangeIncludesDay(range: WeekDayRange, day: Int): Boolean { + if (range.startDay != null && range.startDay.ordinal == day) { + return true + } else if (range.startDay == null || range.endDay == null) { return false } if (range.endDay < range.startDay) { for (ord in 0..range.endDay.ordinal) { - if (ord == day.ordinal) { + if (ord == day) { return true } } for (ord in range.startDay.ordinal..6) { - if (ord == day.ordinal) { + if (ord == day) { return true } } } else { for (ord in range.startDay.ordinal..range.endDay.ordinal) { - if (ord == day.ordinal) { + if (ord == day) { return true } } @@ -112,39 +218,183 @@ fun weekdayRangeIncludesDay(range: WeekDayRange, day: DayOfWeek): Boolean { @OptIn(ExperimentalTime::class) @Composable -fun OpenClosed(place: Place, now: LocalDateTime, timeZone: TimeZone, use24HourFormat: Boolean) { +fun ExpandableOpeningHours( + place: Place, + now: LocalDateTime, + timeZone: TimeZone, + use24HourFormat: Boolean +) { + var expanded by remember { mutableStateOf(false) } + val openingHoursData = place.openingHours?.let { openingHours -> + getOpeningHoursForNext7Days(openingHours, now, use24HourFormat) + } ?: return + + // Get current status for collapsed view + val currentStatus = getCurrentOpeningStatus(place, now, timeZone, use24HourFormat) + + Card( + modifier = Modifier + .fillMaxWidth() + .padding(bottom = 8.dp) + .clickable { expanded = !expanded }, + elevation = CardDefaults.cardElevation(defaultElevation = 2.dp) + ) { + Column( + modifier = Modifier + .fillMaxWidth() + .padding(16.dp) + ) { + // Header with current status and expand/collapse icon + Row( + modifier = Modifier + .fillMaxWidth() + .clickable { expanded = !expanded }, + horizontalArrangement = Arrangement.SpaceBetween, + verticalAlignment = Alignment.CenterVertically + ) { + Column( + modifier = Modifier.weight(1f) + ) { + Text( + text = stringResource(string.opening_hours_title), + style = MaterialTheme.typography.titleMedium, + fontWeight = FontWeight.Medium + ) + + currentStatus?.let { status -> + Row( + modifier = Modifier.padding(top = 4.dp), + verticalAlignment = Alignment.CenterVertically + ) { + Text( + text = status.statusText, + color = status.statusColor, + style = MaterialTheme.typography.bodyMedium + ) + status.nextTimeText?.let { nextTime -> + Spacer(modifier = Modifier.width(4.dp)) + Text( + text = nextTime, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant + ) + } + } + } + } + + Icon( + painter = painterResource( + if (expanded) drawable.ic_arrow_down else drawable.ic_arrow_down + ), + contentDescription = stringResource( + if (expanded) string.content_description_collapse_opening_hours + else string.content_description_expand_opening_hours + ), + modifier = Modifier.size(24.dp) + ) + } + + // Expanded content with table + if (expanded && openingHoursData.isNotEmpty()) { + Spacer(modifier = Modifier.height(8.dp)) + + // Table header + Row( + modifier = Modifier.fillMaxWidth() + ) { + Text( + text = "Day", + modifier = Modifier.weight(1f), + style = MaterialTheme.typography.bodySmall, + fontWeight = FontWeight.Medium, + color = MaterialTheme.colorScheme.onSurfaceVariant + ) + Text( + text = "Hours", + modifier = Modifier.weight(2f), + style = MaterialTheme.typography.bodySmall, + fontWeight = FontWeight.Medium, + color = MaterialTheme.colorScheme.onSurfaceVariant + ) + } + + HorizontalDivider( + modifier = Modifier.padding(vertical = 4.dp), + color = MaterialTheme.colorScheme.outlineVariant + ) + + // Table rows for each day + openingHoursData.forEach { dayHours -> + Row( + modifier = Modifier + .fillMaxWidth() + .padding(vertical = 4.dp), + verticalAlignment = Alignment.CenterVertically + ) { + Text( + text = dayHours.dayName, + modifier = Modifier.weight(1f), + style = MaterialTheme.typography.bodyMedium, + fontWeight = if (dayHours.isToday) FontWeight.Bold else FontWeight.Normal, + color = MaterialTheme.colorScheme.onSurface + ) + + Text( + text = if (dayHours.timeRanges.isEmpty()) { + stringResource(string.opening_hours_closed_all_day) + } else { + dayHours.timeRanges.joinToString(", ") + }, + modifier = Modifier.weight(2f), + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurface + ) + } + + if (dayHours != openingHoursData.last()) { + HorizontalDivider( + color = MaterialTheme.colorScheme.outlineVariant + ) + } + } + } + } + } +} + +data class OpeningStatus( + val statusText: String, + val statusColor: Color, + val nextTimeText: String? +) + +@OptIn(ExperimentalTime::class) +@Composable +fun getCurrentOpeningStatus( + place: Place, + now: LocalDateTime, + timeZone: TimeZone, + use24HourFormat: Boolean +): OpeningStatus? { place.openingHours?.let { openingHours -> val today = now.dayOfWeek - val parser = OpeningHoursParser(ByteArrayInputStream(openingHours.toByteArray(charset = Charsets.UTF_8))) + val parser = + OpeningHoursParser(ByteArrayInputStream(openingHours.toByteArray(charset = Charsets.UTF_8))) val rules = try { parser.rules(false, false) } catch (e: OpeningHoursParseException) { Log.e("PlaceCardScreen", "Failed to parse opening hours", e) - return + return null } val timeOfDay = now.time - // My rationale for doing the cavewoman-brained solution instead of handling DST and other issues is that the opening hours - // are going to be outdated far more often than any other failure mode, and we can get better ROI by spending time on other - // areas of the app. - - // The opening hours parser gives us times in minutes from the beginning of the day, so we want the current time in that format. val minutesFromMidnight = timeOfDay.minute + timeOfDay.hour * 60 - // This logic gets complicated due to things like lunch breaks, but the user wants one of two things, depending on whether the place is open currently: - // If the place is open: - // - When does it close? Does it reopen after that, and if so, when? - // If the place is closed: - // - When does it open? - - // Whether the place is currently open. var isOpen = false - // The first time today, not before the current time, that the POI will open. This may or may not be the first or last time the POI opens today. var openingTimeToday: Int? = null - // The first time today, not before the current time, that the POI will close. This may or may not be the first or last time that the POI closes today. var closingTimeToday: Int? = null - // Iterate through the rules in this opening hours expression and find the ones that apply to today (defined above). for (rule in rules) { val days = rule.days val times = rule.times @@ -152,8 +402,7 @@ fun OpenClosed(place: Place, now: LocalDateTime, timeZone: TimeZone, use24HourFo continue } for (dayRule in days) { - if (weekdayRangeIncludesDay(dayRule, today)) { - // Look for opening and closing times today. + if (weekdayRangeIncludesDay(dayRule, today.ordinal)) { for (timeRule in times) { if (timeRule.start < minutesFromMidnight && timeRule.end > minutesFromMidnight) { isOpen = true @@ -174,26 +423,32 @@ fun OpenClosed(place: Place, now: LocalDateTime, timeZone: TimeZone, use24HourFo val closingInstantToday = closingTimeToday?.let { closingTime -> now.date.atTime(0, 0).toInstant(timeZone = timeZone).plus(closingTime.minutes) } - Row { - if (isOpen) { - Text(modifier = Modifier.padding(bottom = 8.dp), color = Color.Green, text = stringResource(string.opening_hours_open)) - closingInstantToday?.let { closingInstant -> - Spacer(modifier = Modifier.width(4.dp)) - Text(modifier = Modifier.padding(bottom = 8.dp), text = stringResource(string.opening_hours_closes_at, closingInstant.toString().formatTime(use24HourFormat))) - } - openingInstantToday?.let { openingInstant -> - Spacer(modifier = Modifier.width(4.dp)) - Text(modifier = Modifier.padding(bottom = 8.dp), text = stringResource(string.opening_hours_opens_again_at, openingInstant.toString().formatTime(use24HourFormat))) + + return if (isOpen) { + OpeningStatus( + statusText = stringResource(string.opening_hours_open), + statusColor = Color.Green, + nextTimeText = closingInstantToday?.let { closingInstant -> + stringResource( + string.opening_hours_closes_at, + closingInstant.toString().formatTime(use24HourFormat) + ) } - } else { - Text(modifier = Modifier.padding(bottom = 8.dp), color = Color.Red, text = stringResource(string.opening_hours_closed)) - openingInstantToday?.let { openingInstant -> - Spacer(modifier = Modifier.width(4.dp)) - Text(modifier = Modifier.padding(bottom = 8.dp), text = stringResource(string.opening_hours_opens_at, openingInstant.toString().formatTime(use24HourFormat))) + ) + } else { + OpeningStatus( + statusText = stringResource(string.opening_hours_closed), + statusColor = Color.Red, + nextTimeText = openingInstantToday?.let { openingInstant -> + stringResource( + string.opening_hours_opens_at, + openingInstant.toString().formatTime(use24HourFormat) + ) } - } + ) } } + return null } @OptIn(ExperimentalTime::class) @@ -243,7 +498,12 @@ fun PlaceCardScreen( ) { PlaceHeader(displayedPlace) PlaceAddress(displayedPlace, addressFormatter) - OpenClosed(displayedPlace, Clock.System.now().toLocalDateTime(timeZone = TimeZone.currentSystemDefault()), TimeZone.currentSystemDefault(), use24HourFormat) + ExpandableOpeningHours( + displayedPlace, + Clock.System.now().toLocalDateTime(timeZone = TimeZone.currentSystemDefault()), + TimeZone.currentSystemDefault(), + use24HourFormat + ) PlaceActions( displayedPlace, viewModel, @@ -296,7 +556,7 @@ private fun PlaceAddress(displayedPlace: Place, addressFormatter: AddressFormatt Row( modifier = Modifier .fillMaxWidth() - .padding(vertical = 8.dp), + .padding(top = 8.dp), verticalAlignment = Alignment.CenterVertically ) { Icon( diff --git a/cardinal-android/app/src/main/res/values/strings.xml b/cardinal-android/app/src/main/res/values/strings.xml index bf13169..60dcb63 100644 --- a/cardinal-android/app/src/main/res/values/strings.xml +++ b/cardinal-android/app/src/main/res/values/strings.xml @@ -270,4 +270,18 @@ Closes %1$s. Opens again %1$s. Opens %1$s. + Opening Hours + Closed + Expand opening hours + Collapse opening hours + + Monday + Tuesday + Wednesday + Thursday + Friday + Saturday + Sunday + + Today -- GitLab From ac415251f8a5841e58f9b6068548af1428216e21 Mon Sep 17 00:00:00 2001 From: Ellen Poe Date: Fri, 21 Nov 2025 11:18:37 -0800 Subject: [PATCH 3/3] refactor: reduce cognitive complexity in opening hours code --- .../maps/cardinal/ui/place/PlaceCardScreen.kt | 312 ++++++++++-------- .../app/src/main/res/values/strings.xml | 2 + 2 files changed, 174 insertions(+), 140 deletions(-) diff --git a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardScreen.kt b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardScreen.kt index 4d71c12..bde8370 100644 --- a/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardScreen.kt +++ b/cardinal-android/app/src/main/java/earth/maps/cardinal/ui/place/PlaceCardScreen.kt @@ -64,6 +64,7 @@ import androidx.compose.ui.unit.dp import androidx.hilt.navigation.compose.hiltViewModel import ch.poole.openinghoursparser.OpeningHoursParseException import ch.poole.openinghoursparser.OpeningHoursParser +import ch.poole.openinghoursparser.Rule import ch.poole.openinghoursparser.WeekDayRange import earth.maps.cardinal.R.dimen import earth.maps.cardinal.R.drawable @@ -189,31 +190,21 @@ fun getOpeningHoursForNext7Days( return dayOpeningHours } +fun ordinalInRange(ord: Int, start: Int, end: Int): Boolean { + return ord >= start && ord <= end +} + fun weekdayRangeIncludesDay(range: WeekDayRange, day: Int): Boolean { if (range.startDay != null && range.startDay.ordinal == day) { return true } else if (range.startDay == null || range.endDay == null) { return false } - if (range.endDay < range.startDay) { - for (ord in 0..range.endDay.ordinal) { - if (ord == day) { - return true - } - } - for (ord in range.startDay.ordinal..6) { - if (ord == day) { - return true - } - } + return if (range.endDay < range.startDay) { + ordinalInRange(day, 0, range.endDay.ordinal) || ordinalInRange(day, range.startDay.ordinal, 6) } else { - for (ord in range.startDay.ordinal..range.endDay.ordinal) { - if (ord == day) { - return true - } - } + ordinalInRange(day, range.startDay.ordinal, range.endDay.ordinal) } - return false } @OptIn(ExperimentalTime::class) @@ -245,54 +236,8 @@ fun ExpandableOpeningHours( .padding(16.dp) ) { // Header with current status and expand/collapse icon - Row( - modifier = Modifier - .fillMaxWidth() - .clickable { expanded = !expanded }, - horizontalArrangement = Arrangement.SpaceBetween, - verticalAlignment = Alignment.CenterVertically - ) { - Column( - modifier = Modifier.weight(1f) - ) { - Text( - text = stringResource(string.opening_hours_title), - style = MaterialTheme.typography.titleMedium, - fontWeight = FontWeight.Medium - ) - - currentStatus?.let { status -> - Row( - modifier = Modifier.padding(top = 4.dp), - verticalAlignment = Alignment.CenterVertically - ) { - Text( - text = status.statusText, - color = status.statusColor, - style = MaterialTheme.typography.bodyMedium - ) - status.nextTimeText?.let { nextTime -> - Spacer(modifier = Modifier.width(4.dp)) - Text( - text = nextTime, - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurfaceVariant - ) - } - } - } - } - - Icon( - painter = painterResource( - if (expanded) drawable.ic_arrow_down else drawable.ic_arrow_down - ), - contentDescription = stringResource( - if (expanded) string.content_description_collapse_opening_hours - else string.content_description_expand_opening_hours - ), - modifier = Modifier.size(24.dp) - ) + OpeningHoursHeader(expanded, currentStatus) { + expanded = it } // Expanded content with table @@ -300,24 +245,7 @@ fun ExpandableOpeningHours( Spacer(modifier = Modifier.height(8.dp)) // Table header - Row( - modifier = Modifier.fillMaxWidth() - ) { - Text( - text = "Day", - modifier = Modifier.weight(1f), - style = MaterialTheme.typography.bodySmall, - fontWeight = FontWeight.Medium, - color = MaterialTheme.colorScheme.onSurfaceVariant - ) - Text( - text = "Hours", - modifier = Modifier.weight(2f), - style = MaterialTheme.typography.bodySmall, - fontWeight = FontWeight.Medium, - color = MaterialTheme.colorScheme.onSurfaceVariant - ) - } + OpeningHoursTableHeader() HorizontalDivider( modifier = Modifier.padding(vertical = 4.dp), @@ -326,31 +254,7 @@ fun ExpandableOpeningHours( // Table rows for each day openingHoursData.forEach { dayHours -> - Row( - modifier = Modifier - .fillMaxWidth() - .padding(vertical = 4.dp), - verticalAlignment = Alignment.CenterVertically - ) { - Text( - text = dayHours.dayName, - modifier = Modifier.weight(1f), - style = MaterialTheme.typography.bodyMedium, - fontWeight = if (dayHours.isToday) FontWeight.Bold else FontWeight.Normal, - color = MaterialTheme.colorScheme.onSurface - ) - - Text( - text = if (dayHours.timeRanges.isEmpty()) { - stringResource(string.opening_hours_closed_all_day) - } else { - dayHours.timeRanges.joinToString(", ") - }, - modifier = Modifier.weight(2f), - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurface - ) - } + OpeningHoursTableRow(dayHours) if (dayHours != openingHoursData.last()) { HorizontalDivider( @@ -363,7 +267,115 @@ fun ExpandableOpeningHours( } } -data class OpeningStatus( +@Composable +private fun OpeningHoursTableRow(dayHours: DayOpeningHours) { + Row( + modifier = Modifier + .fillMaxWidth() + .padding(vertical = 4.dp), + verticalAlignment = Alignment.CenterVertically + ) { + Text( + text = dayHours.dayName, + modifier = Modifier.weight(1f), + style = MaterialTheme.typography.bodyMedium, + fontWeight = if (dayHours.isToday) FontWeight.Bold else FontWeight.Normal, + color = MaterialTheme.colorScheme.onSurface + ) + + Text( + text = if (dayHours.timeRanges.isEmpty()) { + stringResource(string.opening_hours_closed_all_day) + } else { + dayHours.timeRanges.joinToString(", ") + }, + modifier = Modifier.weight(2f), + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurface + ) + } +} + +@Composable +private fun OpeningHoursTableHeader() { + Row( + modifier = Modifier.fillMaxWidth() + ) { + Text( + text = stringResource(string.opening_hours_day), + modifier = Modifier.weight(1f), + style = MaterialTheme.typography.bodySmall, + fontWeight = FontWeight.Medium, + color = MaterialTheme.colorScheme.onSurfaceVariant + ) + Text( + text = stringResource(string.opening_hours_hours), + modifier = Modifier.weight(2f), + style = MaterialTheme.typography.bodySmall, + fontWeight = FontWeight.Medium, + color = MaterialTheme.colorScheme.onSurfaceVariant + ) + } +} + +@Composable +private fun OpeningHoursHeader( + expanded: Boolean, + currentStatus: OpeningStatusDisplay?, + onExpandChanged: (Boolean) -> Unit, +) { + Row( + modifier = Modifier + .fillMaxWidth() + .clickable { onExpandChanged(!expanded) }, + horizontalArrangement = Arrangement.SpaceBetween, + verticalAlignment = Alignment.CenterVertically + ) { + Column( + modifier = Modifier.weight(1f) + ) { + Text( + text = stringResource(string.opening_hours_title), + style = MaterialTheme.typography.titleMedium, + fontWeight = FontWeight.Medium + ) + + currentStatus?.let { status -> + Row( + modifier = Modifier.padding(top = 4.dp), + verticalAlignment = Alignment.CenterVertically + ) { + Text( + text = status.statusText, + color = status.statusColor, + style = MaterialTheme.typography.bodyMedium + ) + status.nextTimeText?.let { nextTime -> + Spacer(modifier = Modifier.width(4.dp)) + Text( + text = nextTime, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant + ) + } + } + } + } + + Icon( + painter = painterResource( + drawable.ic_arrow_down + ), + contentDescription = stringResource( + if (expanded) string.content_description_collapse_opening_hours + else string.content_description_expand_opening_hours + ), + modifier = Modifier.size(24.dp) + ) + } +} + +data class OpeningStatusDisplay( val statusText: String, val statusColor: Color, val nextTimeText: String? @@ -376,7 +388,7 @@ fun getCurrentOpeningStatus( now: LocalDateTime, timeZone: TimeZone, use24HourFormat: Boolean -): OpeningStatus? { +): OpeningStatusDisplay? { place.openingHours?.let { openingHours -> val today = now.dayOfWeek val parser = @@ -391,41 +403,17 @@ fun getCurrentOpeningStatus( val timeOfDay = now.time val minutesFromMidnight = timeOfDay.minute + timeOfDay.hour * 60 - var isOpen = false - var openingTimeToday: Int? = null - var closingTimeToday: Int? = null + val openingStatus = processOpeningHoursRules(rules, today, minutesFromMidnight) - for (rule in rules) { - val days = rule.days - val times = rule.times - if (days == null || times == null) { - continue - } - for (dayRule in days) { - if (weekdayRangeIncludesDay(dayRule, today.ordinal)) { - for (timeRule in times) { - if (timeRule.start < minutesFromMidnight && timeRule.end > minutesFromMidnight) { - isOpen = true - if (closingTimeToday == null || timeRule.end < closingTimeToday) { - closingTimeToday = timeRule.end - } - } else if (openingTimeToday == null || timeRule.start < openingTimeToday) { - openingTimeToday = timeRule.start - } - } - } - } - } - - val openingInstantToday = openingTimeToday?.let { openingTime -> + val openingInstantToday = openingStatus.openingTimeToday?.let { openingTime -> now.date.atTime(0, 0).toInstant(timeZone = timeZone).plus(openingTime.minutes) } - val closingInstantToday = closingTimeToday?.let { closingTime -> + val closingInstantToday = openingStatus.closingTimeToday?.let { closingTime -> now.date.atTime(0, 0).toInstant(timeZone = timeZone).plus(closingTime.minutes) } - return if (isOpen) { - OpeningStatus( + return if (openingStatus.isOpen) { + OpeningStatusDisplay( statusText = stringResource(string.opening_hours_open), statusColor = Color.Green, nextTimeText = closingInstantToday?.let { closingInstant -> @@ -436,7 +424,7 @@ fun getCurrentOpeningStatus( } ) } else { - OpeningStatus( + OpeningStatusDisplay( statusText = stringResource(string.opening_hours_closed), statusColor = Color.Red, nextTimeText = openingInstantToday?.let { openingInstant -> @@ -451,6 +439,50 @@ fun getCurrentOpeningStatus( return null } +data class OpeningStatus( + val isOpen: Boolean, + val closingTimeToday: Int?, + val openingTimeToday: Int?, + +) + +@Suppress("CognitiveComplexMethod") +private fun processOpeningHoursRules( + rules: List, + today: DayOfWeek, + minutesFromMidnight: Int, +): OpeningStatus { + var isOpen = false + var closingTimeToday: Int? = null + var openingTimeToday: Int? = null + for (rule in rules) { + val days = rule.days + val times = rule.times + if (days == null || times == null) { + continue + } + for (dayRule in days) { + if (weekdayRangeIncludesDay(dayRule, today.ordinal)) { + for (timeRule in times) { + if (timeRule.start < minutesFromMidnight && timeRule.end > minutesFromMidnight) { + isOpen = true + if (closingTimeToday == null || timeRule.end < closingTimeToday) { + closingTimeToday = timeRule.end + } + } else if (openingTimeToday == null || timeRule.start < openingTimeToday) { + openingTimeToday = timeRule.start + } + } + } + } + } + return OpeningStatus( + isOpen, + closingTimeToday, + openingTimeToday, + ) +} + @OptIn(ExperimentalTime::class) @Composable fun PlaceCardScreen( diff --git a/cardinal-android/app/src/main/res/values/strings.xml b/cardinal-android/app/src/main/res/values/strings.xml index 60dcb63..983f34f 100644 --- a/cardinal-android/app/src/main/res/values/strings.xml +++ b/cardinal-android/app/src/main/res/values/strings.xml @@ -284,4 +284,6 @@ Sunday Today + Hours + Day -- GitLab