Standardize navigation results #130
Conversation
The picker UI component is now managed as a modal sheet at the `HomeNavHost` level, centralizing its presentation and allowing `SecondScreen` to trigger its display without direct management.
This new component allows navigation destinations to send results back to their callers, supporting serialization for Decompose navigation configurations.
Introduces ResultFlow to allow FruitPickerComponent and VegetablePickerComponent to send selected items back to the calling component (SecondComponent).
matejsemancik
requested review from
PavelMesicek,
RudolfHladik,
Syntey and
okalman
WalkthroughThe changes introduce a sheet-based picker flow integrated at the HomeNavHost level for both Android (Material3 ModalBottomSheet) and iOS (SwiftUI .sheet). Navigation is split into stack (screens) and sheet (modal) channels. A new ResultFlow enables passing picker results back to the Second screen. Picker components (Fruit/Vegetable) now use PickerScreen and send results via PickerArgs.results, then dismiss the sheet. SecondComponent opens pickers via HomeSheetConfig.Picker and collects results to navigate to Third. UI modules are updated to bind the new sheet state; prior picker handling in Second screens/view models is removed. Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant SecondComponent
participant HomeNavigator as HomeNavigator (stack + sheet)
participant HomeNavHost as HomeNavHostComponent
participant UI as UI Layer (Android/iOS)
participant Picker as PickerComponent (Fruit/Vegetable)
Note over SecondComponent,Picker: Sheet-based picker activation
User->>SecondComponent: Tap "Pick Fruit/Vegetable"
SecondComponent->>SecondComponent: Create ResultFlow<String> in PickerArgs
SecondComponent->>HomeNavigator: openPicker(type, args)
HomeNavigator->>HomeNavHost: sheetNavigator.activate(HomeSheetConfig.Picker)
HomeNavHost-->>UI: sheet StateFlow emits Picker child
UI->>Picker: Present PickerScreen (sheet)
Note over SecondComponent: Collect PickerArgs.results
sequenceDiagram
autonumber
participant Picker as PickerComponent
participant ResultFlow as PickerArgs.results (ResultFlow<String>)
participant HomeNavigator as HomeNavigator.sheetNavigator
participant SecondComponent
participant HomeNavHost as HomeNavHostComponent
participant UI as UI Layer
Note over Picker,SecondComponent: Result propagation and dismissal
Picker->>ResultFlow: sendResult(selectedId)
ResultFlow-->>SecondComponent: emit selectedId
SecondComponent->>SecondComponent: navigateToThird(selectedId)
Picker->>HomeNavigator: dismiss()
HomeNavigator->>HomeNavHost: sheetNavigator.dismiss()
HomeNavHost-->>UI: sheet StateFlow emits null
UI->>UI: Sheet closes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
shared/arkitekt-decompose/src/commonMain/kotlin/app/futured/arkitekt/decompose/navigation/ResultFlow.kt (1)
70-75: Consider configuring MutableSharedFlow replay/buffer for typical ResultFlow usage.The backing
MutableSharedFlow()uses default parameters (replay=0, extraBufferCapacity=0), meaningsendResultwill suspend if there are no active collectors. For navigation result scenarios where a result might be sent before collection starts, this could cause the result to be lost or create unexpected suspension.Consider one of these configurations based on your use case:
Option 1: Buffer one result if sent before collection
-internal class ResultFlowImpl<T>(private val backingFlow: MutableSharedFlow<T> = MutableSharedFlow()) : ResultFlow<T> { +internal class ResultFlowImpl<T>( + private val backingFlow: MutableSharedFlow<T> = MutableSharedFlow( + replay = 0, + extraBufferCapacity = 1, + onBufferOverflow = BufferOverflow.DROP_OLDEST + ) +) : ResultFlow<T> {Option 2: Replay last result to late collectors
-internal class ResultFlowImpl<T>(private val backingFlow: MutableSharedFlow<T> = MutableSharedFlow()) : ResultFlow<T> { +internal class ResultFlowImpl<T>( + private val backingFlow: MutableSharedFlow<T> = MutableSharedFlow(replay = 1) +) : ResultFlow<T> {shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/PickerArgs.kt (1)
6-12: Consider making PickerArgs generic for reusability.The current implementation hardcodes
ResultFlow<String>inPickerArgs. If future pickers need to return different types (e.g., data classes, IDs), you'll need separate Args classes.Consider making it generic:
@Serializable -data class PickerArgs(val results: ResultFlow<String>) +data class PickerArgs<T>(val results: ResultFlow<T>) enum class PickerType { Fruit, Vegetable, }This is optional—the current String-based approach is acceptable if all pickers will return string identifiers.
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/secondScreen/SecondComponent.kt (1)
38-48: Result collection on create is finecollectLatest + navigateToThird keeps flow simple. Consider adding cancellation-safe navigation if multiple results could arrive rapidly, but current approach is acceptable.
androidApp/src/main/kotlin/app/futured/kmptemplate/android/ui/navigation/HomeNavHostUi.kt (1)
78-93: PickerSheet implementation is appropriateSimple pass-through to PickerScreenUi. Zero insets avoid double system padding; keep an eye on device cutouts if you later add interactive elements near edges.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
androidApp/src/main/kotlin/app/futured/kmptemplate/android/ui/navigation/HomeNavHostUi.kt(2 hunks)androidApp/src/main/kotlin/app/futured/kmptemplate/android/ui/screen/PickerScreenUi.kt(3 hunks)androidApp/src/main/kotlin/app/futured/kmptemplate/android/ui/screen/SecondScreenUi.kt(1 hunks)iosApp/iosApp/Views/Navigation/HomeTabNavigationView.swift(2 hunks)iosApp/iosApp/Views/Screen/Picker/PickerViewModel.swift(1 hunks)iosApp/iosApp/Views/Screen/Second/SecondView.swift(0 hunks)iosApp/iosApp/Views/Screen/Second/SecondViewModel.swift(0 hunks)shared/arkitekt-decompose/src/commonMain/kotlin/app/futured/arkitekt/decompose/navigation/ResultFlow.kt(1 hunks)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHost.kt(3 hunks)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHostComponent.kt(2 hunks)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHostNavigation.kt(1 hunks)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/FruitPickerComponent.kt(2 hunks)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/PickerArgs.kt(1 hunks)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/PickerNavigation.kt(1 hunks)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/PickerScreen.kt(1 hunks)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/VegetablePickerComponent.kt(2 hunks)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/secondScreen/SecondComponent.kt(2 hunks)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/secondScreen/SecondScreen.kt(0 hunks)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/secondScreen/SecondScreenNavigation.kt(1 hunks)
💤 Files with no reviewable changes (3)
- iosApp/iosApp/Views/Screen/Second/SecondView.swift
- shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/secondScreen/SecondScreen.kt
- iosApp/iosApp/Views/Screen/Second/SecondViewModel.swift
🧰 Additional context used
📓 Path-based instructions (3)
**/*{Component,Screen,UseCase,ViewState}.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Naming: Use PascalCase for classes and prefer suffixes Component, Screen, UseCase, and ViewState
Files:
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/PickerScreen.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/VegetablePickerComponent.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/FruitPickerComponent.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHostComponent.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/secondScreen/SecondComponent.kt
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Error handling: Use sealed NetworkError classes and model operations as results (Success/Failure)
Max 25 functions per class
Max 20 functions per file
Files:
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/PickerScreen.ktandroidApp/src/main/kotlin/app/futured/kmptemplate/android/ui/screen/SecondScreenUi.ktandroidApp/src/main/kotlin/app/futured/kmptemplate/android/ui/screen/PickerScreenUi.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/secondScreen/SecondScreenNavigation.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/VegetablePickerComponent.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHostNavigation.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/PickerNavigation.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/FruitPickerComponent.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/PickerArgs.ktshared/arkitekt-decompose/src/commonMain/kotlin/app/futured/arkitekt/decompose/navigation/ResultFlow.ktandroidApp/src/main/kotlin/app/futured/kmptemplate/android/ui/navigation/HomeNavHostUi.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHostComponent.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHost.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/secondScreen/SecondComponent.kt
**/*.{kt,kts}
📄 CodeRabbit inference engine (CLAUDE.md)
Max line length: 140 characters
Files:
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/PickerScreen.ktandroidApp/src/main/kotlin/app/futured/kmptemplate/android/ui/screen/SecondScreenUi.ktandroidApp/src/main/kotlin/app/futured/kmptemplate/android/ui/screen/PickerScreenUi.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/secondScreen/SecondScreenNavigation.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/VegetablePickerComponent.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHostNavigation.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/PickerNavigation.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/FruitPickerComponent.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/PickerArgs.ktshared/arkitekt-decompose/src/commonMain/kotlin/app/futured/arkitekt/decompose/navigation/ResultFlow.ktandroidApp/src/main/kotlin/app/futured/kmptemplate/android/ui/navigation/HomeNavHostUi.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHostComponent.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHost.ktshared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/secondScreen/SecondComponent.kt
🧬 Code graph analysis (7)
androidApp/src/main/kotlin/app/futured/kmptemplate/android/ui/screen/SecondScreenUi.kt (1)
androidApp/src/main/kotlin/app/futured/kmptemplate/android/ui/screen/PickerScreenUi.kt (1)
Content(48-103)
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/VegetablePickerComponent.kt (4)
shared/arkitekt-decompose/src/commonMain/kotlin/app/futured/arkitekt/decompose/navigation/NavigationActions.kt (1)
navigation(17-19)shared/arkitekt-cr-usecases/src/commonMain/kotlin/app/futured/arkitekt/crusecases/scope/UseCaseExecutionScope.kt (2)
launchWithHandler(12-48)launchWithHandler(26-41)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHostNavigation.kt (2)
dismiss(52-52)dismiss(54-54)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/PickerNavigation.kt (3)
dismiss(5-8)dismiss(6-6)dismiss(7-7)
iosApp/iosApp/Views/Navigation/HomeTabNavigationView.swift (2)
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHost.kt (8)
stack(17-28)onSheetDismissed(26-26)screen(50-55)screen(52-52)screen(53-53)screen(54-54)screen(57-60)screen(59-59)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHostComponent.kt (1)
onSheetDismissed(103-103)
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/FruitPickerComponent.kt (3)
shared/arkitekt-cr-usecases/src/commonMain/kotlin/app/futured/arkitekt/crusecases/scope/UseCaseExecutionScope.kt (2)
launchWithHandler(12-48)launchWithHandler(26-41)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHostNavigation.kt (2)
dismiss(52-52)dismiss(54-54)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/PickerNavigation.kt (3)
dismiss(5-8)dismiss(6-6)dismiss(7-7)
androidApp/src/main/kotlin/app/futured/kmptemplate/android/ui/navigation/HomeNavHostUi.kt (1)
androidApp/src/main/kotlin/app/futured/kmptemplate/android/ui/screen/PickerScreenUi.kt (1)
PickerScreenUi(37-46)
iosApp/iosApp/Views/Screen/Picker/PickerViewModel.swift (1)
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHost.kt (6)
screen(50-55)screen(52-52)screen(53-53)screen(54-54)screen(57-60)screen(59-59)
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/secondScreen/SecondComponent.kt (4)
shared/arkitekt-decompose/src/commonMain/kotlin/app/futured/arkitekt/decompose/navigation/ResultFlow.kt (1)
ResultFlow(19-19)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHostNavigation.kt (4)
pop(39-40)pop(48-50)openPicker(42-43)navigateToThird(45-46)shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/secondScreen/SecondScreenNavigation.kt (4)
pop(7-11)pop(8-8)openPicker(9-9)navigateToThird(10-10)shared/arkitekt-cr-usecases/src/commonMain/kotlin/app/futured/arkitekt/crusecases/scope/UseCaseExecutionScope.kt (2)
launchWithHandler(12-48)launchWithHandler(26-41)
🔇 Additional comments (33)
androidApp/src/main/kotlin/app/futured/kmptemplate/android/ui/screen/SecondScreenUi.kt (1)
37-37: LGTM! Simplification aligns with centralized sheet navigation. Picker actions are correctly wired throughHomeSheetConfig.Pickerto the global sheet navigator.shared/arkitekt-decompose/src/commonMain/kotlin/app/futured/arkitekt/decompose/navigation/ResultFlow.kt (1)
91-111: LGTM with caveat on result loss.The serializer approach is sound for navigation state preservation: creating a fresh empty
ResultFlowon deserialization avoids serializing runtime flow state. The documentation clearly states that flow state cannot be persisted.However, be aware that any results in flight during serialization will be lost. This is acceptable for navigation scenarios where the picker is dismissed and results are consumed before state saving occurs.
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/PickerScreen.kt (1)
5-13: LGTM!The rename from
PickertoPickerScreenimproves naming consistency by applying theScreensuffix convention. The interface contract remains unchanged.shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/secondScreen/SecondScreenNavigation.kt (1)
9-9: LGTM!The new
openPicker(type: PickerType, args: PickerArgs)action decouples picker navigation fromSecondComponent, enabling centralized sheet-based picker handling. The signature correctly accepts type and args for result propagation viaResultFlow.iosApp/iosApp/Views/Screen/Picker/PickerViewModel.swift (1)
18-23: LGTM!The type updates from
PickerActionstoPickerScreenActionsandKMP.PickertoPickerScreencorrectly align with the shared Kotlin layer rename. The initialization and action binding remain correct.shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/PickerNavigation.kt (1)
5-8: LGTM!The refactor from a callback-based data class to an interface with component-specific extension functions improves type safety. Each picker component (
FruitPickerComponent,VegetablePickerComponent) now has its own dismiss contract, preventing misuse.iosApp/iosApp/Views/Navigation/HomeTabNavigationView.swift (2)
7-12: LGTM!The sheet state is correctly initialized from
component.sheetand bound to the SwiftUI state system via@StateObjectand@KotlinStateFlow.
30-43: Confirm SlotNavigation.dismiss cleans up sheet slot.
Ensure thatdismiss()resets the active sheet slot (emittingnullviachildSlot), soHomeTabNavigationView’s swipe-down dismiss correctly triggers the Kotlin navigation cleanup.androidApp/src/main/kotlin/app/futured/kmptemplate/android/ui/screen/PickerScreenUi.kt (2)
38-46: LGTM!The function rename from
PickerUitoPickerScreenUiand parameter updates fromPickertoPickerScreenalign with the shared layer rename. State and actions are correctly extracted from thepickerScreenparameter.
117-120: LGTM!The preview correctly implements
PickerScreen.Actionswith the updated type.shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHost.kt (3)
20-21: New sheet API on host looks goodAddition of the sheet StateFlow aligns with slot navigation and keeps API cohesive.
26-27: onSheetDismissed hook is appropriateClear contract for UI-layer dismiss callbacks.
43-48: ResultFlow custom serializer makes PickerArgs safe to serialize
PickerArgs.results uses ResultFlow, which is annotated with a custom serializer that no-ops on serialize and recreates an empty flow on deserialize, so keeping @serializable on HomeSheetConfig is fine.shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/FruitPickerComponent.kt (3)
22-27: Updated surface matches PickerScreen and navigation delegationConstructor args and interface changes are consistent.
50-53: Result-first, then dismiss — correct orderingsendResult before dismiss avoids lost results.
55-55: Simple dismiss handler is fineKeeps UX consistent with sheet behavior.
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHostComponent.kt (2)
35-67: Stack navigation wiring LGTMFactory calls with named params are clear and robust.
99-104: Actions delegate correctlynavigate/pop use stackNavigator; onSheetDismissed delegates to sheetNavigator.dismiss(). Good separation.
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/secondScreen/SecondComponent.kt (1)
30-37: Clean migration to ResultFlow-based picker openingSingle ResultFlow shared across pickers is simple and effective.
Confirm that PickerArgs (with ResultFlow) is not serialized in navigation config; otherwise see my comments on HomeNavHost to avoid crashes.
androidApp/src/main/kotlin/app/futured/kmptemplate/android/ui/navigation/HomeNavHostUi.kt (2)
33-43: Android sheet state wiring looks solidState collection and rememberModalBottomSheetState usage are correct.
66-75: Conditional sheet rendering is correctHandles only Picker child; no-ops otherwise. onDismissRequest correctly calls host action.
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/ui/picker/VegetablePickerComponent.kt (6)
19-26: LGTM! Clean component signature with proper delegation.The class declaration correctly implements
PickerScreenandPickerScreen.Actions, and delegatesPickerNavigationto the injected navigation parameter. The newargs: PickerArgsparameter integrates well with the result-passing architecture.
51-54: No race between sendResult and dismiss in onPick sendResult uses suspend emit on an active MutableSharedFlow, completing before dismiss(); no changes needed.
19-26: LGTM! Clean interface composition.The class signature correctly implements
PickerScreenandPickerScreen.Actions, and delegatesPickerNavigationto enable direct method calls likedismiss(). The naming follows PascalCase with the Component suffix as per coding guidelines.
29-29: LGTM!The actions property correctly delegates to
this, as the component implementsPickerScreen.Actions.
56-56: LGTM!Clean dismissal implementation that correctly delegates to the navigation handler.
51-54: No issue:sendResultwon’t throw
The override ofsendResultsimply callsbackingFlow.emit(item)and does not throw non-cancellation exceptions, sodismiss()will always be executed.shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHostNavigation.kt (6)
22-30: LGTM! Well-designed navigation split.The interface correctly separates stack-based navigation (
stackNavigator) from sheet-based navigation (sheetNavigator), enabling independent control of screen stacks and modal sheets. The multiple interface inheritance aggregates all navigation concerns at the HomeNavHost level.
42-43: LGTM! Centralized picker activation.The
openPickermethod correctly activates the sheet-based picker flow viasheetNavigator.activate, passing both the picker type and arguments toHomeSheetConfig.Picker. This centralizes picker handling at the HomeNavHost level as intended.
22-30: LGTM! Clean dual-navigator architecture.The split between
stackNavigatorfor screen navigation andsheetNavigatorfor modal sheets provides clear separation of concerns and aligns well with the PR's goal of standardizing result passing through modal pickers.
32-34: LGTM!Navigator instantiation is straightforward and type-safe.
36-50: LGTM! Consistent navigator usage.All navigation methods correctly delegate to the appropriate navigator: stack operations use
stackNavigator, and modal sheets usesheetNavigator. The implementation is clean and consistent.
52-54: LGTM!Both picker dismiss implementations correctly use
sheetNavigator.dismiss(), maintaining consistency with the sheet-based architecture.
| @OptIn(ExperimentalForInheritanceCoroutinesApi::class) | ||
| @Serializable(with = ResultFlowSerializer::class) | ||
| interface ResultFlow<T> : Flow<T> { | ||
| /** | ||
| * Sends a result item through this flow. | ||
| * | ||
| * @param item The result item to send to collectors | ||
| */ | ||
| suspend fun sendResult(item: T) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify that experimental API is acceptable for public surface.
The @OptIn(ExperimentalForInheritanceCoroutinesApi::class) annotation exposes an experimental coroutines API in the public ResultFlow interface. This may cause downstream clients to require opt-in or face warnings.
Run the following script to check if this experimental API is used elsewhere and assess the impact:
🏁 Script executed:
#!/bin/bash
# Description: Check usage of ExperimentalForInheritanceCoroutinesApi across the codebase
# Search for other usages of this experimental API
rg -n "ExperimentalForInheritanceCoroutinesApi" --type=kt
# Search for ResultFlow usage to understand public API surface impact
rg -n "ResultFlow" --type=kt -C2Length of output: 53
🏁 Script executed:
#!/bin/bash
# Check for any other usages of ExperimentalForInheritanceCoroutinesApi
rg -n "ExperimentalForInheritanceCoroutinesApi" -C2 .
# Locate all references to ResultFlow in the codebase
rg -n "ResultFlow" -C2 .Length of output: 15449
Move experimental opt-in to internal implementation Remove @OptIn(ExperimentalForInheritanceCoroutinesApi::class) from the public ResultFlow interface and instead annotate only the internal ResultFlowImpl class—this avoids leaking an experimental coroutines API in your public surface.
🤖 Prompt for AI Agents
shared/arkitekt-decompose/src/commonMain/kotlin/app/futured/arkitekt/decompose/navigation/ResultFlow.kt
lines 50-59: The public interface ResultFlow currently carries
@OptIn(ExperimentalForInheritanceCoroutinesApi::class); remove that annotation
from the interface declaration so the experimental opt-in is not exposed
publicly, and instead add @OptIn(ExperimentalForInheritanceCoroutinesApi::class)
to the internal implementation class (ResultFlowImpl) where the experimental
inheritance behavior is used; update imports if needed and ensure the interface
remains annotated only with @Serializable(with = ResultFlowSerializer::class).
| override val sheet: StateFlow<ChildSlot<HomeSheetConfig, HomeSheetChild>> = childSlot( | ||
| source = homeNavigator.sheetNavigator, | ||
| serializer = HomeSheetConfig.serializer(), | ||
| initialConfiguration = { null }, | ||
| key = "HomeSheetSlot", | ||
| handleBackButton = true, | ||
| childFactory = { config, childCtx -> |
There was a problem hiding this comment.
Do not serialize sheet config if it carries runtime args
You pass serializer = HomeSheetConfig.serializer() while config includes PickerArgs (with ResultFlow). This will break state saving.
Fix: Drop the serializer for this slot, treating sheets as ephemeral UI:
- override val sheet: StateFlow<ChildSlot<HomeSheetConfig, HomeSheetChild>> = childSlot(
- source = homeNavigator.sheetNavigator,
- serializer = HomeSheetConfig.serializer(),
+ override val sheet: StateFlow<ChildSlot<HomeSheetConfig, HomeSheetChild>> = childSlot(
+ source = homeNavigator.sheetNavigator,
initialConfiguration = { null },Alternatively, make the config fully serializable (no ResultFlow inside) and reconstruct args at factory time.
🤖 Prompt for AI Agents
In
shared/feature/src/commonMain/kotlin/app/futured/kmptemplate/feature/navigation/home/HomeNavHostComponent.kt
around lines 68 to 74, the sheet slot is using HomeSheetConfig.serializer() even
though the config carries runtime PickerArgs with a ResultFlow
(non-serializable) which will break state saving; fix by removing the serializer
for this slot (treat sheets as ephemeral UI) — i.e., omit or set serializer to
null/leave it out when calling childSlot — or alternatively make HomeSheetConfig
fully serializable by removing ResultFlow from the config and instead
reconstructing any runtime args (ResultFlow) inside the childFactory before
creating the child.
Generated by 🚫 Danger |
| * @param decoder The decoder to use for deserialization | ||
| * @return A new empty ResultFlow instance | ||
| */ | ||
| override fun deserialize(decoder: Decoder): ResultFlow<T> = ResultFlow() |
There was a problem hiding this comment.
tyjo tomhlue asi nerozumim... když uděláš nové flow při deserializaci, tak přece ten kdo poslouchá na tu původní instanci, tak nebude dostávat žádné updaty ne?
There was a problem hiding this comment.
V momente keď sa deserializuje Flow, tak pôvodná instancia toho kto počúva neexistuje pretože bol zabitý proces a v tomto momente sa obnovuje navigačný stack v novom procese. Nový proces = nový Component ktorý začne počúvať na nové Flow ktoré sa tuna vytvorí.
Generated by 🚫 Danger |
Standardize navigation results
This PR aims to standardize how we pass results between navigation destinations
Key Features:
ResultFlowfor passing results back from navigation components.HomeNavHostUi(Android) andHomeTabNavigationView(iOS).SecondScreenby using a global sheet navigator.PickertoPickerScreen.SecondScreenHomeNavHostviaHomeSheetConfigSecondComponentResultFlowfor generic result passingModalBottomSheetduplicated inSecondScreenUiModalBottomSheetcentralized inHomeNavHostUiSummary by CodeRabbit
New Features
Refactor