-
Notifications
You must be signed in to change notification settings - Fork 0
Update library UI to conform to MVP release requirements #136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -43,6 +43,7 @@ import com.cornellappdev.transit.ui.theme.robotoFamily | |||||
| import com.cornellappdev.transit.ui.viewmodels.EcosystemFavoritesUiState | ||||||
| import com.cornellappdev.transit.ui.viewmodels.FavoritesFilterSheetState | ||||||
| import com.cornellappdev.transit.ui.viewmodels.FilterState | ||||||
| import com.cornellappdev.transit.ui.viewmodels.LibraryCardUiState | ||||||
| import com.cornellappdev.transit.util.TimeUtils.isOpenAnnotatedStringFromOperatingHours | ||||||
| import com.cornellappdev.transit.util.ecosystem.capacityPercentAnnotatedString | ||||||
| import com.cornellappdev.transit.ui.viewmodels.PrinterCardUiState | ||||||
|
|
@@ -65,6 +66,7 @@ fun EcosystemBottomSheetContent( | |||||
| activeFilter: FilterState, | ||||||
| onFilterClick: (FilterState) -> Unit, | ||||||
| staticPlaces: StaticPlaces, | ||||||
| libraryCardsApiResponse: ApiResponse<List<LibraryCardUiState>>, | ||||||
| favorites: Set<Place>, | ||||||
| favoritesUiState: EcosystemFavoritesUiState, | ||||||
| modifier: Modifier = Modifier, | ||||||
|
|
@@ -119,6 +121,7 @@ fun EcosystemBottomSheetContent( | |||||
| BottomSheetFilteredContent( | ||||||
| currentFilter = activeFilter, | ||||||
| staticPlaces = staticPlaces, | ||||||
| libraryCardsApiResponse = libraryCardsApiResponse, | ||||||
| favorites = favorites, | ||||||
| favoritesUiState = favoritesUiState, | ||||||
| navigateToPlace = navigateToPlace, | ||||||
|
|
@@ -153,6 +156,7 @@ fun EcosystemBottomSheetContent( | |||||
| private fun BottomSheetFilteredContent( | ||||||
| currentFilter: FilterState, | ||||||
| staticPlaces: StaticPlaces, | ||||||
| libraryCardsApiResponse: ApiResponse<List<LibraryCardUiState>>, | ||||||
| favorites: Set<Place>, | ||||||
| favoritesUiState: EcosystemFavoritesUiState, | ||||||
| navigateToPlace: (Place) -> Unit, | ||||||
|
|
@@ -200,7 +204,7 @@ private fun BottomSheetFilteredContent( | |||||
| favorites = favorites, | ||||||
| filteredFavorites = favoritesUiState.filteredSortedFavorites, | ||||||
| eateryByPlace = favoritesUiState.eateryByPlace, | ||||||
| libraryByPlace = favoritesUiState.libraryByPlace, | ||||||
| libraryCardByPlace = favoritesUiState.libraryCardByPlace, | ||||||
| gymByPlace = favoritesUiState.gymByPlace, | ||||||
| printerByPlace = favoritesUiState.printerByPlace, | ||||||
| navigateToPlace = navigateToPlace, | ||||||
|
|
@@ -248,7 +252,7 @@ private fun BottomSheetFilteredContent( | |||||
|
|
||||||
| FilterState.LIBRARIES -> { | ||||||
| libraryList( | ||||||
| staticPlaces, | ||||||
| libraryCardsApiResponse, | ||||||
| navigateToPlace, | ||||||
| onDetailsClick, | ||||||
| favorites, | ||||||
|
|
@@ -269,7 +273,7 @@ private fun LazyListScope.favoriteList( | |||||
| favorites: Set<Place>, | ||||||
| filteredFavorites: List<Place>, | ||||||
| eateryByPlace: Map<Place, Eatery>, | ||||||
| libraryByPlace: Map<Place, Library>, | ||||||
| libraryCardByPlace: Map<Place, LibraryCardUiState>, | ||||||
| gymByPlace: Map<Place, UpliftGym>, | ||||||
| printerByPlace: Map<Place, PrinterCardUiState>, | ||||||
| navigateToPlace: (Place) -> Unit, | ||||||
|
|
@@ -319,18 +323,22 @@ private fun LazyListScope.favoriteList( | |||||
| } | ||||||
|
|
||||||
| PlaceType.LIBRARY -> { | ||||||
| val matchingLibrary = libraryByPlace[place] | ||||||
| if (matchingLibrary != null) { | ||||||
| val matchingLibraryCard = libraryCardByPlace[place] | ||||||
| if (matchingLibraryCard != null) { | ||||||
| val matchingLibrary = matchingLibraryCard.library | ||||||
| RoundedImagePlaceCard( | ||||||
| title = matchingLibrary.location, | ||||||
| subtitle = matchingLibrary.address + distanceStringToPlace( | ||||||
| matchingLibrary.latitude, | ||||||
| matchingLibrary.longitude | ||||||
| ), | ||||||
| isFavorite = true, | ||||||
| onFavoriteClick = { onFavoriteStarClick(place) } | ||||||
| onFavoriteClick = { onFavoriteStarClick(place) }, | ||||||
| placeholderRes = matchingLibraryCard.placeholderRes | ||||||
| ) { | ||||||
| onDetailsClick(matchingLibrary) | ||||||
| // Use detailed content sheet when backend is updated | ||||||
| // onDetailsClick(matchingLibrary) | ||||||
| navigateToPlace(matchingLibrary.toPlace()) | ||||||
| } | ||||||
| } else { | ||||||
| StandardCard( | ||||||
|
|
@@ -564,32 +572,35 @@ private fun LazyListScope.eateryList( | |||||
| * LazyList scoped enumeration of libraries for bottom sheet | ||||||
| */ | ||||||
| private fun LazyListScope.libraryList( | ||||||
| staticPlaces: StaticPlaces, | ||||||
| libraryCardsApiResponse: ApiResponse<List<LibraryCardUiState>>, | ||||||
| navigateToPlace: (Place) -> Unit, | ||||||
| navigateToDetails: (DetailedEcosystemPlace) -> Unit, | ||||||
|
||||||
| navigateToDetails: (DetailedEcosystemPlace) -> Unit, | |
| _navigateToDetails: (DetailedEcosystemPlace) -> Unit, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't leave the libraries tab blank while loading or after a failure.
Both branches render nothing, so a slow or failed fetch is indistinguishable from “there are no libraries.” At minimum, the Pending case should show the existing spinner, and the Error case should show some unavailable state instead of an empty list.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/cornellappdev/transit/ui/components/home/EcosystemBottomSheetContent.kt`
around lines 582 - 587, When handling libraryCardsApiResponse, don't render
nothing for ApiResponse.Pending and ApiResponse.Error: update the when branches
for libraryCardsApiResponse to display a loading indicator when the state is
ApiResponse.Pending (reuse the existing spinner/composable used elsewhere), and
display an "unavailable" error state when the state is ApiResponse.Error (show a
message and optional retry action or placeholder composable). Specifically
modify the ApiResponse.Pending branch to call the existing spinner composable
and the ApiResponse.Error branch to call or create a lightweight
librariesUnavailable/error placeholder (with a retry lambda that re-fetches the
library cards), referencing libraryCardsApiResponse, ApiResponse.Pending and
ApiResponse.Error to locate the change.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,10 @@ | ||
| package com.cornellappdev.transit.ui.viewmodels | ||
|
|
||
| import android.content.Context | ||
| import androidx.annotation.DrawableRes | ||
| import androidx.lifecycle.ViewModel | ||
| import androidx.lifecycle.viewModelScope | ||
| import com.cornellappdev.transit.R | ||
| import com.cornellappdev.transit.models.LocationRepository | ||
| import com.cornellappdev.transit.models.Place | ||
| import com.cornellappdev.transit.models.PlaceType | ||
|
|
@@ -49,6 +51,25 @@ class HomeViewModel @Inject constructor( | |
| private val selectedRouteRepository: SelectedRouteRepository | ||
| ) : ViewModel() { | ||
|
|
||
| val libraryCardsFlow: StateFlow<ApiResponse<List<LibraryCardUiState>>> = | ||
| routeRepository.libraryFlow.map { response -> | ||
| when (response) { | ||
| is ApiResponse.Success -> { | ||
| ApiResponse.Success( | ||
| response.data | ||
| .filterNot { it.isExcludedLibrary() } | ||
| .map { it.toLibraryCardUiState() } | ||
| ) | ||
| } | ||
| is ApiResponse.Pending -> ApiResponse.Pending | ||
| is ApiResponse.Error -> ApiResponse.Error | ||
| } | ||
|
Comment on lines
+54
to
+66
|
||
| }.stateIn( | ||
| scope = viewModelScope, | ||
| started = SharingStarted.Eagerly, | ||
| initialValue = ApiResponse.Pending | ||
| ) | ||
|
|
||
| /** | ||
| * The current query in the add favorites search bar, as a StateFlow | ||
| */ | ||
|
|
@@ -97,7 +118,7 @@ class HomeViewModel @Inject constructor( | |
| ) { printers, libraries, eateries, gyms -> | ||
| StaticPlaces( | ||
| printers, | ||
| libraries, | ||
| libraries.withExcludedLibrariesRemoved(), | ||
| eateries, | ||
| gyms | ||
| ) | ||
|
|
@@ -146,6 +167,7 @@ class HomeViewModel @Inject constructor( | |
|
|
||
| val filteredSortedFavorites = favorites.asSequence() | ||
| .filter { allowedTypes.isEmpty() || it.type in allowedTypes } | ||
| .filterNot { it.isExcludedLibraryPlace() } | ||
|
Comment on lines
168
to
+170
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apply the exclusion before favorites reach any UI surface.
Possible extraction+private fun Set<Place>.withoutExcludedLibraryPlaces(): Set<Place> =
+ filterNot { it.isExcludedLibraryPlace() }.toSet()Then reuse that helper both here and anywhere 🤖 Prompt for AI Agents |
||
| .sortedWith(compareBy<Place>({ it.type.ordinal }, { it.name })) | ||
| .toList() | ||
|
|
||
|
|
@@ -157,7 +179,9 @@ class HomeViewModel @Inject constructor( | |
| EcosystemFavoritesUiState( | ||
| filteredSortedFavorites = filteredSortedFavorites, | ||
| eateryByPlace = eateries.associateBy { it.toPlace() }, | ||
| libraryByPlace = libraries.associateBy { it.toPlace() }, | ||
| libraryCardByPlace = libraries | ||
| .map { it.toLibraryCardUiState() } | ||
| .associateBy { it.library.toPlace() }, | ||
| gymByPlace = gyms.associateBy { it.toPlace() }, | ||
| printerByPlace = printers.associate { printer -> | ||
| val place = printer.toPlace() | ||
|
|
@@ -402,11 +426,19 @@ class HomeViewModel @Inject constructor( | |
| data class EcosystemFavoritesUiState( | ||
| val filteredSortedFavorites: List<Place> = emptyList(), | ||
| val eateryByPlace: Map<Place, Eatery> = emptyMap(), | ||
| val libraryByPlace: Map<Place, Library> = emptyMap(), | ||
| val libraryCardByPlace: Map<Place, LibraryCardUiState> = emptyMap(), | ||
| val gymByPlace: Map<Place, UpliftGym> = emptyMap(), | ||
| val printerByPlace: Map<Place, PrinterCardUiState> = emptyMap() | ||
| ) | ||
|
|
||
| /** | ||
| * UI-ready library fields so composables receive configured fallback/override images. | ||
| */ | ||
| data class LibraryCardUiState( | ||
| val library: Library, | ||
| @DrawableRes val placeholderRes: Int | ||
| ) | ||
|
|
||
| /** | ||
| * UI-ready printer fields so composables don't parse backend strings. | ||
| */ | ||
|
|
@@ -431,6 +463,57 @@ private fun Printer.toPrinterCardUiState(): PrinterCardUiState { | |
| ) | ||
| } | ||
|
|
||
| private val libraryImageOverridesByLocation: Map<String, Int> = mapOf( | ||
| // Temporary placeholders: each location is explicitly configurable for future per-library assets. | ||
| "africana studies and research center" to R.drawable.library_img_africana_studies, | ||
| "carpenter hall" to R.drawable.library_img_carpenter_hall, | ||
| "clark hall" to R.drawable.library_img_clark_hall, | ||
| "comstock hall" to R.drawable.library_img_comstock_hall, | ||
| "imogene powers johnson center for birds and biodiversity" to R.drawable.olin_library, | ||
| "ives hall" to R.drawable.library_img_ives_hall, | ||
| "jordan hall" to R.drawable.olin_library, | ||
| "lincoln hall" to R.drawable.library_img_lincoln_hall, | ||
| "malott hall" to R.drawable.library_img_malott_hall, | ||
| "mann library" to R.drawable.library_img_mann_library, | ||
| "myron taylor hall" to R.drawable.library_img_myron_taylor_hall, | ||
| "myron taylor jane foster library addition" to R.drawable.olin_library, | ||
| "olin library" to R.drawable.olin_library, | ||
| "rand hall" to R.drawable.library_img_rand_hall, | ||
| "sage hall" to R.drawable.library_img_sage_hall, | ||
| "statler hall" to R.drawable.library_img_statler_hall, | ||
| "vet education center" to R.drawable.library_img_vet_center | ||
| ) | ||
|
|
||
| private val excludedLibraryLocations: Set<String> = setOf( | ||
| "imogene powers johnson center for birds and biodiversity", | ||
| "myron taylor jane foster library addition", | ||
| "jordan hall" | ||
| ) | ||
|
|
||
| private fun ApiResponse<List<Library>>.withExcludedLibrariesRemoved(): ApiResponse<List<Library>> { | ||
| return when (this) { | ||
| is ApiResponse.Success -> ApiResponse.Success(data.filterNot { it.isExcludedLibrary() }) | ||
| is ApiResponse.Pending -> ApiResponse.Pending | ||
| is ApiResponse.Error -> ApiResponse.Error | ||
| } | ||
| } | ||
|
|
||
| private fun Library.isExcludedLibrary(): Boolean { | ||
| return location.trim().lowercase() in excludedLibraryLocations | ||
| } | ||
|
|
||
| private fun Place.isExcludedLibraryPlace(): Boolean { | ||
| return type == PlaceType.LIBRARY && name.trim().lowercase() in excludedLibraryLocations | ||
| } | ||
|
|
||
| private fun Library.toLibraryCardUiState(): LibraryCardUiState { | ||
| val normalizedLocation = location.trim().lowercase() | ||
| return LibraryCardUiState( | ||
| library = this, | ||
| placeholderRes = libraryImageOverridesByLocation[normalizedLocation] ?: R.drawable.olin_library | ||
| ) | ||
| } | ||
|
|
||
| private fun Set<FavoritesFilterSheetState>.toAllowedPlaceTypes(): Set<PlaceType> = buildSet { | ||
| if (FavoritesFilterSheetState.EATERIES in this@toAllowedPlaceTypes) add(PlaceType.EATERY) | ||
| if (FavoritesFilterSheetState.LIBRARIES in this@toAllowedPlaceTypes) add(PlaceType.LIBRARY) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The KDoc above
EcosystemBottomSheetContentdoesn’t mention the newlibraryCardsApiResponseparameter, and thestaticPlacesdescription implies libraries come only fromstaticPlaces. Please update the KDoc to reflect the new source of library UI state.