From 33d6e70d82e1220be3eb8f9b4f5ecb2e0fb740ca Mon Sep 17 00:00:00 2001 From: Scott Olcott Date: Sat, 12 Sep 2026 16:35:10 -0600 Subject: [PATCH 1/3] Show a spinner, not an empty list, while a never-fetched key loads A Room-backed Store source of truth returns [] for a key it has never fetched, and Store emits that before the fetch starts. asOutcomes() passed it on as data and foldToState counted it as loaded, so a first visit rendered "No recipes found" under a refresh bar until the network answered -- and kept rendering it, instead of the error and Retry, if the network failed. Categories, Areas and Ingredients share foldToState and had the same problem. Every refreshing repository stream now calls asOutcomes(fetching = refresh) { ... } inside its flatMapLatest, so the adapter can hold that first empty read back until the fetch answers. getFavoritesAsFlow is left alone: its request never refreshes, and there is no fetcher to wait on. foldToState checks hasAnswer instead of hasLoaded, as a backstop: an empty cached value while its request is in flight or has failed is not an answer. Idle always is, so a legitimately empty tab still settles rather than spinning. Side effect: AreaRepository.countryFor now waits for the areas fetch on a fresh install instead of reading null from the empty cache. Needs the kmp-dataresult snapshot from solcott/kmp-dataresult#5. CI resolves it from GitHub Packages, so this stays red until that PR merges and publish-snapshot runs. Co-Authored-By: Claude Opus 5 --- .claude/skills/circuit-screen/SKILL.md | 6 ++- CLAUDE.md | 20 +++++--- .../recipe/domain/presenter/ListContent.kt | 13 +++-- .../domain/presenter/RecipesPresenterTest.kt | 48 +++++++++++++++++++ .../recipe/repository/AreaRepository.kt | 12 +++-- .../recipe/repository/CategoryRepository.kt | 12 +++-- .../recipe/repository/IngredientRepository.kt | 12 +++-- .../recipe/repository/RecipeRepository.kt | 47 ++++++++++++------ 8 files changed, 127 insertions(+), 43 deletions(-) diff --git a/.claude/skills/circuit-screen/SKILL.md b/.claude/skills/circuit-screen/SKILL.md index 6948954..1a68b9f 100644 --- a/.claude/skills/circuit-screen/SKILL.md +++ b/.claude/skills/circuit-screen/SKILL.md @@ -81,8 +81,10 @@ return state.foldToState( `Success(isRefreshing = true)` rather than dropping back to `Loading`. Do not add a `retain`ed var beside it to do that job — that pattern predates `ContentState` and is gone. -Inside `foldToState`, **`hasLoaded` is the discriminator, not `data.isEmpty()`**: an empty list is a -real answer, and reading it as "nothing yet" hangs a spinner over an empty screen. See the +Inside `foldToState`, **`hasAnswer` is the discriminator, not `data.isEmpty()`**: an empty list is a +real answer, and reading it as "nothing yet" hangs a spinner over an empty screen. The one empty list +that isn't an answer is an empty read from cache while its request is still in flight. The +repository holds it back (`asOutcomes(fetching = refresh) { … }`) and `hasAnswer` refuses it. See the Architecture section of `CLAUDE.md`. ### Screen persistence diff --git a/CLAUDE.md b/CLAUDE.md index b8464d9..aaedf45 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -160,7 +160,8 @@ full pattern when adding a screen. Repositories return `Flow>`, not Store's own `StoreReadResponse`. **Store 5 is an implementation detail of `:repository`** — it is an `implementation` dependency there, and nothing above that module compiles against it. `io.github.solcott:dataresult-store5` does the translation -in `asOutcomes()`, at the end of each repository's chain. +in `asOutcomes(fetching = refresh) { … }`, at the end of each repository's chain. It sits *inside* +the `flatMapLatest { refresh -> … }` so it can see whether a fetch is coming. The types come from [kmp-dataresult](https://github.com/solcott/kmp-dataresult), shared with the `Countries` project. Resolving them needs `gpr.user`/`gpr.key` in `~/.gradle/gradle.properties` (a @@ -169,7 +170,10 @@ classic PAT with `read:packages`) — GitHub Packages authenticates even public The path from a repository to a screen state is three steps: 1. `Flow>` — `Loading`, `Data(value, origin)` or `Error(cause, origin)`. Store's - `Initial` and `Loading` both become `Outcome.Loading`; `NoNewData` is dropped. + `Initial` and `Loading` both become `Outcome.Loading`; `NoNewData` is dropped. An empty first + read from cache while a fetch is pending is held back until the fetch answers. Room returns `[]` + for a key it has never fetched, and that's a cache miss, not an empty result. + `getFavoritesAsFlow` passes no `fetching`, because nothing is coming to answer instead. 2. `produceRetainedContentState(initial, keys) { … }` from `libs.uistateCircuit` (`io.github.solcott.uistate.circuit`) folds those emissions into a retained `ContentState`. **`ContentState.data` is what holds the last loaded value** — the separate retained @@ -187,10 +191,14 @@ the example. Two things are easy to get wrong: -- **Check `hasLoaded`, never `data.isEmpty()`**, to tell "nothing has loaded yet" from "loaded and - empty". `hasLoaded` reads `origin`, which stays null until the first value arrives. Testing the - data for emptiness leaves a spinner over a legitimately empty tab forever. -- **Having data beats being in flight.** `foldToState` checks `hasLoaded` first, which is what +- **Check `hasAnswer`, never `data.isEmpty()`**, to tell "nothing has loaded yet" from "loaded and + empty". `hasLoaded` reads `origin`, which stays null until the first value arrives. `hasAnswer` + also refuses an empty read *from cache* while its request is in flight or has failed. That's a key + the database has never seen, not an empty result. The repositories already hold that read back + with `asOutcomes(fetching = refresh) { … }`, and `hasAnswer` is the backstop. Testing the data for + emptiness leaves a spinner over a legitimately empty tab forever. `hasAnswer` can't, because it is + only false while a request is outstanding or has failed. +- **Having data beats being in flight.** `foldToState` checks `hasAnswer` first, which is what renders a background refresh as `isRefreshing` over the existing grid instead of dropping back to a full-screen spinner. `domain`'s `refreshKeepsPreviousAreas` test pins this — don't reorder those branches. diff --git a/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/ListContent.kt b/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/ListContent.kt index f6f88d3..83e271e 100644 --- a/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/ListContent.kt +++ b/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/ListContent.kt @@ -3,7 +3,7 @@ package com.scottolcott.recipe.domain.presenter import io.github.solcott.dataresult.DataError import io.github.solcott.uistate.ContentState import io.github.solcott.uistate.errorOrNull -import io.github.solcott.uistate.hasLoaded +import io.github.solcott.uistate.hasAnswer import io.github.solcott.uistate.isLoading /** @@ -17,9 +17,12 @@ import io.github.solcott.uistate.isLoading * keeps the last list it loaded, so a background refresh reports it as content that is refreshing * rather than dropping the grid for a spinner. * - * [ContentState.hasLoaded] is what separates the first two branches, not `data.isEmpty()` — an - * empty list is a real answer, and reading it as "nothing yet" would leave a spinner over a - * legitimately empty tab forever. + * `hasAnswer` is what separates the first two branches, not `data.isEmpty()` — an empty list is a + * real answer, and reading it as "nothing yet" would leave a spinner over a legitimately empty tab + * forever. What it adds over `hasLoaded` is the one empty list that is *not* an answer: an empty + * read from cache while its request is still in flight or has failed. That is a key the database + * has never seen, so it gets the spinner, or the error, rather than "nothing found". The + * repositories already hold that read back with `asOutcomes(fetching = …)`; this is the backstop. */ internal inline fun ContentState>.foldToState( onLoading: () -> S, @@ -27,7 +30,7 @@ internal inline fun ContentState>.foldToState( onContent: (items: List, isRefreshing: Boolean) -> S, ): S = when { - hasLoaded -> onContent(data, isLoading) + hasAnswer(List::isEmpty) -> onContent(data, isLoading) isLoading -> onLoading() else -> onError(errorOrNull.toMessage()) } diff --git a/domain/src/commonTest/kotlin/com/scottolcott/recipe/domain/presenter/RecipesPresenterTest.kt b/domain/src/commonTest/kotlin/com/scottolcott/recipe/domain/presenter/RecipesPresenterTest.kt index 2949764..99c15e8 100644 --- a/domain/src/commonTest/kotlin/com/scottolcott/recipe/domain/presenter/RecipesPresenterTest.kt +++ b/domain/src/commonTest/kotlin/com/scottolcott/recipe/domain/presenter/RecipesPresenterTest.kt @@ -7,6 +7,7 @@ import com.scottolcott.recipe.repository.RecipeRepository import com.slack.circuit.test.FakeNavigator import com.slack.circuit.test.test import de.infix.testBalloon.framework.core.testSuite +import io.github.solcott.dataresult.DataError import io.github.solcott.dataresult.Origin import io.github.solcott.dataresult.Outcome import kotlin.test.assertEquals @@ -112,4 +113,51 @@ val recipesPresenterTests by testSuite { assertEquals(screen, state.screen) } } + + // Regression: an empty read from cache while the network is being asked is a key the database + // has never seen, not an empty result. It used to render as `Success(empty, isRefreshing = true)` + // -- "No recipes found" under a progress bar -- until the network answered. + test("an empty cache read while the network is asked is still loading") { + val screen = RecipesScreen.BySearch("chicken") + val repository = FakeRecipeRepository() + val presenter = RecipesPresenter(screen, FakeNavigator(screen), RecipesProducer(repository)) + + presenter.test { + assertIs(awaitItem()) + + // Settled, so it is an answer: nothing else was coming. + repository.responses.value = Outcome.Data(emptyList(), Origin.Cache) + assertIs(awaitItem()) + + repository.responses.value = Outcome.Loading + assertIs(awaitItem()) + + // One instance: `recipe()` stamps `lastFetched` with the current time. + val fresh = listOf(recipe("1")) + repository.responses.value = Outcome.Data(fresh, Origin.Network) + val state = assertIs(awaitItem()) + assertEquals(fresh, state.recipes) + } + } + + // Offline on a first visit: the screen owes the user the failure and a retry, not "No recipes + // found". + test("an empty cache read whose fetch fails is an error") { + val screen = RecipesScreen.BySearch("chicken") + val repository = FakeRecipeRepository() + val presenter = RecipesPresenter(screen, FakeNavigator(screen), RecipesProducer(repository)) + + presenter.test { + assertIs(awaitItem()) + + repository.responses.value = Outcome.Data(emptyList(), Origin.Cache) + assertIs(awaitItem()) + + repository.responses.value = Outcome.Loading + assertIs(awaitItem()) + + repository.responses.value = Outcome.Error(DataError.Network, Origin.Network) + assertIs(awaitItem()) + } + } } diff --git a/repository/src/commonMain/kotlin/com/scottolcott/recipe/repository/AreaRepository.kt b/repository/src/commonMain/kotlin/com/scottolcott/recipe/repository/AreaRepository.kt index 3ad83cf..dda725d 100644 --- a/repository/src/commonMain/kotlin/com/scottolcott/recipe/repository/AreaRepository.kt +++ b/repository/src/commonMain/kotlin/com/scottolcott/recipe/repository/AreaRepository.kt @@ -92,11 +92,13 @@ internal class AreaRepositoryImpl( StoreBuilder.from(fetcher, sourceOfTruth, converter).build() override fun getAreas(): Flow>> { - return fetchHistoryDataStore - .refreshNeeded(cacheExpiration) - .flatMapLatest { refresh -> store.stream(StoreReadRequest.cached(Unit, refresh)) } - .logErrors(logger, "Error loading areas") - .asOutcomes() + return fetchHistoryDataStore.refreshNeeded(cacheExpiration).flatMapLatest { refresh -> + // `fetching` holds back a first read of `[]`: a key never fetched, not an empty result. + store + .stream(StoreReadRequest.cached(Unit, refresh)) + .logErrors(logger, "Error loading areas") + .asOutcomes(fetching = refresh) { it.isEmpty() } + } } override suspend fun countryFor(area: String): String? { diff --git a/repository/src/commonMain/kotlin/com/scottolcott/recipe/repository/CategoryRepository.kt b/repository/src/commonMain/kotlin/com/scottolcott/recipe/repository/CategoryRepository.kt index 6c0d2d4..e10ce35 100644 --- a/repository/src/commonMain/kotlin/com/scottolcott/recipe/repository/CategoryRepository.kt +++ b/repository/src/commonMain/kotlin/com/scottolcott/recipe/repository/CategoryRepository.kt @@ -123,10 +123,12 @@ internal class CategoryRepositoryImpl( @OptIn(ExperimentalCoroutinesApi::class) private fun loadCategoriesByKey(key: CategoriesKey): Flow>> { - return fetchHistoryDataStore - .refreshNeeded(key, cacheExpiration) - .flatMapLatest { refresh -> store.stream(StoreReadRequest.cached(key, refresh)) } - .logErrors(logger, "Error loading categories by $key") - .asOutcomes() + return fetchHistoryDataStore.refreshNeeded(key, cacheExpiration).flatMapLatest { refresh -> + // `fetching` holds back a first read of `[]`: a key never fetched, not an empty result. + store + .stream(StoreReadRequest.cached(key, refresh)) + .logErrors(logger, "Error loading categories by $key") + .asOutcomes(fetching = refresh) { it.isEmpty() } + } } } diff --git a/repository/src/commonMain/kotlin/com/scottolcott/recipe/repository/IngredientRepository.kt b/repository/src/commonMain/kotlin/com/scottolcott/recipe/repository/IngredientRepository.kt index 11a48b7..8a9ef33 100644 --- a/repository/src/commonMain/kotlin/com/scottolcott/recipe/repository/IngredientRepository.kt +++ b/repository/src/commonMain/kotlin/com/scottolcott/recipe/repository/IngredientRepository.kt @@ -115,11 +115,13 @@ internal class IngredientRepositoryImpl( } private fun loadIngredientsByKey(key: IngredientsKey): Flow>> { - return fetchHistoryDataStore - .refreshNeeded(cacheExpiration) - .flatMapLatest { refresh -> store.stream(StoreReadRequest.cached(key, refresh)) } - .logErrors(logger, "Error loading ingredients by $key") - .asOutcomes() + return fetchHistoryDataStore.refreshNeeded(cacheExpiration).flatMapLatest { refresh -> + // `fetching` holds back a first read of `[]`: a key never fetched, not an empty result. + store + .stream(StoreReadRequest.cached(key, refresh)) + .logErrors(logger, "Error loading ingredients by $key") + .asOutcomes(fetching = refresh) { it.isEmpty() } + } } private fun Flow>.mapToIngredients(): Flow> = diff --git a/repository/src/commonMain/kotlin/com/scottolcott/recipe/repository/RecipeRepository.kt b/repository/src/commonMain/kotlin/com/scottolcott/recipe/repository/RecipeRepository.kt index 9b894ef..51af0b2 100644 --- a/repository/src/commonMain/kotlin/com/scottolcott/recipe/repository/RecipeRepository.kt +++ b/repository/src/commonMain/kotlin/com/scottolcott/recipe/repository/RecipeRepository.kt @@ -69,9 +69,14 @@ internal class RecipeRepositoryImpl( val key = RecipesKey.Query(query.trim()) return fetchHistoryDataStore .refreshNeeded(key, cacheExpiration) - .flatMapLatest { refresh -> recipeStore.stream(StoreReadRequest.cached(key, refresh)) } - .logErrors(logger, "Error searching recipes by $query") - .asOutcomes() + .flatMapLatest { refresh -> + // `fetching` holds back a first read of `[]`: a key never fetched, not an empty result. + // Every refreshing stream below does the same. + recipeStore + .stream(StoreReadRequest.cached(key, refresh)) + .logErrors(logger, "Error searching recipes by $query") + .asOutcomes(fetching = refresh) { it.recipes.isEmpty() } + } .map { outcome -> outcome.mapData { it.recipes } } } @@ -80,9 +85,12 @@ internal class RecipeRepositoryImpl( val key = RecipesKey.ByCategory(category) return fetchHistoryDataStore .refreshNeeded(key, cacheExpiration) - .flatMapLatest { refresh -> recipeStore.stream(StoreReadRequest.cached(key, refresh)) } - .logErrors(logger, "Error loading recipes by category $category") - .asOutcomes() + .flatMapLatest { refresh -> + recipeStore + .stream(StoreReadRequest.cached(key, refresh)) + .logErrors(logger, "Error loading recipes by category $category") + .asOutcomes(fetching = refresh) { it.recipes.isEmpty() } + } .map { outcome -> outcome.mapData { it.recipes } } } @@ -91,9 +99,12 @@ internal class RecipeRepositoryImpl( val key = RecipesKey.ByIngredient.of(ingredients) return fetchHistoryDataStore .refreshNeeded(key, cacheExpiration) - .flatMapLatest { refresh -> recipeStore.stream(StoreReadRequest.cached(key, refresh)) } - .logErrors(logger, "Error loading recipes by ingredients $ingredients") - .asOutcomes() + .flatMapLatest { refresh -> + recipeStore + .stream(StoreReadRequest.cached(key, refresh)) + .logErrors(logger, "Error loading recipes by ingredients $ingredients") + .asOutcomes(fetching = refresh) { it.recipes.isEmpty() } + } .map { outcome -> outcome.mapData { it.recipes } } } @@ -102,9 +113,12 @@ internal class RecipeRepositoryImpl( val key = RecipesKey.ByArea(area) return fetchHistoryDataStore .refreshNeeded(key, cacheExpiration) - .flatMapLatest { refresh -> recipeStore.stream(StoreReadRequest.cached(key, refresh)) } - .logErrors(logger, "Error loading recipes by area $area") - .asOutcomes() + .flatMapLatest { refresh -> + recipeStore + .stream(StoreReadRequest.cached(key, refresh)) + .logErrors(logger, "Error loading recipes by area $area") + .asOutcomes(fetching = refresh) { it.recipes.isEmpty() } + } .map { outcome -> outcome.mapData { it.recipes } } } @@ -113,9 +127,12 @@ internal class RecipeRepositoryImpl( val key = RecipesKey.ById(id) return fetchHistoryDataStore .refreshNeeded(key, cacheExpiration) - .flatMapLatest { refresh -> recipeStore.stream(StoreReadRequest.cached(key, refresh)) } - .logErrors(logger, "Error loading recipes by id : $id") - .asOutcomes() + .flatMapLatest { refresh -> + recipeStore + .stream(StoreReadRequest.cached(key, refresh)) + .logErrors(logger, "Error loading recipes by id : $id") + .asOutcomes(fetching = refresh) { it.recipes.isEmpty() } + } .map { outcome -> outcome.mapData { it.recipes.firstOrNull() } } } From 48d68d50d03a4aaa05d8bd822c00a46faac54f33 Mon Sep 17 00:00:00 2001 From: Scott Olcott Date: Sat, 12 Sep 2026 16:40:39 -0600 Subject: [PATCH 2/3] Refactor ContentState mapping and animate state transitions. * **Domain**: * Added a `foldToState` extension on `ContentState` to standardize mapping the loaded/loading/error matrix into three explicit UI states (Loading, Error, Success). * Refactored `RecipeDetailsState` and `RecipeDetailsEvent` into sealed interfaces matching the other screens, replacing the boolean `loading` and `error` flags. * **UI**: * Wrapped the top-level state evaluation blocks in all five screens (`AreasScreen`, `CategoriesScreen`, `IngredientsScreen`, `RecipeDetailsScreen`, and `RecipesScreen`) with `AnimatedContent` to crossfade between loading, error, and success states. * Added a `LinearProgressIndicator` to `RecipeDetailsScreen` and `RecipesScreen`, wrapped in `AnimatedVisibility`, to display background refreshes (`isRefreshing`) without dropping the currently displayed data. * Centered `ErrorDisplay` contents vertically and horizontally. Signed-off-by: Scott Olcott --- .../recipe/domain/presenter/Content.kt | 35 +++++++ .../presenter/RecipeDetailsPresenter.kt | 81 +++++++++------ .../com/scottolcott/recipe/ui/ErrorDisplay.kt | 7 +- .../scottolcott/recipe/ui/area/AreasScreen.kt | 61 ++++++------ .../recipe/ui/category/CategoriesScreen.kt | 61 ++++++------ .../recipe/ui/ingredient/IngredientsScreen.kt | 61 ++++++------ .../recipe/ui/recipe/RecipeCategoryAndArea.kt | 6 +- .../recipe/ui/recipe/RecipeDetailsScreen.kt | 46 ++++++--- .../recipe/ui/recipe/RecipesScreen.kt | 99 +++++++++++-------- 9 files changed, 284 insertions(+), 173 deletions(-) create mode 100644 domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/Content.kt diff --git a/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/Content.kt b/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/Content.kt new file mode 100644 index 0000000..6742c24 --- /dev/null +++ b/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/Content.kt @@ -0,0 +1,35 @@ +package com.scottolcott.recipe.domain.presenter + +import io.github.solcott.uistate.ContentState +import io.github.solcott.uistate.errorOrNull +import io.github.solcott.uistate.hasAnswer +import io.github.solcott.uistate.isLoading + +/** + * The three states every screen renders, decoupled from any one screen's `CircuitUiState`. + * + * Each tab keeps its own state type — Circuit pairs a state with exactly one UI — but the mapping + * from a [ContentState] onto these three cases is the same everywhere, so it lives here rather than + * three times over. + * + * The order of the branches is the point. Having data wins over being in flight: [ContentState] + * keeps the last item it loaded, so a background refresh reports it as content that is refreshing + * rather than dropping the grid for a spinner. + * + * `hasAnswer` is what separates the first two branches, not `data == null` — a null is a real + * answer, and reading it as "nothing yet" would leave a spinner over a legitimately empty tab + * forever. What it adds over `hasLoaded` is the one empty list that is *not* an answer: an empty + * read from cache while its request is still in flight or has failed. That is a key the database + * has never seen, so it gets the spinner, or the error, rather than "nothing found". The + * repositories already hold that read back with `asOutcomes(fetching = …)`; this is the backstop. + */ +internal inline fun ContentState.foldToState( + onLoading: () -> S, + onError: (message: String) -> S, + onContent: (item: T, isRefreshing: Boolean) -> S, +): S = + when { + hasAnswer { it != null } -> onContent(checkNotNull(data) { "data was null." }, isLoading) + isLoading -> onLoading() + else -> onError(errorOrNull.toMessage()) + } diff --git a/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/RecipeDetailsPresenter.kt b/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/RecipeDetailsPresenter.kt index 7565473..6ffa286 100644 --- a/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/RecipeDetailsPresenter.kt +++ b/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/RecipeDetailsPresenter.kt @@ -22,8 +22,6 @@ import dev.zacsweers.metro.AppScope import dev.zacsweers.metro.Inject import dev.zacsweers.redacted.annotations.Redacted import io.github.solcott.uistate.circuit.produceRetainedContentState -import io.github.solcott.uistate.errorOrNull -import io.github.solcott.uistate.isLoading import kotlinx.coroutines.launch @CircuitInject(RecipeDetailsScreen::class, AppScope::class) @@ -45,46 +43,71 @@ internal constructor( // Unlike the old response-shaped state, this keeps the recipe on screen through a refresh // rather than blanking it whenever the store goes back to the network. val recipe: Recipe? = state.data - return RecipeDetailsState( - recipe, - loading = state.isLoading, - error = state.errorOrNull != null, - ) { event -> - when (event) { - RecipeDetailsEvent.ToggleFavorite -> - coroutineScope.launch { - if (recipe != null) { - if (recipe.favorite) { - recipeRepository.removeFavorite(screen.id) - } else { - recipeRepository.addFavorite(screen.id) + val successEventSink: (RecipeDetailsEvent.Success) -> Unit = remember { + { event -> + when (event) { + RecipeDetailsEvent.Success.ToggleFavorite -> + coroutineScope.launch { + if (recipe != null) { + if (recipe.favorite) { + recipeRepository.removeFavorite(screen.id) + } else { + recipeRepository.addFavorite(screen.id) + } } } - } - RecipeDetailsEvent.RetryClicked -> retryTrigger++ - is RecipeDetailsEvent.CategoryClicked -> navigator.goTo(ByCategory(event.category)) + is RecipeDetailsEvent.Success.CategoryClicked -> + navigator.goTo(ByCategory(event.category)) - is RecipeDetailsEvent.AreaClicked -> navigator.goTo(ByArea(event.area)) + is RecipeDetailsEvent.Success.AreaClicked -> navigator.goTo(ByArea(event.area)) + } } } + + val errorEventSink: (RecipeDetailsEvent.Error) -> Unit = remember { + { event -> + when (event) { + RecipeDetailsEvent.Error.RetryClicked -> retryTrigger++ + } + } + } + return state.foldToState( + onLoading = { RecipeDetailsState.Loading }, + onError = { message -> RecipeDetailsState.Error(message, errorEventSink) }, + onContent = { recipe, isRefreshing -> + RecipeDetailsState.Success(recipe, isRefreshing, successEventSink) + }, + ) } } -data class RecipeDetailsState( - val recipe: Recipe?, - val loading: Boolean, - val error: Boolean, - @Redacted val eventSink: (RecipeDetailsEvent) -> Unit, -) : CircuitUiState +sealed interface RecipeDetailsState : CircuitUiState { + data object Loading : RecipeDetailsState + + data class Error( + val message: String, + @Redacted val eventSink: (RecipeDetailsEvent.Error) -> Unit, + ) : RecipeDetailsState + + data class Success( + val recipe: Recipe, + val isRefreshing: Boolean, + @Redacted val eventSink: (RecipeDetailsEvent.Success) -> Unit, + ) : RecipeDetailsState +} sealed interface RecipeDetailsEvent : CircuitUiEvent { - data object ToggleFavorite : RecipeDetailsEvent + sealed interface Success : RecipeDetailsEvent { + data class CategoryClicked(val category: String) : Success - data object RetryClicked : RecipeDetailsEvent + data class AreaClicked(val area: String) : Success - data class CategoryClicked(val category: String) : RecipeDetailsEvent + data object ToggleFavorite : Success + } - data class AreaClicked(val area: String) : RecipeDetailsEvent + sealed interface Error : RecipeDetailsEvent { + data object RetryClicked : Error + } } @CircuitSerializable(AppScope::class) data class RecipeDetailsScreen(val id: RecipeId) : Screen diff --git a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/ErrorDisplay.kt b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/ErrorDisplay.kt index b2fb5ba..8b677e8 100644 --- a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/ErrorDisplay.kt +++ b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/ErrorDisplay.kt @@ -1,5 +1,6 @@ package com.scottolcott.recipe.ui +import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column import androidx.compose.material3.Button import androidx.compose.material3.Text @@ -9,7 +10,11 @@ import org.jetbrains.compose.resources.stringResource @Composable fun ErrorDisplay(onRetryClick: () -> Unit, modifier: Modifier = Modifier) { - Column(modifier = modifier) { + Column( + modifier = modifier, + horizontalAlignment = androidx.compose.ui.Alignment.CenterHorizontally, + verticalArrangement = Arrangement.Center, + ) { Text(stringResource(Res.string.an_error_occurred)) Button(onRetryClick) { Text(stringResource(Res.string.retry)) } } diff --git a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/area/AreasScreen.kt b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/area/AreasScreen.kt index d95b98a..11a7ccc 100644 --- a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/area/AreasScreen.kt +++ b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/area/AreasScreen.kt @@ -1,5 +1,6 @@ package com.scottolcott.recipe.ui.area +import androidx.compose.animation.AnimatedContent import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -63,38 +64,40 @@ fun AreasScreen(state: AreasState, modifier: Modifier = Modifier) { // the gloss it is rather than a second title. val countryTextStyle = MaterialTheme.typography.bodyMedium Box(modifier, contentAlignment = Alignment.TopCenter) { - when (state) { - is AreasState.Error -> - Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - ErrorDisplay(onRetryClick = { state.eventSink(AreasEvent.Error.RetryClicked) }) - } - - AreasState.Loading -> - Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - CircularProgressIndicator() - } + AnimatedContent(state) { targetState -> + when (targetState) { + is AreasState.Error -> + Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + ErrorDisplay(onRetryClick = { targetState.eventSink(AreasEvent.Error.RetryClicked) }) + } - is AreasState.Success -> { - if (state.areas.isEmpty()) { + AreasState.Loading -> Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - Text(stringResource(Res.string.no_areas_found)) + CircularProgressIndicator() } - } else { - LazyVerticalGrid( - cells, - modifier = Modifier.fillMaxSize(), - verticalArrangement = Arrangement.spacedBy(12.dp), - horizontalArrangement = Arrangement.spacedBy(12.dp), - contentPadding = padding, - ) { - items(state.areas, key = { it.area }, contentType = { "area_item" }) { area -> - AreaItem( - area, - areaTextStyle = areaTextStyle, - countryTextStyle = countryTextStyle, - { state.eventSink(AreasEvent.Success.AreaClicked(area.area)) }, - Modifier.animateItem().pointerHoverIcon(PointerIcon.Hand, true), - ) + + is AreasState.Success -> { + if (targetState.areas.isEmpty()) { + Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + Text(stringResource(Res.string.no_areas_found)) + } + } else { + LazyVerticalGrid( + cells, + modifier = Modifier.fillMaxSize(), + verticalArrangement = Arrangement.spacedBy(12.dp), + horizontalArrangement = Arrangement.spacedBy(12.dp), + contentPadding = padding, + ) { + items(targetState.areas, key = { it.area }, contentType = { "area_item" }) { area -> + AreaItem( + area, + areaTextStyle = areaTextStyle, + countryTextStyle = countryTextStyle, + { targetState.eventSink(AreasEvent.Success.AreaClicked(area.area)) }, + Modifier.animateItem().pointerHoverIcon(PointerIcon.Hand, true), + ) + } } } } diff --git a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/category/CategoriesScreen.kt b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/category/CategoriesScreen.kt index 9817c75..8e965cd 100644 --- a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/category/CategoriesScreen.kt +++ b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/category/CategoriesScreen.kt @@ -2,6 +2,7 @@ package com.scottolcott.recipe.ui.category +import androidx.compose.animation.AnimatedContent import androidx.compose.foundation.background import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -61,37 +62,41 @@ fun CategoriesScreen(state: CategoriesState, modifier: Modifier = Modifier) { MaterialTheme.typography.titleSmallEmphasized } Box(modifier, contentAlignment = Alignment.TopCenter) { - when (state) { - is CategoriesState.Error -> - Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - ErrorDisplay(onRetryClick = { state.eventSink(CategoriesEvent.Error.RetryClicked) }) - } - - CategoriesState.Loading -> - Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - CircularProgressIndicator() - } + AnimatedContent(state) { targetState -> + when (targetState) { + is CategoriesState.Error -> + Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + ErrorDisplay( + onRetryClick = { targetState.eventSink(CategoriesEvent.Error.RetryClicked) } + ) + } - is CategoriesState.Success -> { - if (state.categories.isEmpty()) { + CategoriesState.Loading -> Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - Text(stringResource(Res.string.no_categories_found)) + CircularProgressIndicator() } - } else { - LazyVerticalGrid( - cells, - modifier = Modifier.fillMaxSize(), - verticalArrangement = Arrangement.spacedBy(12.dp), - horizontalArrangement = Arrangement.spacedBy(12.dp), - contentPadding = padding, - ) { - items(state.categories, key = { it.id }, contentType = { "category_item" }) { - CategoryItem( - it, - labelTextStyle = labelTextStyle, - { state.eventSink(CategoriesEvent.Success.CategoryClicked(it.name)) }, - Modifier.animateItem().pointerHoverIcon(PointerIcon.Hand, true), - ) + + is CategoriesState.Success -> { + if (targetState.categories.isEmpty()) { + Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + Text(stringResource(Res.string.no_categories_found)) + } + } else { + LazyVerticalGrid( + cells, + modifier = Modifier.fillMaxSize(), + verticalArrangement = Arrangement.spacedBy(12.dp), + horizontalArrangement = Arrangement.spacedBy(12.dp), + contentPadding = padding, + ) { + items(targetState.categories, key = { it.id }, contentType = { "category_item" }) { + CategoryItem( + it, + labelTextStyle = labelTextStyle, + { targetState.eventSink(CategoriesEvent.Success.CategoryClicked(it.name)) }, + Modifier.animateItem().pointerHoverIcon(PointerIcon.Hand, true), + ) + } } } } diff --git a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/ingredient/IngredientsScreen.kt b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/ingredient/IngredientsScreen.kt index 20aa048..606e318 100644 --- a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/ingredient/IngredientsScreen.kt +++ b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/ingredient/IngredientsScreen.kt @@ -1,5 +1,6 @@ package com.scottolcott.recipe.ui.ingredient +import androidx.compose.animation.AnimatedContent import androidx.compose.foundation.background import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -58,37 +59,41 @@ fun IngredientsScreen(state: IngredientsState, modifier: Modifier = Modifier) { MaterialTheme.typography.titleSmallEmphasized } Box(modifier, contentAlignment = Alignment.TopCenter) { - when (state) { - is IngredientsState.Error -> - Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - ErrorDisplay(onRetryClick = { state.eventSink(IngredientsEvent.Error.RetryClicked) }) - } - - IngredientsState.Loading -> - Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - CircularProgressIndicator() - } + AnimatedContent(state) { targetState -> + when (targetState) { + is IngredientsState.Error -> + Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + ErrorDisplay( + onRetryClick = { targetState.eventSink(IngredientsEvent.Error.RetryClicked) } + ) + } - is IngredientsState.Success -> { - if (state.ingredients.isEmpty()) { + IngredientsState.Loading -> Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - Text(stringResource(Res.string.no_ingredients_found)) + CircularProgressIndicator() } - } else { - LazyVerticalGrid( - cells, - modifier = Modifier.fillMaxSize(), - verticalArrangement = Arrangement.spacedBy(12.dp), - horizontalArrangement = Arrangement.spacedBy(12.dp), - contentPadding = padding, - ) { - items(state.ingredients, key = { it.id }, contentType = { "ingredient_item" }) { - IngredientItem( - it, - labelTextStyle = labelTextStyle, - { state.eventSink(IngredientsEvent.Success.IngredientClicked(it.name)) }, - Modifier.animateItem().pointerHoverIcon(PointerIcon.Hand, true), - ) + + is IngredientsState.Success -> { + if (targetState.ingredients.isEmpty()) { + Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + Text(stringResource(Res.string.no_ingredients_found)) + } + } else { + LazyVerticalGrid( + cells, + modifier = Modifier.fillMaxSize(), + verticalArrangement = Arrangement.spacedBy(12.dp), + horizontalArrangement = Arrangement.spacedBy(12.dp), + contentPadding = padding, + ) { + items(targetState.ingredients, key = { it.id }, contentType = { "ingredient_item" }) { + IngredientItem( + it, + labelTextStyle = labelTextStyle, + { targetState.eventSink(IngredientsEvent.Success.IngredientClicked(it.name)) }, + Modifier.animateItem().pointerHoverIcon(PointerIcon.Hand, true), + ) + } } } } diff --git a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipeCategoryAndArea.kt b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipeCategoryAndArea.kt index f921fbb..bf19a99 100644 --- a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipeCategoryAndArea.kt +++ b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipeCategoryAndArea.kt @@ -24,7 +24,7 @@ import org.jetbrains.compose.resources.painterResource @Composable fun RecipeCategoryAndArea( recipe: Recipe, - eventSink: (RecipeDetailsEvent) -> Unit, + eventSink: (RecipeDetailsEvent.Success) -> Unit, modifier: Modifier = Modifier, ) { // FlexBox rather than Row: a Row's min intrinsic width is the sum of both chips, which a narrow @@ -33,7 +33,7 @@ fun RecipeCategoryAndArea( val category = recipe.category.orEmpty() if (category.isNotBlank()) { AssistChip( - onClick = { eventSink(RecipeDetailsEvent.CategoryClicked(category)) }, + onClick = { eventSink(RecipeDetailsEvent.Success.CategoryClicked(category)) }, label = { Text(category) }, leadingIcon = { Icon(painterResource(Res.drawable.label_24px), null) }, colors = @@ -48,7 +48,7 @@ fun RecipeCategoryAndArea( val area = recipe.area.orEmpty() if (area.isNotBlank()) { AssistChip( - onClick = { eventSink(RecipeDetailsEvent.AreaClicked(area)) }, + onClick = { eventSink(RecipeDetailsEvent.Success.AreaClicked(area)) }, label = { Text(area) }, leadingIcon = { Icon(painterResource(Res.drawable.location_on_24px), null) }, colors = diff --git a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipeDetailsScreen.kt b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipeDetailsScreen.kt index 06753f4..6092035 100644 --- a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipeDetailsScreen.kt +++ b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipeDetailsScreen.kt @@ -1,5 +1,7 @@ package com.scottolcott.recipe.ui.recipe +import androidx.compose.animation.AnimatedContent +import androidx.compose.animation.AnimatedVisibility import androidx.compose.animation.animateBounds import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -25,6 +27,7 @@ import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi import androidx.compose.material3.Icon +import androidx.compose.material3.LinearProgressIndicator import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable @@ -62,9 +65,9 @@ import com.scottolcott.recipe.model.Recipe import com.scottolcott.recipe.model.RecipeDetails import com.scottolcott.recipe.model.RecipeId import com.scottolcott.recipe.model.RecipeIngredient +import com.scottolcott.recipe.ui.ErrorDisplay import com.scottolcott.recipe.ui.Res import com.scottolcott.recipe.ui.ThemeWrapper -import com.scottolcott.recipe.ui.an_error_occurred import com.scottolcott.recipe.ui.image_24px import com.scottolcott.recipe.ui.image_source import com.scottolcott.recipe.ui.link_24px @@ -82,19 +85,34 @@ import org.jetbrains.compose.resources.stringResource @CircuitInject(RecipeDetailsScreen::class, AppScope::class) fun RecipeDetailsScreen(state: RecipeDetailsState, modifier: Modifier = Modifier) { Box(modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - val recipe = state.recipe - if (state.loading) { - CircularProgressIndicator() - } else if (state.error) { - Text(stringResource(Res.string.an_error_occurred)) - } else { - recipe?.let { RecipeDetails(it, state.eventSink) } + AnimatedContent(state) { targetState -> + when (targetState) { + is RecipeDetailsState.Error -> + ErrorDisplay( + onRetryClick = { targetState.eventSink(RecipeDetailsEvent.Error.RetryClicked) } + ) + RecipeDetailsState.Loading -> CircularProgressIndicator() + is RecipeDetailsState.Success -> { + RecipeDetails( + targetState.recipe, + targetState.eventSink, + modifier = Modifier.fillMaxSize(), + ) + AnimatedVisibility(targetState.isRefreshing) { + LinearProgressIndicator(modifier = Modifier.fillMaxWidth().align(Alignment.TopStart)) + } + } + } } } } @Composable -private fun RecipeDetails(recipe: Recipe, eventSink: (RecipeDetailsEvent) -> Unit) { +private fun RecipeDetails( + recipe: Recipe, + eventSink: (RecipeDetailsEvent.Success) -> Unit, + modifier: Modifier = Modifier, +) { val windowSizeClass = LocalWindowSizeClass.current val columns = when { @@ -108,7 +126,7 @@ private fun RecipeDetails(recipe: Recipe, eventSink: (RecipeDetailsEvent) -> Uni LookaheadScope { SelectionContainer { - Column(Modifier.fillMaxSize().verticalScroll(rememberScrollState()).padding(padding)) { + Column(modifier.verticalScroll(rememberScrollState()).padding(padding)) { Text( recipe.name, style = MaterialTheme.typography.headlineMediumEmphasized, @@ -126,7 +144,7 @@ private fun LookaheadScope.RecipeGrid( recipe: Recipe, columns: Int, padding: PaddingValues, - eventSink: (RecipeDetailsEvent) -> Unit, + eventSink: (RecipeDetailsEvent.Success) -> Unit, ) { // The movable contents are remembered without keys so they keep their identity as the layout // moves them between columns. That means their lambdas capture whatever was in scope when they @@ -139,7 +157,7 @@ private fun LookaheadScope.RecipeGrid( RecipeImage( currentRecipe, currentRecipe.favorite, - onToggleFavorite = { currentEventSink(RecipeDetailsEvent.ToggleFavorite) }, + onToggleFavorite = { currentEventSink(RecipeDetailsEvent.Success.ToggleFavorite) }, modifier = modifier.animateBounds(this@RecipeGrid), ) } @@ -237,7 +255,7 @@ private fun GridScope.RecipeGridLayout( @Composable private fun RecipeMetaInfo( recipe: Recipe, - eventSink: (RecipeDetailsEvent) -> Unit, + eventSink: (RecipeDetailsEvent.Success) -> Unit, modifier: Modifier = Modifier, ) { val details = recipe.details @@ -381,5 +399,5 @@ private fun RecipeDetailsPreview() { details = details, lastFetched = Clock.System.now(), ) - RecipeDetails(recipe) {} + RecipeDetails(recipe, eventSink = {}) } diff --git a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipesScreen.kt b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipesScreen.kt index 6dd5bd6..eed0323 100644 --- a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipesScreen.kt +++ b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipesScreen.kt @@ -1,5 +1,7 @@ package com.scottolcott.recipe.ui.recipe +import androidx.compose.animation.AnimatedContent +import androidx.compose.animation.AnimatedVisibility import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -13,6 +15,7 @@ import androidx.compose.foundation.lazy.grid.GridItemSpan import androidx.compose.foundation.lazy.grid.LazyVerticalGrid import androidx.compose.foundation.lazy.grid.items import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.LinearProgressIndicator import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable @@ -54,50 +57,59 @@ fun RecipesScreen(state: RecipesState, modifier: Modifier = Modifier) { val horizontalCards = isShortWindow() // Whoever the bar is not naming has to name itself; see [LocalAppBarShowsScreenTitle]. val titledByAppBar = LocalAppBarShowsScreenTitle.current - when (state) { - is RecipesState.Error -> - Box(modifier = modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - ErrorDisplay(onRetryClick = { state.eventSink(RecipesEvent.Error.RetryClicked) }) - } - RecipesState.Loading -> - Box(modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - CircularProgressIndicator() - } - is RecipesState.Success -> { - if (state.recipes.isEmpty()) { - Column(modifier.fillMaxSize().padding(padding)) { - if (!titledByAppBar) RecipesHeading(state.screen) - Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - Text(stringResource(Res.string.no_recipes_found)) - } - } - } else { - LazyVerticalGrid( - cells, + AnimatedContent(state) { targetState -> + when (targetState) { + is RecipesState.Error -> + ErrorDisplay( + onRetryClick = { targetState.eventSink(RecipesEvent.Error.RetryClicked) }, modifier = modifier.fillMaxSize(), - verticalArrangement = Arrangement.spacedBy(12.dp), - horizontalArrangement = Arrangement.spacedBy(12.dp), - contentPadding = padding, - ) { - // Inside the grid rather than above it: it picks up the same contentPadding as the cards - // it heads, so the two line up with no second padding calculation, and it scrolls away - // with them -- which is what a short window wants from a headline. - // Skipped only when the top app bar is already showing this name, which under Cupertino - // it usually is -- but not on a layout wide enough for the navigation rail, where the - // search field takes the whole bar and this heading is the only name the screen has. - if (!titledByAppBar) { - item(span = { GridItemSpan(maxLineSpan) }, contentType = "heading") { - RecipesHeading(state.screen) + ) + RecipesState.Loading -> LoadingScreen(modifier) + is RecipesState.Success -> { + if (targetState.recipes.isEmpty()) { + Column(modifier.fillMaxSize().padding(padding)) { + if (!titledByAppBar) RecipesHeading(targetState.screen) + Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + Text(stringResource(Res.string.no_recipes_found)) } } - items(state.recipes, key = { it.id }, contentType = { "recipe_item" }) { - RecipeCard( - it, - showAreaLabel = state.showAreaLabel, - horizontalCards = horizontalCards, - onClick = { state.eventSink(RecipesEvent.Success.RecipeClicked(it.id)) }, - Modifier.animateItem(), - ) + } else { + Column(modifier.fillMaxSize()) { + AnimatedVisibility(targetState.isRefreshing) { + LinearProgressIndicator(modifier = Modifier.fillMaxWidth()) + } + LazyVerticalGrid( + cells, + modifier = Modifier.fillMaxSize(), + verticalArrangement = Arrangement.spacedBy(12.dp), + horizontalArrangement = Arrangement.spacedBy(12.dp), + contentPadding = padding, + ) { + + // Inside the grid rather than above it: it picks up the same contentPadding as the + // cards + // it heads, so the two line up with no second padding calculation, and it scrolls + // away + // with them -- which is what a short window wants from a headline. + // Skipped only when the top app bar is already showing this name, which under + // Cupertino + // it usually is -- but not on a layout wide enough for the navigation rail, where the + // search field takes the whole bar and this heading is the only name the screen has. + if (!titledByAppBar) { + item(span = { GridItemSpan(maxLineSpan) }, contentType = "heading") { + RecipesHeading(targetState.screen) + } + } + items(targetState.recipes, key = { it.id }, contentType = { "recipe_item" }) { + RecipeCard( + it, + showAreaLabel = targetState.showAreaLabel, + horizontalCards = horizontalCards, + onClick = { targetState.eventSink(RecipesEvent.Success.RecipeClicked(it.id)) }, + Modifier.animateItem(), + ) + } + } } } } @@ -105,6 +117,11 @@ fun RecipesScreen(state: RecipesState, modifier: Modifier = Modifier) { } } +@Composable +private fun LoadingScreen(modifier: Modifier = Modifier) { + Box(modifier.fillMaxSize(), contentAlignment = Alignment.Center) { CircularProgressIndicator() } +} + /** * Names the list -- `Category: Seafood`, `Favorites`, `Results for "chicken"`. * From de7415c7cff3d557d84941512680b87ecd2434d1 Mon Sep 17 00:00:00 2001 From: Scott Olcott Date: Sat, 12 Sep 2026 18:55:22 -0600 Subject: [PATCH 3/3] Fix recipe details fold, favorite toggle and state animations 48d68d5 moved RecipeDetailsPresenter onto foldToState and animated every screen's state changes, and shipped three bugs with it. The single-item foldToState handed hasAnswer a non-null test where it takes an isEmpty one. A cached recipe therefore counted as a cache miss the moment a refresh started and dropped to a full-screen spinner, and any settled null -- an id the API does not know, reached by a deep link -- hit checkNotNull and threw in the presenter. A non-null item is always an answer, so the fold now reads the item directly: an item is content (refreshing while a request is in flight), none while loading is Loading, none once settled is Error. The details success sink was remembered along with the recipe it closed over, which on the first composition is always null, so the favorite button did nothing. It now reads the recipe through rememberUpdatedState, and retryTrigger is retained like every other presenter's. AnimatedContent(state) keyed each state on itself, so every new Success -- a favorite toggled, a refresh finishing -- animated into a freshly composed subtree and reset grid and scroll positions to the top. AnimatedStateContent keys on the state's class instead: a change of type animates, a same-type update recomposes in place. Also: RefreshingContent lays the refresh bar over the content instead of pushing it down, LoadingDisplay sits beside ErrorDisplay, the three tab screens drop their duplicated wrappers, and each screen applies its modifier at the root. RecipeDetailsPresenterTest covers the three bugs; three of its four cases fail against the previous code. Co-Authored-By: Claude Opus 5 Signed-off-by: Scott Olcott --- .../recipe/domain/presenter/Content.kt | 33 ++-- .../presenter/RecipeDetailsPresenter.kt | 21 ++- .../presenter/RecipeDetailsPresenterTest.kt | 177 ++++++++++++++++++ .../com/scottolcott/recipe/ui/ErrorDisplay.kt | 3 +- .../com/scottolcott/recipe/ui/StateContent.kt | 88 +++++++++ .../scottolcott/recipe/ui/area/AreasScreen.kt | 64 +++---- .../recipe/ui/category/CategoriesScreen.kt | 64 +++---- .../recipe/ui/ingredient/IngredientsScreen.kt | 64 +++---- .../recipe/ui/recipe/RecipeDetailsScreen.kt | 36 ++-- .../recipe/ui/recipe/RecipesScreen.kt | 53 ++---- 10 files changed, 413 insertions(+), 190 deletions(-) create mode 100644 domain/src/commonTest/kotlin/com/scottolcott/recipe/domain/presenter/RecipeDetailsPresenterTest.kt create mode 100644 ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/StateContent.kt diff --git a/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/Content.kt b/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/Content.kt index 6742c24..9cec3f4 100644 --- a/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/Content.kt +++ b/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/Content.kt @@ -2,34 +2,27 @@ package com.scottolcott.recipe.domain.presenter import io.github.solcott.uistate.ContentState import io.github.solcott.uistate.errorOrNull -import io.github.solcott.uistate.hasAnswer import io.github.solcott.uistate.isLoading /** - * The three states every screen renders, decoupled from any one screen's `CircuitUiState`. + * [foldToState] for a screen that shows one optional item, such as a recipe by id. * - * Each tab keeps its own state type — Circuit pairs a state with exactly one UI — but the mapping - * from a [ContentState] onto these three cases is the same everywhere, so it lives here rather than - * three times over. - * - * The order of the branches is the point. Having data wins over being in flight: [ContentState] - * keeps the last item it loaded, so a background refresh reports it as content that is refreshing - * rather than dropping the grid for a spinner. - * - * `hasAnswer` is what separates the first two branches, not `data == null` — a null is a real - * answer, and reading it as "nothing yet" would leave a spinner over a legitimately empty tab - * forever. What it adds over `hasLoaded` is the one empty list that is *not* an answer: an empty - * read from cache while its request is still in flight or has failed. That is a key the database - * has never seen, so it gets the spinner, or the error, rather than "nothing found". The - * repositories already hold that read back with `asOutcomes(fetching = …)`; this is the backstop. + * Simpler than the list version, because a non-null item is always an answer: the one read + * `hasAnswer` exists to refuse -- an empty read from cache while its request is still outstanding + * -- is a null here, and a null is never content. So having the item wins over being in flight (a + * refresh renders as content that is refreshing, not a spinner); no item while in flight is still + * loading; and no item once settled is an error -- the request failed, or the source answered that + * there is no such item. */ -internal inline fun ContentState.foldToState( +internal inline fun ContentState.foldToState( onLoading: () -> S, onError: (message: String) -> S, onContent: (item: T, isRefreshing: Boolean) -> S, -): S = - when { - hasAnswer { it != null } -> onContent(checkNotNull(data) { "data was null." }, isLoading) +): S { + val item = data + return when { + item != null -> onContent(item, isLoading) isLoading -> onLoading() else -> onError(errorOrNull.toMessage()) } +} diff --git a/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/RecipeDetailsPresenter.kt b/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/RecipeDetailsPresenter.kt index 6ffa286..e79fcdd 100644 --- a/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/RecipeDetailsPresenter.kt +++ b/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/RecipeDetailsPresenter.kt @@ -5,6 +5,8 @@ import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.rememberUpdatedState +import androidx.compose.runtime.retain.retain import androidx.compose.runtime.setValue import com.scottolcott.recipe.domain.presenter.RecipesScreen.ByArea import com.scottolcott.recipe.domain.presenter.RecipesScreen.ByCategory @@ -35,25 +37,24 @@ internal constructor( @Composable override fun present(): RecipeDetailsState { val coroutineScope = rememberCoroutineScope() - var retryTrigger by remember { mutableIntStateOf(0) } + var retryTrigger by retain { mutableIntStateOf(0) } val state = produceRetainedContentState(null, retryTrigger) { recipeRepository.getById(screen.id) } - // Unlike the old response-shaped state, this keeps the recipe on screen through a refresh - // rather than blanking it whenever the store goes back to the network. - val recipe: Recipe? = state.data + // Read through updated state: the sink below is remembered once, so closing over a plain local + // would pin it to the recipe from the first composition -- which is always null. + val latestRecipe by rememberUpdatedState(state.data) val successEventSink: (RecipeDetailsEvent.Success) -> Unit = remember { { event -> when (event) { RecipeDetailsEvent.Success.ToggleFavorite -> coroutineScope.launch { - if (recipe != null) { - if (recipe.favorite) { - recipeRepository.removeFavorite(screen.id) - } else { - recipeRepository.addFavorite(screen.id) - } + val recipe = latestRecipe ?: return@launch + if (recipe.favorite) { + recipeRepository.removeFavorite(screen.id) + } else { + recipeRepository.addFavorite(screen.id) } } is RecipeDetailsEvent.Success.CategoryClicked -> diff --git a/domain/src/commonTest/kotlin/com/scottolcott/recipe/domain/presenter/RecipeDetailsPresenterTest.kt b/domain/src/commonTest/kotlin/com/scottolcott/recipe/domain/presenter/RecipeDetailsPresenterTest.kt new file mode 100644 index 0000000..e1950bf --- /dev/null +++ b/domain/src/commonTest/kotlin/com/scottolcott/recipe/domain/presenter/RecipeDetailsPresenterTest.kt @@ -0,0 +1,177 @@ +package com.scottolcott.recipe.domain.presenter + +import com.scottolcott.recipe.model.Recipe +import com.scottolcott.recipe.model.RecipeId +import com.scottolcott.recipe.repository.RecipeRepository +import com.slack.circuit.test.FakeNavigator +import com.slack.circuit.test.test +import de.infix.testBalloon.framework.core.testSuite +import io.github.solcott.dataresult.DataError +import io.github.solcott.dataresult.Origin +import io.github.solcott.dataresult.Outcome +import kotlin.test.assertEquals +import kotlin.test.assertIs +import kotlin.time.Clock +import kotlinx.coroutines.awaitCancellation +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.emitAll +import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.flow.flowOf + +private class FakeRecipeDetailsRepository : RecipeRepository { + // Driven emission by emission: a flow that emits several values up front collapses into a single + // recomposition, which would hide the transitions these tests are about. + private val responses = MutableSharedFlow>(replay = 1) + private var stored: Recipe? = null + + var getByIdHandler: () -> Flow> = { responses } + + /** Counts how many times the recipe flow was *collected*, not how many times it was built. */ + var subscriptions = 0 + private set + + /** Every favorite write, in order: `true` for an add, `false` for a remove. */ + val favoriteWrites = mutableListOf() + + suspend fun emit(outcome: Outcome) { + if (outcome is Outcome.Data) stored = outcome.data + responses.emit(outcome) + } + + override fun getById(id: RecipeId): Flow> = flow { + subscriptions++ + emitAll(getByIdHandler()) + } + + // Room re-emits the row after a favorite write, so this does too. + override suspend fun addFavorite(id: RecipeId) = writeFavorite(true) + + override suspend fun removeFavorite(id: RecipeId) = writeFavorite(false) + + private suspend fun writeFavorite(favorite: Boolean) { + favoriteWrites += favorite + stored?.let { emit(Outcome.Data(it.copy(favorite = favorite), Origin.Cache)) } + } + + override fun recipesByIngredients(ingredients: Set): Flow>> = + emptyFlow() + + override fun searchRecipes(query: String): Flow>> = emptyFlow() + + override fun recipesByCategory(category: String): Flow>> = emptyFlow() + + override fun recipesByArea(area: String): Flow>> = emptyFlow() + + override fun getFavoritesAsFlow(): Flow>> = emptyFlow() +} + +private val screen = RecipeDetailsScreen(RecipeId("1")) + +private fun presenterFor(repository: RecipeRepository) = + RecipeDetailsPresenter(screen, FakeNavigator(screen), repository) + +private fun recipe(favorite: Boolean = false) = + Recipe( + id = screen.id, + name = "Recipe 1", + thumbnail = "thumb1", + category = null, + area = null, + favorite = favorite, + details = null, + lastFetched = Clock.System.now(), + ) + +val recipeDetailsPresenterTests by testSuite { + // Regression: the single-item fold once handed `hasAnswer` a non-null test where it takes an + // `isEmpty` one, so a cached recipe counted as a miss the moment the network was asked, and the + // screen dropped to a full-screen spinner over a recipe it already had. + test("a cached recipe stays on screen while it refreshes") { + val repository = FakeRecipeDetailsRepository() + // One instance: `recipe()` stamps `lastFetched` with the current time. + val cached = recipe() + repository.emit(Outcome.Data(cached, Origin.Cache)) + + presenterFor(repository).test { + var state = awaitItem() + if (state is RecipeDetailsState.Loading) state = awaitItem() + assertIs(state) + assertEquals(false, state.isRefreshing) + + repository.emit(Outcome.Loading) + + val refreshing = assertIs(awaitItem()) + assertEquals(true, refreshing.isRefreshing) + assertEquals(cached, refreshing.recipe) + } + } + + // Regression: a settled null -- an id the API does not know, reached by a deep link -- is an + // answer, and the fold's `checkNotNull` threw on it inside the presenter. + test("a recipe that does not exist is an error, not a crash") { + val repository = FakeRecipeDetailsRepository() + repository.emit(Outcome.Data(null, Origin.Network)) + + presenterFor(repository).test { + var state = awaitItem() + if (state is RecipeDetailsState.Loading) state = awaitItem() + assertIs(state) + } + } + + // Regression: the success sink was remembered along with the recipe it closed over, which on the + // first composition is always null -- so the favorite button did nothing at all. + test("toggling favorite adds it and then removes it") { + val repository = FakeRecipeDetailsRepository() + repository.emit(Outcome.Data(recipe(favorite = false), Origin.Network)) + + presenterFor(repository).test { + var state = awaitItem() + if (state is RecipeDetailsState.Loading) state = awaitItem() + assertIs(state) + + state.eventSink(RecipeDetailsEvent.Success.ToggleFavorite) + val favorited = assertIs(awaitItem()) + assertEquals(true, favorited.recipe.favorite) + + favorited.eventSink(RecipeDetailsEvent.Success.ToggleFavorite) + val unfavorited = assertIs(awaitItem()) + assertEquals(false, unfavorited.recipe.favorite) + + assertEquals(listOf(true, false), repository.favoriteWrites) + } + } + + test("retry asks for the recipe again") { + val repository = FakeRecipeDetailsRepository() + repository.getByIdHandler = { + if (repository.subscriptions == 1) { + flowOf(Outcome.Error(DataError.Network, Origin.Network)) + } else { + inFlight() + } + } + + presenterFor(repository).test { + var state = awaitItem() + if (state is RecipeDetailsState.Loading) state = awaitItem() + assertIs(state) + + state.eventSink(RecipeDetailsEvent.Error.RetryClicked) + + assertIs(awaitItem()) + assertEquals(2, repository.subscriptions) + } + } +} + +/** + * A request that is still in flight: it reports loading and then stays open. Not + * `flowOf(Outcome.Loading)`, which completes and so reads as a settled empty result. + */ +private fun inFlight(): Flow> = flow { + emit(Outcome.Loading) + awaitCancellation() +} diff --git a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/ErrorDisplay.kt b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/ErrorDisplay.kt index 8b677e8..3999a32 100644 --- a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/ErrorDisplay.kt +++ b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/ErrorDisplay.kt @@ -5,6 +5,7 @@ import androidx.compose.foundation.layout.Column import androidx.compose.material3.Button import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import org.jetbrains.compose.resources.stringResource @@ -12,7 +13,7 @@ import org.jetbrains.compose.resources.stringResource fun ErrorDisplay(onRetryClick: () -> Unit, modifier: Modifier = Modifier) { Column( modifier = modifier, - horizontalAlignment = androidx.compose.ui.Alignment.CenterHorizontally, + horizontalAlignment = Alignment.CenterHorizontally, verticalArrangement = Arrangement.Center, ) { Text(stringResource(Res.string.an_error_occurred)) diff --git a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/StateContent.kt b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/StateContent.kt new file mode 100644 index 0000000..c8c9e20 --- /dev/null +++ b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/StateContent.kt @@ -0,0 +1,88 @@ +package com.scottolcott.recipe.ui + +import androidx.compose.animation.AnimatedContent +import androidx.compose.animation.AnimatedContentScope +import androidx.compose.animation.AnimatedContentTransitionScope +import androidx.compose.animation.AnimatedVisibility +import androidx.compose.animation.ContentTransform +import androidx.compose.animation.core.tween +import androidx.compose.animation.fadeIn +import androidx.compose.animation.fadeOut +import androidx.compose.animation.scaleIn +import androidx.compose.animation.togetherWith +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.LinearProgressIndicator +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier + +/** + * [AnimatedContent] for a screen's state, keyed on the state's *type* (`state::class`) rather than + * the state itself. + * + * Moving between types -- Loading to Success, Success to Error -- runs [transitionSpec] as usual. A + * new state of the *same* type -- a Success with fresh data, a refresh finishing, a favorite + * toggled -- does not animate: it takes over the slot already on screen, and [content] recomposes + * in place with it. Keyed on the state itself, as plain [AnimatedContent] is, each of those updates + * would animate into a freshly composed subtree, and every grid and scroll position inside it would + * start over at the top. + * + * The parameters mirror [AnimatedContent]'s, defaults included. The trade-off is that two states of + * the same class can never animate between each other; a screen that wants that should call + * [AnimatedContent] with a key of its own. + */ +@Composable +fun AnimatedStateContent( + state: S, + modifier: Modifier = Modifier, + transitionSpec: AnimatedContentTransitionScope.() -> ContentTransform = { + (fadeIn(animationSpec = tween(220, delayMillis = 90)) + + scaleIn(initialScale = 0.92f, animationSpec = tween(220, delayMillis = 90))) + .togetherWith(fadeOut(animationSpec = tween(90))) + }, + contentAlignment: Alignment = Alignment.TopStart, + label: String = "AnimatedStateContent", + content: @Composable AnimatedContentScope.(targetState: S) -> Unit, +) { + AnimatedContent( + state, + modifier, + transitionSpec = transitionSpec, + contentAlignment = contentAlignment, + label = label, + contentKey = { it::class }, + content = content, + ) +} + +/** + * [content] with a progress bar over its top edge while [isRefreshing]. + * + * Over it rather than above it, so the content does not jump down and back up as a refresh starts + * and ends. + */ +@Composable +fun RefreshingContent( + isRefreshing: Boolean, + modifier: Modifier = Modifier, + content: @Composable () -> Unit, +) { + Box(modifier) { + content() + AnimatedVisibility( + isRefreshing, + Modifier.align(Alignment.TopCenter).fillMaxWidth(), + enter = fadeIn(), + exit = fadeOut(), + ) { + LinearProgressIndicator(Modifier.fillMaxWidth()) + } + } +} + +@Composable +fun LoadingDisplay(modifier: Modifier = Modifier) { + Box(modifier, contentAlignment = Alignment.Center) { CircularProgressIndicator() } +} diff --git a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/area/AreasScreen.kt b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/area/AreasScreen.kt index 11a7ccc..ff93707 100644 --- a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/area/AreasScreen.kt +++ b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/area/AreasScreen.kt @@ -1,6 +1,5 @@ package com.scottolcott.recipe.ui.area -import androidx.compose.animation.AnimatedContent import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -11,7 +10,6 @@ import androidx.compose.foundation.layout.padding import androidx.compose.foundation.lazy.grid.LazyVerticalGrid import androidx.compose.foundation.lazy.grid.items import androidx.compose.foundation.text.TextAutoSize -import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable @@ -32,7 +30,9 @@ import com.scottolcott.recipe.domain.presenter.AreasEvent import com.scottolcott.recipe.domain.presenter.AreasScreen import com.scottolcott.recipe.domain.presenter.AreasState import com.scottolcott.recipe.model.Area +import com.scottolcott.recipe.ui.AnimatedStateContent import com.scottolcott.recipe.ui.ErrorDisplay +import com.scottolcott.recipe.ui.LoadingDisplay import com.scottolcott.recipe.ui.Res import com.scottolcott.recipe.ui.design.AppCard import com.scottolcott.recipe.ui.isShortWindow @@ -63,41 +63,37 @@ fun AreasScreen(state: AreasState, modifier: Modifier = Modifier) { // The country is the same word again for most of the list -- Algerian, Algeria -- so it reads as // the gloss it is rather than a second title. val countryTextStyle = MaterialTheme.typography.bodyMedium - Box(modifier, contentAlignment = Alignment.TopCenter) { - AnimatedContent(state) { targetState -> - when (targetState) { - is AreasState.Error -> - Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - ErrorDisplay(onRetryClick = { targetState.eventSink(AreasEvent.Error.RetryClicked) }) - } + AnimatedStateContent(state, modifier) { targetState -> + when (targetState) { + is AreasState.Error -> + ErrorDisplay( + onRetryClick = { targetState.eventSink(AreasEvent.Error.RetryClicked) }, + modifier = Modifier.fillMaxSize(), + ) + + AreasState.Loading -> LoadingDisplay(Modifier.fillMaxSize()) - AreasState.Loading -> + is AreasState.Success -> { + if (targetState.areas.isEmpty()) { Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - CircularProgressIndicator() + Text(stringResource(Res.string.no_areas_found)) } - - is AreasState.Success -> { - if (targetState.areas.isEmpty()) { - Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - Text(stringResource(Res.string.no_areas_found)) - } - } else { - LazyVerticalGrid( - cells, - modifier = Modifier.fillMaxSize(), - verticalArrangement = Arrangement.spacedBy(12.dp), - horizontalArrangement = Arrangement.spacedBy(12.dp), - contentPadding = padding, - ) { - items(targetState.areas, key = { it.area }, contentType = { "area_item" }) { area -> - AreaItem( - area, - areaTextStyle = areaTextStyle, - countryTextStyle = countryTextStyle, - { targetState.eventSink(AreasEvent.Success.AreaClicked(area.area)) }, - Modifier.animateItem().pointerHoverIcon(PointerIcon.Hand, true), - ) - } + } else { + LazyVerticalGrid( + cells, + modifier = Modifier.fillMaxSize(), + verticalArrangement = Arrangement.spacedBy(12.dp), + horizontalArrangement = Arrangement.spacedBy(12.dp), + contentPadding = padding, + ) { + items(targetState.areas, key = { it.area }, contentType = { "area_item" }) { area -> + AreaItem( + area, + areaTextStyle = areaTextStyle, + countryTextStyle = countryTextStyle, + { targetState.eventSink(AreasEvent.Success.AreaClicked(area.area)) }, + Modifier.animateItem().pointerHoverIcon(PointerIcon.Hand, true), + ) } } } diff --git a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/category/CategoriesScreen.kt b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/category/CategoriesScreen.kt index 8e965cd..c999aeb 100644 --- a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/category/CategoriesScreen.kt +++ b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/category/CategoriesScreen.kt @@ -2,7 +2,6 @@ package com.scottolcott.recipe.ui.category -import androidx.compose.animation.AnimatedContent import androidx.compose.foundation.background import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -12,7 +11,6 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.lazy.grid.LazyVerticalGrid import androidx.compose.foundation.lazy.grid.items -import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text @@ -33,7 +31,9 @@ import com.scottolcott.recipe.domain.presenter.CategoriesEvent import com.scottolcott.recipe.domain.presenter.CategoriesScreen import com.scottolcott.recipe.domain.presenter.CategoriesState import com.scottolcott.recipe.model.Category +import com.scottolcott.recipe.ui.AnimatedStateContent import com.scottolcott.recipe.ui.ErrorDisplay +import com.scottolcott.recipe.ui.LoadingDisplay import com.scottolcott.recipe.ui.Res import com.scottolcott.recipe.ui.design.AppCard import com.scottolcott.recipe.ui.isShortWindow @@ -61,42 +61,36 @@ fun CategoriesScreen(state: CategoriesState, modifier: Modifier = Modifier) { } else { MaterialTheme.typography.titleSmallEmphasized } - Box(modifier, contentAlignment = Alignment.TopCenter) { - AnimatedContent(state) { targetState -> - when (targetState) { - is CategoriesState.Error -> - Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - ErrorDisplay( - onRetryClick = { targetState.eventSink(CategoriesEvent.Error.RetryClicked) } - ) - } + AnimatedStateContent(state, modifier) { targetState -> + when (targetState) { + is CategoriesState.Error -> + ErrorDisplay( + onRetryClick = { targetState.eventSink(CategoriesEvent.Error.RetryClicked) }, + modifier = Modifier.fillMaxSize(), + ) + + CategoriesState.Loading -> LoadingDisplay(Modifier.fillMaxSize()) - CategoriesState.Loading -> + is CategoriesState.Success -> { + if (targetState.categories.isEmpty()) { Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - CircularProgressIndicator() + Text(stringResource(Res.string.no_categories_found)) } - - is CategoriesState.Success -> { - if (targetState.categories.isEmpty()) { - Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - Text(stringResource(Res.string.no_categories_found)) - } - } else { - LazyVerticalGrid( - cells, - modifier = Modifier.fillMaxSize(), - verticalArrangement = Arrangement.spacedBy(12.dp), - horizontalArrangement = Arrangement.spacedBy(12.dp), - contentPadding = padding, - ) { - items(targetState.categories, key = { it.id }, contentType = { "category_item" }) { - CategoryItem( - it, - labelTextStyle = labelTextStyle, - { targetState.eventSink(CategoriesEvent.Success.CategoryClicked(it.name)) }, - Modifier.animateItem().pointerHoverIcon(PointerIcon.Hand, true), - ) - } + } else { + LazyVerticalGrid( + cells, + modifier = Modifier.fillMaxSize(), + verticalArrangement = Arrangement.spacedBy(12.dp), + horizontalArrangement = Arrangement.spacedBy(12.dp), + contentPadding = padding, + ) { + items(targetState.categories, key = { it.id }, contentType = { "category_item" }) { + CategoryItem( + it, + labelTextStyle = labelTextStyle, + { targetState.eventSink(CategoriesEvent.Success.CategoryClicked(it.name)) }, + Modifier.animateItem().pointerHoverIcon(PointerIcon.Hand, true), + ) } } } diff --git a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/ingredient/IngredientsScreen.kt b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/ingredient/IngredientsScreen.kt index 606e318..6db3542 100644 --- a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/ingredient/IngredientsScreen.kt +++ b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/ingredient/IngredientsScreen.kt @@ -1,6 +1,5 @@ package com.scottolcott.recipe.ui.ingredient -import androidx.compose.animation.AnimatedContent import androidx.compose.foundation.background import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -10,7 +9,6 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.lazy.grid.LazyVerticalGrid import androidx.compose.foundation.lazy.grid.items -import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable @@ -30,7 +28,9 @@ import com.scottolcott.recipe.domain.presenter.IngredientsEvent import com.scottolcott.recipe.domain.presenter.IngredientsScreen import com.scottolcott.recipe.domain.presenter.IngredientsState import com.scottolcott.recipe.model.Ingredient +import com.scottolcott.recipe.ui.AnimatedStateContent import com.scottolcott.recipe.ui.ErrorDisplay +import com.scottolcott.recipe.ui.LoadingDisplay import com.scottolcott.recipe.ui.Res import com.scottolcott.recipe.ui.design.AppCard import com.scottolcott.recipe.ui.isShortWindow @@ -58,42 +58,36 @@ fun IngredientsScreen(state: IngredientsState, modifier: Modifier = Modifier) { } else { MaterialTheme.typography.titleSmallEmphasized } - Box(modifier, contentAlignment = Alignment.TopCenter) { - AnimatedContent(state) { targetState -> - when (targetState) { - is IngredientsState.Error -> - Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - ErrorDisplay( - onRetryClick = { targetState.eventSink(IngredientsEvent.Error.RetryClicked) } - ) - } + AnimatedStateContent(state, modifier) { targetState -> + when (targetState) { + is IngredientsState.Error -> + ErrorDisplay( + onRetryClick = { targetState.eventSink(IngredientsEvent.Error.RetryClicked) }, + modifier = Modifier.fillMaxSize(), + ) + + IngredientsState.Loading -> LoadingDisplay(Modifier.fillMaxSize()) - IngredientsState.Loading -> + is IngredientsState.Success -> { + if (targetState.ingredients.isEmpty()) { Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - CircularProgressIndicator() + Text(stringResource(Res.string.no_ingredients_found)) } - - is IngredientsState.Success -> { - if (targetState.ingredients.isEmpty()) { - Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - Text(stringResource(Res.string.no_ingredients_found)) - } - } else { - LazyVerticalGrid( - cells, - modifier = Modifier.fillMaxSize(), - verticalArrangement = Arrangement.spacedBy(12.dp), - horizontalArrangement = Arrangement.spacedBy(12.dp), - contentPadding = padding, - ) { - items(targetState.ingredients, key = { it.id }, contentType = { "ingredient_item" }) { - IngredientItem( - it, - labelTextStyle = labelTextStyle, - { targetState.eventSink(IngredientsEvent.Success.IngredientClicked(it.name)) }, - Modifier.animateItem().pointerHoverIcon(PointerIcon.Hand, true), - ) - } + } else { + LazyVerticalGrid( + cells, + modifier = Modifier.fillMaxSize(), + verticalArrangement = Arrangement.spacedBy(12.dp), + horizontalArrangement = Arrangement.spacedBy(12.dp), + contentPadding = padding, + ) { + items(targetState.ingredients, key = { it.id }, contentType = { "ingredient_item" }) { + IngredientItem( + it, + labelTextStyle = labelTextStyle, + { targetState.eventSink(IngredientsEvent.Success.IngredientClicked(it.name)) }, + Modifier.animateItem().pointerHoverIcon(PointerIcon.Hand, true), + ) } } } diff --git a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipeDetailsScreen.kt b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipeDetailsScreen.kt index 6092035..1a7056b 100644 --- a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipeDetailsScreen.kt +++ b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipeDetailsScreen.kt @@ -1,7 +1,5 @@ package com.scottolcott.recipe.ui.recipe -import androidx.compose.animation.AnimatedContent -import androidx.compose.animation.AnimatedVisibility import androidx.compose.animation.animateBounds import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -23,11 +21,9 @@ import androidx.compose.foundation.text.selection.SelectionContainer import androidx.compose.foundation.verticalScroll import androidx.compose.material3.AssistChip import androidx.compose.material3.AssistChipDefaults -import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi import androidx.compose.material3.Icon -import androidx.compose.material3.LinearProgressIndicator import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable @@ -65,7 +61,10 @@ import com.scottolcott.recipe.model.Recipe import com.scottolcott.recipe.model.RecipeDetails import com.scottolcott.recipe.model.RecipeId import com.scottolcott.recipe.model.RecipeIngredient +import com.scottolcott.recipe.ui.AnimatedStateContent import com.scottolcott.recipe.ui.ErrorDisplay +import com.scottolcott.recipe.ui.LoadingDisplay +import com.scottolcott.recipe.ui.RefreshingContent import com.scottolcott.recipe.ui.Res import com.scottolcott.recipe.ui.ThemeWrapper import com.scottolcott.recipe.ui.image_24px @@ -84,25 +83,18 @@ import org.jetbrains.compose.resources.stringResource @Composable @CircuitInject(RecipeDetailsScreen::class, AppScope::class) fun RecipeDetailsScreen(state: RecipeDetailsState, modifier: Modifier = Modifier) { - Box(modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - AnimatedContent(state) { targetState -> - when (targetState) { - is RecipeDetailsState.Error -> - ErrorDisplay( - onRetryClick = { targetState.eventSink(RecipeDetailsEvent.Error.RetryClicked) } - ) - RecipeDetailsState.Loading -> CircularProgressIndicator() - is RecipeDetailsState.Success -> { - RecipeDetails( - targetState.recipe, - targetState.eventSink, - modifier = Modifier.fillMaxSize(), - ) - AnimatedVisibility(targetState.isRefreshing) { - LinearProgressIndicator(modifier = Modifier.fillMaxWidth().align(Alignment.TopStart)) - } + AnimatedStateContent(state, modifier.fillMaxSize()) { targetState -> + when (targetState) { + is RecipeDetailsState.Error -> + ErrorDisplay( + onRetryClick = { targetState.eventSink(RecipeDetailsEvent.Error.RetryClicked) }, + modifier = Modifier.fillMaxSize(), + ) + RecipeDetailsState.Loading -> LoadingDisplay(Modifier.fillMaxSize()) + is RecipeDetailsState.Success -> + RefreshingContent(targetState.isRefreshing, Modifier.fillMaxSize()) { + RecipeDetails(targetState.recipe, targetState.eventSink, Modifier.fillMaxSize()) } - } } } } diff --git a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipesScreen.kt b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipesScreen.kt index eed0323..f6661e7 100644 --- a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipesScreen.kt +++ b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/recipe/RecipesScreen.kt @@ -1,7 +1,5 @@ package com.scottolcott.recipe.ui.recipe -import androidx.compose.animation.AnimatedContent -import androidx.compose.animation.AnimatedVisibility import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -14,8 +12,6 @@ import androidx.compose.foundation.layout.size import androidx.compose.foundation.lazy.grid.GridItemSpan import androidx.compose.foundation.lazy.grid.LazyVerticalGrid import androidx.compose.foundation.lazy.grid.items -import androidx.compose.material3.CircularProgressIndicator -import androidx.compose.material3.LinearProgressIndicator import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable @@ -34,7 +30,10 @@ import com.scottolcott.recipe.domain.presenter.RecipesEvent import com.scottolcott.recipe.domain.presenter.RecipesScreen import com.scottolcott.recipe.domain.presenter.RecipesState import com.scottolcott.recipe.model.Recipe +import com.scottolcott.recipe.ui.AnimatedStateContent import com.scottolcott.recipe.ui.ErrorDisplay +import com.scottolcott.recipe.ui.LoadingDisplay +import com.scottolcott.recipe.ui.RefreshingContent import com.scottolcott.recipe.ui.Res import com.scottolcott.recipe.ui.design.AppCard import com.scottolcott.recipe.ui.isShortWindow @@ -57,27 +56,24 @@ fun RecipesScreen(state: RecipesState, modifier: Modifier = Modifier) { val horizontalCards = isShortWindow() // Whoever the bar is not naming has to name itself; see [LocalAppBarShowsScreenTitle]. val titledByAppBar = LocalAppBarShowsScreenTitle.current - AnimatedContent(state) { targetState -> + AnimatedStateContent(state, modifier) { targetState -> when (targetState) { is RecipesState.Error -> ErrorDisplay( onRetryClick = { targetState.eventSink(RecipesEvent.Error.RetryClicked) }, - modifier = modifier.fillMaxSize(), + modifier = Modifier.fillMaxSize(), ) - RecipesState.Loading -> LoadingScreen(modifier) - is RecipesState.Success -> { - if (targetState.recipes.isEmpty()) { - Column(modifier.fillMaxSize().padding(padding)) { - if (!titledByAppBar) RecipesHeading(targetState.screen) - Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - Text(stringResource(Res.string.no_recipes_found)) - } - } - } else { - Column(modifier.fillMaxSize()) { - AnimatedVisibility(targetState.isRefreshing) { - LinearProgressIndicator(modifier = Modifier.fillMaxWidth()) + RecipesState.Loading -> LoadingDisplay(Modifier.fillMaxSize()) + is RecipesState.Success -> + RefreshingContent(targetState.isRefreshing, Modifier.fillMaxSize()) { + if (targetState.recipes.isEmpty()) { + Column(Modifier.fillMaxSize().padding(padding)) { + if (!titledByAppBar) RecipesHeading(targetState.screen) + Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + Text(stringResource(Res.string.no_recipes_found)) + } } + } else { LazyVerticalGrid( cells, modifier = Modifier.fillMaxSize(), @@ -85,16 +81,13 @@ fun RecipesScreen(state: RecipesState, modifier: Modifier = Modifier) { horizontalArrangement = Arrangement.spacedBy(12.dp), contentPadding = padding, ) { - // Inside the grid rather than above it: it picks up the same contentPadding as the - // cards - // it heads, so the two line up with no second padding calculation, and it scrolls - // away - // with them -- which is what a short window wants from a headline. + // cards it heads, so the two line up with no second padding calculation, and it + // scrolls away with them -- which is what a short window wants from a headline. // Skipped only when the top app bar is already showing this name, which under - // Cupertino - // it usually is -- but not on a layout wide enough for the navigation rail, where the - // search field takes the whole bar and this heading is the only name the screen has. + // Cupertino it usually is -- but not on a layout wide enough for the navigation + // rail, where the search field takes the whole bar and this heading is the only name + // the screen has. if (!titledByAppBar) { item(span = { GridItemSpan(maxLineSpan) }, contentType = "heading") { RecipesHeading(targetState.screen) @@ -112,16 +105,10 @@ fun RecipesScreen(state: RecipesState, modifier: Modifier = Modifier) { } } } - } } } } -@Composable -private fun LoadingScreen(modifier: Modifier = Modifier) { - Box(modifier.fillMaxSize(), contentAlignment = Alignment.Center) { CircularProgressIndicator() } -} - /** * Names the list -- `Category: Seafood`, `Favorites`, `Results for "chicken"`. *