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/Content.kt b/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/Content.kt new file mode 100644 index 0000000..9cec3f4 --- /dev/null +++ b/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/Content.kt @@ -0,0 +1,28 @@ +package com.scottolcott.recipe.domain.presenter + +import io.github.solcott.uistate.ContentState +import io.github.solcott.uistate.errorOrNull +import io.github.solcott.uistate.isLoading + +/** + * [foldToState] for a screen that shows one optional item, such as a recipe by id. + * + * 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( + onLoading: () -> S, + onError: (message: String) -> S, + onContent: (item: T, isRefreshing: Boolean) -> S, +): 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/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/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/RecipeDetailsPresenter.kt b/domain/src/commonMain/kotlin/com/scottolcott/recipe/domain/presenter/RecipeDetailsPresenter.kt index 7565473..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 @@ -22,8 +24,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) @@ -37,54 +37,78 @@ 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 - return RecipeDetailsState( - recipe, - loading = state.isLoading, - error = state.errorOrNull != null, - ) { event -> - when (event) { - RecipeDetailsEvent.ToggleFavorite -> - coroutineScope.launch { - if (recipe != null) { + // 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 { + val recipe = latestRecipe ?: return@launch 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/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/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() } } } 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..3999a32 100644 --- a/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/ErrorDisplay.kt +++ b/ui/src/commonMain/kotlin/com/scottolcott/recipe/ui/ErrorDisplay.kt @@ -1,15 +1,21 @@ 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 import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import org.jetbrains.compose.resources.stringResource @Composable fun ErrorDisplay(onRetryClick: () -> Unit, modifier: Modifier = Modifier) { - Column(modifier = modifier) { + Column( + modifier = modifier, + horizontalAlignment = 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/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 d95b98a..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 @@ -10,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 @@ -31,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 @@ -62,20 +63,18 @@ 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) { - when (state) { + AnimatedStateContent(state, modifier) { targetState -> + when (targetState) { is AreasState.Error -> - Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - ErrorDisplay(onRetryClick = { state.eventSink(AreasEvent.Error.RetryClicked) }) - } + ErrorDisplay( + onRetryClick = { targetState.eventSink(AreasEvent.Error.RetryClicked) }, + modifier = Modifier.fillMaxSize(), + ) - AreasState.Loading -> - Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - CircularProgressIndicator() - } + AreasState.Loading -> LoadingDisplay(Modifier.fillMaxSize()) is AreasState.Success -> { - if (state.areas.isEmpty()) { + if (targetState.areas.isEmpty()) { Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { Text(stringResource(Res.string.no_areas_found)) } @@ -87,12 +86,12 @@ fun AreasScreen(state: AreasState, modifier: Modifier = Modifier) { horizontalArrangement = Arrangement.spacedBy(12.dp), contentPadding = padding, ) { - items(state.areas, key = { it.area }, contentType = { "area_item" }) { area -> + items(targetState.areas, key = { it.area }, contentType = { "area_item" }) { area -> AreaItem( area, areaTextStyle = areaTextStyle, countryTextStyle = countryTextStyle, - { state.eventSink(AreasEvent.Success.AreaClicked(area.area)) }, + { 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..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 @@ -11,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 @@ -32,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 @@ -60,20 +61,18 @@ fun CategoriesScreen(state: CategoriesState, modifier: Modifier = Modifier) { } else { MaterialTheme.typography.titleSmallEmphasized } - Box(modifier, contentAlignment = Alignment.TopCenter) { - when (state) { + AnimatedStateContent(state, modifier) { targetState -> + when (targetState) { is CategoriesState.Error -> - Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - ErrorDisplay(onRetryClick = { state.eventSink(CategoriesEvent.Error.RetryClicked) }) - } + ErrorDisplay( + onRetryClick = { targetState.eventSink(CategoriesEvent.Error.RetryClicked) }, + modifier = Modifier.fillMaxSize(), + ) - CategoriesState.Loading -> - Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - CircularProgressIndicator() - } + CategoriesState.Loading -> LoadingDisplay(Modifier.fillMaxSize()) is CategoriesState.Success -> { - if (state.categories.isEmpty()) { + if (targetState.categories.isEmpty()) { Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { Text(stringResource(Res.string.no_categories_found)) } @@ -85,11 +84,11 @@ fun CategoriesScreen(state: CategoriesState, modifier: Modifier = Modifier) { horizontalArrangement = Arrangement.spacedBy(12.dp), contentPadding = padding, ) { - items(state.categories, key = { it.id }, contentType = { "category_item" }) { + items(targetState.categories, key = { it.id }, contentType = { "category_item" }) { CategoryItem( it, labelTextStyle = labelTextStyle, - { state.eventSink(CategoriesEvent.Success.CategoryClicked(it.name)) }, + { 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..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 @@ -9,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 @@ -29,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 @@ -57,20 +58,18 @@ fun IngredientsScreen(state: IngredientsState, modifier: Modifier = Modifier) { } else { MaterialTheme.typography.titleSmallEmphasized } - Box(modifier, contentAlignment = Alignment.TopCenter) { - when (state) { + AnimatedStateContent(state, modifier) { targetState -> + when (targetState) { is IngredientsState.Error -> - Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - ErrorDisplay(onRetryClick = { state.eventSink(IngredientsEvent.Error.RetryClicked) }) - } + ErrorDisplay( + onRetryClick = { targetState.eventSink(IngredientsEvent.Error.RetryClicked) }, + modifier = Modifier.fillMaxSize(), + ) - IngredientsState.Loading -> - Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - CircularProgressIndicator() - } + IngredientsState.Loading -> LoadingDisplay(Modifier.fillMaxSize()) is IngredientsState.Success -> { - if (state.ingredients.isEmpty()) { + if (targetState.ingredients.isEmpty()) { Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { Text(stringResource(Res.string.no_ingredients_found)) } @@ -82,11 +81,11 @@ fun IngredientsScreen(state: IngredientsState, modifier: Modifier = Modifier) { horizontalArrangement = Arrangement.spacedBy(12.dp), contentPadding = padding, ) { - items(state.ingredients, key = { it.id }, contentType = { "ingredient_item" }) { + items(targetState.ingredients, key = { it.id }, contentType = { "ingredient_item" }) { IngredientItem( it, labelTextStyle = labelTextStyle, - { state.eventSink(IngredientsEvent.Success.IngredientClicked(it.name)) }, + { 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..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 @@ -21,7 +21,6 @@ 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 @@ -62,9 +61,12 @@ 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.an_error_occurred import com.scottolcott.recipe.ui.image_24px import com.scottolcott.recipe.ui.image_source import com.scottolcott.recipe.ui.link_24px @@ -81,20 +83,28 @@ 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) { - 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) } + 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()) + } } } } @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 +118,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 +136,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 +149,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 +247,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 +391,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..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 @@ -12,7 +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.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable @@ -31,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 @@ -54,53 +56,55 @@ 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, - 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) + AnimatedStateContent(state, modifier) { targetState -> + when (targetState) { + is RecipesState.Error -> + ErrorDisplay( + onRetryClick = { targetState.eventSink(RecipesEvent.Error.RetryClicked) }, + modifier = Modifier.fillMaxSize(), + ) + 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(), + 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(), + ) + } } - } - 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(), - ) } } - } } } }