isaac-udy / enro Goto Github PK

Problem:
Given an Activity with a container to hold Fragments, imagine a scenario where the following navigation is executed (assuming that all Fragments are accepted by the Activity's container):
Activity executes forward action to Fragment A
Fragment A executes forward action to Fragment B
Fragment B executes forward action to Fragment C
Activity executes forward action to Fragment D

If you press the "back" button while Fragment D is active, Fragment D would close back to the Activity, leaving the container empty, because technically it was the Activity that opened Fragment D, so Fragment D does not re-open Fragment C into the container.

I feel like this behaviour isn't quite right, and I think that closing Fragment D should go back to Fragment C. I am thinking about adjusting the navigation system, so that if a navigation action is executed, that action will be passed to the currently active destination for execution. In the example above, that would mean when the Activity goes forward to Fragment D, the Activity would find the currently active Fragment C, and ask Fragment C's navigation handle to actually execute the instruction.

@mgod I'd be interested in your thoughts as to what should happen here, or if you see any problems with my proposed changes.

Nice work.
is there any option to use fragments add transaction instead of replace?

Consider migrating to another package repository.

https://jfrog.com/blog/into-the-sunset-bintray-jcenter-gocenter-and-chartcenter/

I'm building a flow to let a user configure something over several screens. At the end of the process, if it's successful, I'd like to return the result back to the fragment that launched the first screen of the flow. So, something like this:

Main fragment -> Fragment 1 -> Fragment 2 -> Fragment 3

And when Fragment 3 returns a result, close fragments 1, 2 and 3 and have Main fragment handle the result.

I can do this manually by having Fragments 1 and 2 do something like:

val result = registerForResult<Value> {
  navigationHandle.closeWithResult(it)
}

Which is fine except that due to how the lifecycles interact with results, this means that on completing Fragment 3, you see it disappear followed by a flash of Fragment 2 then Fragment 1 disappearing. Is there a way to do this? It seems like result channel has enough infrastructure to be able to pass the result all the way back with minimal changes, but I can't see a clean way to dismiss all the fragments.

As a work-around, it is straightforward to start the flow in an Activity so you get:

Main fragment -> Flow activity -> Fragment 1 -> Fragment 2 -> Fragment 3

and have Fragment 3 grab the Flow activity navigation handle to return the result, but I'd prefer to avoid this if possible.

I'm happy to spend some time putting a PR together for this if it seems like a reasonable change and there's not a way to do this currently.

When multistack click:

2020-10-18 02:13:17.760 8920-8920/nav.enro.example W/ActivityThread: handleWindowVisibility: no activity for token android.os.BinderProxy@302f367
2020-10-18 02:13:17.769 8920-8920/nav.enro.example D/Enro: Opened: nav.enro.example.core.navigation.MultiStackKey@1abad98
2020-10-18 02:13:17.878 8920-8920/nav.enro.example D/Enro: Active: nav.enro.example.core.navigation.MultiStackKey@1abad98
2020-10-18 02:13:18.054 8920-8920/nav.enro.example D/Enro: Opened: nav.enro.example.multistack.MultiStackItem@3f22721
2020-10-18 02:13:18.064 8920-8920/nav.enro.example D/Enro: Active: nav.enro.example.multistack.MultiStackItem@3f22721
2020-10-18 02:13:18.111 8920-8920/nav.enro.example D/Enro: Active: DashboardKey(userId=Isaac)
2020-10-18 02:13:18.188 8920-8920/nav.enro.example D/Enro: Closed: nav.enro.example.multistack.MultiStackItem@3f22721

Enro currently does not support KSP, and only supports KAPT. Enro should support KSP.

Currently, screen transition animations in Enro are defined as xml resources (anim or animator), even for Composable screens.

Enro needs to support animations which are defined in Compose for Composable screens, and in general provide a more flexible transition animation system.

Background:

Using enro version: 1.6.0
Setup Bottom Navigation with Enro, UI is 100% built using Compose.
The MainActivity's code is similar with sample code here: https://github.com/isaac-udy/Enro/blob/b4ecd50c9acdf01ce2a003038ea4b87e01d720f0/example/src/main/java/dev/enro/example/MultistackCompose.kt

Each tab's content UI is built with Compose, and has its own ViewModel.

The issue

The onCleared() method of each tab's ViewModel will never be invoked, even if hitting back button until MainActivity destroy.

The code:

If one of my tab's UI code is like this:

@Composable
@ExperimentalComposableDestination
@NavigationDestination(DashboardKey::class)
fun DashboardScreen() {
    val viewModel: DashboardViewModel = viewModel()
    val text = viewModel.text.observeAsState()
    Text(text = text.value ?: "")
}

And the ViewModel is injected by Hilt:

@HiltViewModel
class DashboardViewModel @Inject constructor() : ViewModel() {

    private val _text = MutableLiveData<String>().apply {
        value = "This is dashboard Fragment"
    }
    val text: LiveData<String> = _text

    init {
        Log.i("viewmodel", "DashboardViewModel init ${hashCode()}")
    }

    override fun onCleared() {
        super.onCleared()
        Log.i("viewmodel", "DashboardViewModel onClear ${hashCode()}")
    }
}

Some clues

Dive into the code for a while, found the ViewModel clear logic is located here:

But actually the lifecycle events only hitted ON_PAUSE:

Not sure under which condition it will hit ON_DESTROY:

Issue

Currently, using putNavigationHandleForViewModel(NavigationKey) on unit tests that are not using AndroidJUnit4 runner would result in failure. This is due to TestNavigationHandle that's created inside this extension function relies on being aware of the application context itself.

Both of these function calls will throw some kind of NPE:

LifecycleRegistry(this).apply {
       currentState = Lifecycle.State.RESUMED
}

and

ApplicationProvider
    .getApplicationContext<Application>()
    .navigationController

Proposal

Enable passing in a null-able NavigationHandle to putNavigationHandleForViewModel in which a user can pass in any kind of mock-able NavigationHandle. If it's null, then use the current way of creating TestNavigationHandle as a default parameter.
Pros: More flexibility for the usages of this extension method, it's backwards compatible to any existing usages.
Cons: Passing around the same thing multiple times down multiple functions. This might be hard to read later on.

Other alternatives

1. Passing null-able NavigationController and null-able Lifecycle into to putNavigationHandleForViewModel in which a user can pass in any kind of mock. If any of them are null, then the default values would be like what they currently are.
   Pros: TestNavigationHandle is always used in the test context for a specific view model with only specific properties are changeable
   Cons: extension function signature becomes a bit more confusing as it's not self explanatory.
2. Some other ways I haven't thought of. Feel free for anyone to give more suggestions.

Hi, great work!

Met a small issue when setting minifyEnabled true

The Crash

I'm using the multi stack for bottom navigation.
If set minifyEnabled true for release build, the app will crash on launch.

The stack looks like this:

    f.g: null cannot be cast to non-null type nav.enro.core.navigator.FragmentNavigator<*, *>
        at g.a.b.c.s0(:33)
        at g.a.b.c.M(Unknown Source:10)
        at androidx.fragment.app.Fragment.e0(Unknown Source:15)
        at c.k.d.r.U(:33)
        at c.k.d.r.S(:1)
        at c.k.d.r.T(Unknown Source:47)
        at c.k.d.a.m(:2)
        at c.k.d.r.D(:7)
        at c.k.d.r.Z(Unknown Source:84)
        at c.k.d.r.B(:4)
        at c.k.d.r.v(Unknown Source:14)
        at c.k.d.e.onStart(:2)
        at c.b.k.h.onStart(Unknown Source:0)

It happens on File MultistackControllerFragment
at this line:

navigator as FragmentNavigator<*, *>

Because no navigator is found, so it's null.

The Reason

After digging the reason, I found it's because during the setup, reflection is used:

private val NavigationApplication.generatedComponent get(): NavigationComponentBuilderCommand? =
    runCatching {
        Class.forName(this::class.java.name + "Navigation")
            .newInstance() as NavigationComponentBuilderCommand
    }.getOrNull()

The Solution

Problem get resolve after adding:

-keep class * extends nav.enro.core.controller.NavigationComponentBuilderCommand

in app/proguard-rules.pro

I'm not sure if it's the best way?

Maybe we could package some rules in the library or list them at readme?

When navigationHandle() is used inside of a Composable Preview, the NavigationHandle is not bound, and which causes problems rendering the preview. There should be documentation which explains how to use navigationHandle() inside of a Composable preview, or some other way of supporting this IDE functionality.

Hi!

I'd like to use this library in my project but it seems to be incompatible with Hilt. Just by adding the dependency kapt starts to fail with cannot access GeneratedComponentManagerHolder.

I've seen in the modularised-example that there is an EnroHilt plugin but adding it to the navigation controller doesn't change anything for me.

I'm currently using:

• Hilt version: 1.0.0-alpha02
• Enro version: 1.2.4

Probably there is a way for doing this but I don't see it clearly.

I'm testing the multistack component in my project and eveytime I perform openStack a fragment enter animation is performed (except the first time I open a stack, which doesn't trigger any animation).

I'd like to remove that animation or use one of my own.

Enro is currently incompatible with Compose 1.2.0 due to some downstream dependencies changing from Java files to Kotlin files (causing some Enro-generated code to fail to compile).

This should be resolved.

I'm seeing some issues around this line in the library:

I haven't been able to find reproduction steps yet, but I'm wondering if it's related to switching this from commitNow to commit:

It seems like I run into this in cases where I have several navigation instructions happening in a row, so it seems like maybe child keys are starting to execute before the parent key transaction has finished. I'm inclined to do some experimenting with swapping this back to commitNow and see if that stops this crash in production, but I can't figure out why this changed. Was there a bug when using commitNow or is it just avoiding using the slower call?