feat: implement Activity Result API support for result-based navigation #5
Conversation
nibel-annotations/bin/main/nibel/annotations/ImplementationType.kt
Outdated
Show resolved
Hide resolved
| * with [NavigationController.navigateForResult] and when setting the result | ||
| * with [NavigationController.setResultAndNavigateBack]. | ||
| */ | ||
| val resultType: KClass<out Parcelable> |
There was a problem hiding this comment.
not sure if possible, but instead of adding a new UiResultEntry is it possible to introduce the result in the existing UiEntry and default it to NoResult (similar to what NoArgs is doing for args)
There was a problem hiding this comment.
updated! UiEntry and UiExternalEntry now support optional params. Created a new NoResult.
Currently NoResult is in the same package as UiEntry -> nibel-annotations/src/main/kotlin/nibel/annotations
but i see that NoArgs is defined in the nibel-stub module. Not sure if we should move it there
https://github.com/open-turo/nibel/pull/5/files#diff-846611264d4fb3a7a7b1eed3748b216929be35c386c57522863d6534c6cc48feR10
nibel-compiler/src/main/kotlin/nibel/compiler/generator/ComposableResultGenerator.kt
Outdated
Show resolved
Hide resolved
| is ComposableEntry<*> -> navigateTo(entry, composeSpec) | ||
| is FragmentEntry -> navigateTo(entry, fragmentSpec) | ||
| is ResultEntry<*> -> { | ||
| // ResultEntry can also be a ComposableEntry or FragmentEntry |
There was a problem hiding this comment.
Not sure if I'm understanding this... ResultEntry cannot be FragmentEntry or ComposableEntry at the same time.
/**
* [ResultEntry] represents a screen entry that can return a result after navigation.
* This interface extends [Entry] to support Activity Result API integration.
*
* @param R The type of result that this entry returns
*/
interface ResultEntry<R : Any> : Entry {
/**
* The result type class for this entry.
*/
val resultType: Class<R>
}
If I'm missing something and ResultEntry is indeed a FragmentEntry or ComposableEntry then ResultEntry wouldn't be reachable.
Otherwise, I think the nested when will always return error
| * Processor for @UiExternalResultEntry and @UiResultEntry annotations. | ||
| * Handles both regular entry generation and result-specific metadata. | ||
| */ | ||
| class UiResultEntryProcessor( |
There was a problem hiding this comment.
some duplication with UIEntryProcessor
kikin81
force-pushed
the
f/result-feature
branch
3 times, most recently
from
c4df8b3 to
54973bf
Compare
Release notes previewBelow is a preview of the release notes if your PR gets merged. 2.0.0 (2025-10-15)⚠ BREAKING CHANGES
Miscellaneous
Continuous Integration
Documentation
Features
Bug Fixes
Code Refactoring
Styles
|
kikin81
force-pushed
the
f/result-feature
branch
from
54973bf to
7dd5c79
Compare
kikin81
force-pushed
the
f/result-feature
branch
from
38bb368 to
1ea3373
Compare
| import org.gradle.kotlin.dsl.dependencies | ||
| import org.gradle.kotlin.dsl.withType | ||
|
|
||
| class NibelAndroidCommonPlugin : NibelConventionPlugin({ |
There was a problem hiding this comment.
Not sure why this files are duplicated on bin folder. Can we remove them?
| navigateForResult(destinationEntry, callback, fragmentSpec, composeSpec) | ||
| } | ||
|
|
||
| override fun <R : Any> setResultAndNavigateBack(result: R) { |
There was a problem hiding this comment.
Can we make this type save?
|
|
||
| @Composable | ||
| private fun SideEffectHandler(sideEffects: Flow<FirstSideEffect>) { | ||
| private fun SideEffectHandler(sideEffects: Flow<FirstSideEffect>, viewModel: FirstViewModel) { |
There was a problem hiding this comment.
Should we just pass the viewModel callBack instead of the whole VM?
| navigateTo(entry as Entry, fragmentSpec, composeSpec) | ||
| } | ||
|
|
||
| override fun <R : Any> navigateForResult( |
There was a problem hiding this comment.
I'm trying to update sample app FirstScreen -> SecondScreen -> ThirdScreen and add result to SecondScreen and ThirdScreen with input text of each screen as the result. But I'm getting this exception when using navigateForResult to ThirdScreen(destination)
My code, not sure if I'm missing something:
//FeatureBNavigation.kt
@Parcelize
data class ThirdArgs(val inputText: String) : Parcelable
@Parcelize
data class ThirdResult(val inputText: String) : Parcelable
data class ThirdScreenDestination(override val args: ThirdArgs) : DestinationWithArgs<ThirdArgs>
// ThirdScreen.kt
@UiExternalEntry(
type = ImplementationType.Fragment,
destination = ThirdScreenDestination::class,
result = ThirdResult::class,
)
@Composable
fun ThirdScreen(viewModel: ThirdViewModel = hiltViewModel()) {
...
// SecondScreen.kt
is SecondSideEffect.NavigateToThirdScreen -> {
val args = ThirdArgs(it.inputText)
navigateForResult(
destination = ThirdScreenDestination(args),
callback = { result: ThirdResult? ->
if (result == null) return@navigateForResult
viewModel.onInputTextChanged(result.inputText)
},
)
}
053eedf to
4a864d6
Compare
kikin81
force-pushed
the
f/result-feature
branch
from
4a864d6 to
b6a3607
Compare
Breaking changes file
|
1 similar comment
Breaking changes file
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements comprehensive Activity Result API support for result-based navigation within the Nibel library, enabling type-safe screens that can return data to calling screens. The changes maintain backward compatibility while adding result navigation capabilities across both Fragment and Compose implementations.
Key changes:
- Add
ResultEntryinterface andResultCallbacksystem for type-safe result handling - Extend annotation system with
resultparameter for@UiEntryand@UiExternalEntry - Implement compiler enhancements to generate dual-interface entries for result-based screens
- Provide sample implementation demonstrating photo picker result navigation workflow
Reviewed Changes
Copilot reviewed 46 out of 47 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
tests/src/test/kotlin/nibel/tests/codegen/TestArgs.kt |
Add documentation and TestResult data class for compilation tests |
tests/src/test/kotlin/nibel/tests/codegen/ResultEntryCompileTest.kt |
New compilation tests for result-based navigation entries |
tests/src/test/kotlin/nibel/tests/codegen/README.md |
New comprehensive documentation for test structure and purpose |
| Multiple test files | Add detailed documentation for existing compilation test scenarios |
tests/build.gradle.kts |
Update dependencies to support result navigation testing |
| Sample app files | Implement photo picker demo and integrate result navigation into existing screens |
| Runtime files | Add ResultEntry, ResultCallback, NibelResultContract and navigation controller extensions |
| Compiler files | Extend code generation to support result-based entry creation with dual interfaces |
| Annotation files | Add result parameter and NoResult marker class for result type specification |
README.md |
Extensive documentation update with result navigation guide and migration instructions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| * screen with no result. | ||
| */ | ||
| @Suppress("ParcelCreator") | ||
| object NoResult : Parcelable |
There was a problem hiding this comment.
The NoResult object implements Parcelable but lacks the required Parcelable implementation methods. Consider adding proper Parcelable implementation or using a different approach for the marker type.
| object NoResult : Parcelable | |
| object NoResult : Parcelable { | |
| override fun writeToParcel(dest: android.os.Parcel, flags: Int) { | |
| // No state to write | |
| } | |
| override fun describeContents(): Int = 0 | |
| @JvmField | |
| val CREATOR: Parcelable.Creator<NoResult> = object : Parcelable.Creator<NoResult> { | |
| override fun createFromParcel(source: android.os.Parcel): NoResult = NoResult | |
| override fun newArray(size: Int): Array<NoResult?> = arrayOfNulls(size) | |
| } | |
| } |
| * This screen simulates a photo picker that allows users to select a photo | ||
| * and return it to the calling screen. | ||
| */ | ||
| @Suppress("LongMethod") |
There was a problem hiding this comment.
[nitpick] The PhotoPickerScreen function is marked with @Suppress("LongMethod"), indicating it may be too complex. Consider breaking it down into smaller, more focused composable functions for better maintainability.
| @Suppress("LongMethod") |
| // For Nibel, we don't create separate activities but rather handle navigation | ||
| // internally. This method is required by ActivityResultContract but we'll | ||
| // handle the actual navigation in the NavigationController. | ||
| return Intent() |
There was a problem hiding this comment.
The createIntent method returns an empty Intent() and includes a comment about not creating separate activities. This implementation may be confusing - consider documenting why this method is required but unused, or provide a more appropriate implementation.
| return Intent() | |
| * This method is required by [ActivityResultContract] but is not used in Nibel, | |
| * as navigation is handled internally through fragments and compose navigation, | |
| * not by launching separate activities. | |
| * | |
| * @throws UnsupportedOperationException Always thrown to indicate this method should not be used. | |
| */ | |
| override fun createIntent(context: Context, input: ResultEntry<R>): Intent { | |
| throw UnsupportedOperationException( | |
| "NibelResultContract does not support createIntent; navigation is handled internally." | |
| ) |
| */ | ||
| @Suppress("UNCHECKED_CAST") | ||
| fun <R : Any> consume(key: String): ResultCallback<R>? { | ||
| return callbacks.remove(key) as? ResultCallback<R> |
There was a problem hiding this comment.
The consume method uses an unchecked cast with @Suppress("UNCHECKED_CAST"). Consider adding runtime type checking or using a more type-safe approach to ensure the callback type matches the expected result type.
| return callbacks.remove(key) as? ResultCallback<R> | |
| private data class TypedCallback<R : Any>( | |
| val type: KClass<R>, | |
| val callback: ResultCallback<R> | |
| ) | |
| private val callbacks = mutableMapOf<String, TypedCallback<*>>() | |
| /** | |
| * Registers a result callback for a specific key. | |
| */ | |
| fun <R : Any> register(key: String, type: KClass<R>, callback: ResultCallback<R>) { | |
| callbacks[key] = TypedCallback(type, callback) | |
| } | |
| /** | |
| * Retrieves and removes a result callback for a specific key. | |
| * Returns the callback only if the type matches. | |
| */ | |
| fun <R : Any> consume(key: String, type: KClass<R>): ResultCallback<R>? { | |
| val typed = callbacks.remove(key) ?: return null | |
| @Suppress("UNCHECKED_CAST") | |
| return if (typed.type == type) { | |
| typed.callback as ResultCallback<R> | |
| } else { | |
| null | |
| } |
Add comprehensive Activity Result API integration to enable type-safe result navigation patterns alongside existing Nibel navigation functionality. - Add ResultEntry<R> interface for screens that return results - Extend NavigationController with navigateForResult() methods - Implement NibelResultContract for Activity Result API integration - Add ResultCallback system with registry for handling results - Support setResultAndNavigateBack() and cancelResultAndNavigateBack() - Add @UiResultEntry annotation for internal result-returning screens - Add @UiExternalResultEntry annotation for external result-returning screens - Both annotations include resultType parameter for type safety - Create UiResultEntryProcessor for processing result annotations - Implement ComposableResultGenerator for dual-interface entry generation - Add ResultEntryGeneratingVisitor for result-specific code generation - Generate entry classes implementing both ComposableEntry and ResultEntry - Add ComposableResultEntryTemplate for result-aware code generation - Create PhotoPickerScreen demonstrating result navigation - Add PhotoPickerResult as sample return type with photo metadata - Update FirstScreen to use navigateForResult with callback handling - Integrate photo result display in UI with selection timestamp - Type-safe result navigation with compile-time verification - Backward-compatible with existing navigation patterns - Automatic result callback cleanup and lifecycle management - Support for both Fragment and Compose-based implementations None - all additions are backward compatible with existing navigation code. - Full project builds successfully - Generated code implements correct interfaces - Sample app demonstrates end-to-end result navigation flow Closes integration of Android Activity Result API patterns into Nibel navigation library, enabling modern result-based navigation with full type safety. Fixes: [#8](#8) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> # Conflicts: # nibel-annotations/src/main/kotlin/nibel/annotations/UiEntry.kt # nibel-annotations/src/main/kotlin/nibel/annotations/UiExternalEntry.kt # nibel-compiler/src/main/kotlin/nibel/compiler/generator/AbstractEntryGeneratingVisitor.kt # nibel-compiler/src/main/kotlin/nibel/compiler/generator/EntryMetadata.kt # sample/feature-A/src/main/kotlin/com/turo/nibel/sample/featureA/firstscreen/FirstState.kt
external entry result was not implemented previously.
kikin81
force-pushed
the
f/result-feature
branch
from
bd910ec to
d02eec8
Compare
Breaking changes file
|
|
Closing in favor of #38 |
Description
Add comprehensive Activity Result API integration to enable type-safe result navigation
patterns alongside existing Nibel navigation functionality.
Core Runtime Changes
Annotation System
Compiler Enhancements
Sample Implementation
Developer Experience
Breaking Changes
None - all additions are backward compatible with existing navigation code.
Testing
Closes integration of Android Activity Result API patterns into Nibel navigation
library, enabling modern result-based navigation with full type safety.
🤖 Generated with Claude Code
Co-Authored-By: Claude
Changes
Screen_recording_20250801_124459.mp4
🚀 PR created with fotingo