Fix cache-miss spinner and animate screen state changes - #28
Open
solcott wants to merge 3 commits into
Open
Conversation
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 <noreply@anthropic.com>
* **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 <scottolcott@gmail.com>
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 <noreply@anthropic.com> Signed-off-by: Scott Olcott <scottolcott@gmail.com>
solcott
force-pushed
the
cache-miss-spinner
branch
from
September 13, 2026 00:59
5d428ff to
de7415c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three commits:
[]for a key it has never fetched, and Store emits that before the fetch starts. Every refreshing repository stream now callsasOutcomes(fetching = refresh) { … }, which holds back that first empty read until the fetch answers. As a backstop,foldToStatecheckshasAnswerinstead ofhasLoaded.ContentStatefold and animated states.RecipeDetailsState/RecipeDetailsEventnow use the same Loading / Error / Success shape as the other screens, and every screen animates between states.foldToStatepassedhasAnswera non-null check where it expects an "is empty" one. A cached recipe dropped to a full-screen spinner as soon as a refresh started. Any recipe that loaded as null (for example an unknown id from a deep link) hitcheckNotNulland crashed the presenter.AnimatedContent(state)treated every newSuccessas new content, so a favorite toggle or a finished refresh reset grid and scroll positions to the top. The newAnimatedStateContentkeys on the state's class: a change of type animates, and a new state of the same type updates in place.RefreshingContentlays the refresh bar over the content instead of pushing it down,LoadingDisplaysits next toErrorDisplay, and the three tab screens lose their duplicated wrappers.Testing
RecipeDetailsPresenterTesthas four cases. The three that cover the bugs fail against 48d68d5; the retry case, which was never broken, passes. All domain tests pass now.ktfmtFormat sortDependenciesanddetektAllare clean.:domain:build :ui:buildcompiles for all six targets locally, including iOS, which CI skips on Linux.Dependency
This needs the
dataresultsnapshot from solcott/kmp-dataresult#5, which merged on 2026-09-12. CI pulls it from GitHub Packages, so the build only resolves once that repo's snapshot has been published.🤖 Generated with Claude Code