nhaarman / acorn Goto Github PK

State saving is tricky to get right, and if it crashes it will only do so upon state restoration at runtime, apart from writing tests.

When in debug mode however, the app could be executed in 'strict' mode to catch these kinds of errors quickly. For example, on each Scene change the entire Navigator could be saved and restored in some secondary process to ensure the application doesn't crash.

State loss can occur when a Scene transition occurs after the Activity has called onSaveInstanceState. If the Activity isn't restored after the transition occurred, the transition is lost and the application will be restored to the point it was in onSaveInstanceState.

Android's FragmentManager throws an exception if a transaction is executed after the state has been saved (commit) and allows you to ignore state loss (commitAllowingStateLoss). Currently, Acorn ignores state loss.

Related to #4

It should be possible to deep link into an application, for example to navigate directly to a product detail page.

@nhaarman Do you have ideas about how one might use Acorn together with unidirectional/mvi architecture in the same app? Thanks in advance.

Arrow seems an unnecessary dependency on the acorn-ext module

Tested by running ./gradlew test on the root project.

Is there a reason to have the arrow dependency here?

When returning a Navigator instance from a function, it is picked up as not restored:

fun createNavigator() : Navigator {
  ...
}

To make Scene transitions look appealing, the UI layer may need to pass arguments to the transition, such as view coordinates.

Describe the bug

I'm unable to successfully import either the top-level project or one of the sample projects into Android Studio or IDEA. When I do so, the Build output console in the IDE displays an error.

To Reproduce
Steps to reproduce the behavior:

1. git clone the top-level project
2. In either IDE: Import the project. In Android Studio - In the IDE welcome dialog select Open an existing Android Studio project. Select the directory containing Acorn. The IDE loads the project.
3. When the project is done loading the following error is displayed in the Build output console:

Unable to load class 'org.gradle.execution.taskgraph.TaskInfo'.
Possible causes for this unexpected error include:

• Gradle's dependency cache may be corrupt (this sometimes occurs after a network connection timeout.)
  Re-download dependencies and sync project (requires network)
• The state of a Gradle build process (daemon) may be corrupt. Stopping all Gradle daemons may solve this problem.
  Stop Gradle build processes (requires restart)
• Your project may be using a third-party plugin which is not compatible with the other plugins in the project or the version of Gradle requested by the project.

In the case of corrupt Gradle processes, you can also try closing the IDE and then killing all Java processes.

Expected behavior
I should be able to import the project and perform a build successfully.

Stack trace
Here's an abbreviated version of the stack trace:

Environmental info (please complete the following information):

• Intellij IDEA 2018.3.4, Android Studio 3.3
• OS: Mac Mojave 10.14.2

Describe the bug
A Scene with ActivityController has a strange lifecycle. When the activity launched with the ActivityController.createIntent(): Intent is closed, the scene lifecycle is attached -> destroy -> detach. The detach event happens between start & attach of the previous scene. I think it's right because I close the scene in a callback after onResult() and from DefaultActivityHandler I get that the order of events is attach -> onResult -> detach

v("ActivityHandler", "Attaching container to $scene.")
scene.forceAttach(activityController)

v("ActivityHandler", "Notifying ActivityController of result.") 
activityController.onResult(resultCode, data)

v("ActivityHandler", "Detaching container from $scene.")
scene.forceDetach(activityController)

The bug happens when the screen rotates and the activity is closed in a different orientation. In that case no scene lifecycle event will be invoked, nor ActivityController.onResult

To Reproduce

class ExportDatabaseController(
    private val context: Context
) : ActivityController, ExportDatabaseContainer {

    override var onExportResult: (() -> Unit)? = null

    override fun createIntent(): Intent {
        return Intent(Intent.ACTION_OPEN_DOCUMENT_TREE).apply {
            flags = Intent.FLAG_GRANT_READ_URI_PERMISSION or
                Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION
        }
    }

    override fun onResult(resultCode: Int, data: Intent?) {
        onExportResult?.invoke()
    }
}

interface ExportDatabaseContainer: Container {
    var onExportResult: (() -> Unit)?
}

class ExportDatabaseScene(
    private val listener: CloseSceneEvent
) : Scene<ExportDatabaseContainer> {

    override fun attach(v: ExportDatabaseContainer) {
        super.attach(v)
        Timber.d("attach")
        v.onExportResult = {
            Timber.d("callback")
            listener.closeCurrentScene()
        }
    }

    override fun detach(v: ExportDatabaseContainer) {
        super.detach(v)
        Timber.d("detach")
        v.onExportResult = null
    }

    override fun onDestroy() {
        super.onDestroy()
        Timber.d("destroy activity")
    }
}

Environmental info:

• Library version: 1.2.0
• Device: Oneplus 6T
• OS: Android 10

I'm having some trouble to understand how the back press is handled by navigators

class Navigator : CompositeStackNavigator {
    // I don't know why this function will always return true but the activity will be closed anyway
    override fun onBackPressed(): Boolean {
          super.onBackPressed()
          return true
    }
}

I also tried to register an OnBackPressedCallback on backPressDispatcher but it doesn't work.

How could I achieve a back twice to close behavior?

Scenes can become huge, causing monoliths. It should be possible to divide logic in these Scenes.
This could be done with Presenters which have the same lifecycle as the Scene.

Is this possible? How would we tell Dagger what to inject for the Scene's savedState constructor parameter? For the time being I have switched to using Koin since with that I can locate the Scene's dependencies, and pass those and the savedState into the Scene's constructor manually.

To be clear, what I'm looking to be able to do is annotate my Scene with @Inject, and then either retrieve it through appComponent.getSomeSpecificScene() or let Dagger inject it into other classes. In both scenarios, the Scene is constructed entirely by Dagger.

How would this work?

Starting third party apps to retrieve a result is a powerful thing the Android framework brings. An app might for example start the camera app to take a picture, or the Contacts app to select a contact.

This is done using Intent and Activity#startActivityForResult. When the third party application returns, Activity#onActivityResult is called.

Bravo must be able to support this.

When using a BindingSceneTransitionFactory in a ComposingSceneTransitionFactory, it blocks secondary factories from being called: the BindingSceneTransitionFactory falls back to a default if there is no binding.

Instead, it should return false for the supports() call.

The Material "Bottom Navigation" component allows movement between primary destinations in an app:

Each of the 'primary destinations' is represented by a tab in the bottom navigation bar, and each of them has their own navigational state.

In Acorn, this could be represented by a CompositeParallelNavigator type, which allows for such a component.

A quick sketch-up of the API could be:

enum class MyDestination {
  Favorites,
  Music,
  Places,
  News
}

class MyNavigator(
  savedState: NavigatorState? = null
) : CompositeParallelNavigator<MyDestination>(savedState) {

  override fun createNavigator(destination: MyDestination) : Navigator {
    return when(destination) {
      Favorites -> FavoritesNavigator()
      /* ... */
    }
  }

  override fun instantiateNavigator(
    navigatorClass: KClass<out Navigator>,
    state: NavigatorState?
  ) : Navigator {
    return when(navigatorClass) {
      is FavoritesNavigator::class -> FavoritesNavigator(state)
      /* ... */
    }
  }
}

The CompositeParallelNavigator<T> class could expose methods like select(T) to switch between the nested child Navigators, and provide a way to listen to changes in the selected T type.

Is it possible to have multiplatform support?

I imagine an architecture where Acorn can be used as a navigation in a multiplatform project, and specific implementations can be done on specific platforms.

Currently, all default animations animate a new screen popping on top of the previous one, which may lead to confusing context for end users. Instead, when going back to a previous screen, an appropriate animation should be shown.

Unfortunately, Navigators may have no notion of 'back', and they probably shouldn't even know about going 'back' or 'forward'.
Handling this in the UI may lead to hacky solutions to determine whether a Scene change is back or forward.

I am trying to use this lib in a current project. When building I receive the error below. This happens during the kaptDebugKotlin task

e: error: compiler message file broken: key=compiler.err.Processor: org.jetbrains.kotlin.kapt3.base.ProcessorWrapper@28db3c9f arguments={0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}
e: error: cannot access AcornAppCompatActivity
  class file for com.nhaarman.acorn.android.AcornAppCompatActivity not found
  Consult the following stack trace for details.
  com.sun.tools.javac.code.Symbol$CompletionFailure: class file for com.nhaarman.acorn.android.AcornAppCompatActivity not found

Using a state machine in a Navigator seemed one of the main advantages of using these Navigators when beginning to design the library. In practice actually, using state machines may prove to be more difficult than initially thought -- there's Scene management, View hierarchy saving, Scene state saving, etc. Combining these with a state machine is not that trivial at all.

Especially with the power that comes with the composing of Navigators, state machines become less necessary. There are definitely cases where state machines may be a better choice, but before that happens some more thought in how to actually implement Navigators with them is necessary.

Instead of creating a navigator in the Activity, some mechanism should be created to:

• Cache instances in memory
• Restore from saved instance state
• Ignore saved instance state if > 30 minutes (configurable?)

I need to show the soft keyboard if the view list is empty. Anyway the apperance of the soft keyboard resizes the activity's view (default behaviour), so I delayed that action until the scene transition ends with view.postOnAnimationDelayed(HARDCODED_TRANSITION_DURATION) { view.showKeyboard() } but I want to get rid of dependence on the exact value of duration

What do you think about that workaround?

interface OnSceneTransitionListener {
    fun onSceneTransitionEnd()
}

class MyViewController : ViewController,  OnSceneTransitionListener {
    override fun onSceneTransitionEnd() {
        // show the soft keyboard
    }
}

// transition
val newController = viewControllerFor(parent)
callback.attach(newController)
fakeAnimationMethod()
    .doOnEnd {
        callback.complete(newController)
        (newController as? OnSceneTransitionListener)?.onSceneTransitionEnd()
    }

Heads up that the link in the documentation to Transition animations is returning a 404.

Methods like onStart and onStop need an @CallSuper annotation

Activity theme not applied into child views
Child views does not inherit activity theme might be because setContentView is not called.

To Reproduce
I have created all views programmatically and does not depend on XML.
Issues found so far:

1. Child views did not inherit colors from theme
2. selectableItemBackground does not resolve ripple animation since it does not get the right theme.

Expected behavior
Activity theme should be applied into child views

Environmental info (please complete the following information):

• Library version: 1.0.0
• Device: Pixel 3 x86 emulator
• OS: Android 10

The library includes a CompositeDisposable.plusAssign(DisposableHandle) function, which is used to easily add DisposableHandles to a CompositeDisposable. However, this is rarely used but really gets in the way when using RxJava's plusAssign in the sense that the auto-import may suggest the wrong import.

This method should ideally be removed altogether, possibly introducing an alternative if necessary.

We can easily do an instanceof check and do nothing when the Container is not a RestorableContainer. This allows users to be able to use these classes without needing to use the RestorableContainer interface.

This applies to at least:

• BaseSavableScene
• BasicScene
• RxScene

See #91 (comment):

This could use a lint check: createViewController returns a ViewController instance, but a compile time check for V is not possible, see https://stackoverflow.com/questions/43790137/why-cant-type-parameter-in-kotlin-have-any-other-bounds-if-its-bounded-by-anot

The lint check could check Scenes implementing ViewControllerFactory and check whether the return type of createViewController also adheres to the Scene's Container type.

Also, a Lint check checking whether ViewProvidingScene is only used with Scenes can be created (#91 (comment)).

It's possible that while an app is running, the same app gets started with some intent, for example to serve as a picture picker. Ideally, these are two separate 'tasks' with their own navigators, and doing this doesn't destroy the first navigator state.

Calling StackNavigator.onStart multiple times causes listeners to receive the Scene notification again.

The com.nhaarman.acorn.android.transition.Transition interface clashes with Android's android.transition.Transition. Since the latter is very useful when defining custom transitions, this name clash can be annoying.

Perhaps rename it to SceneTransition?

The Kotlin Android Extensions plugin is considered controversial, and shipping functionality may be unwanted.

Often a tablet layout is two ui panes in one screen.
Android provides resource folders for this, how would this work with navigators?

For example passing a SceneState instance to an RxScene constructor that does not also implement SavableScene should give some warning

When no ViewController could be created, this is the error currently thrown:

11-28 11:45:48.064 3894-3894/com.nhaarman.circleci E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.nhaarman.circleci, PID: 3894
    java.lang.IllegalStateException: Could not dispatch com.nhaarman.circleci.build.BuildScene@1b2a827.
        at com.nhaarman.acorn.android.dispatching.AcornSceneDispatcher$MyListener.scene(AcornSceneDispatcher.kt:103)
        at com.nhaarman.acorn.navigation.StackNavigator$State$Active$push$1.invoke(StackNavigator.kt:366)
        at com.nhaarman.acorn.navigation.StackNavigator$State$Active$push$1.invoke(StackNavigator.kt:328)
        at com.nhaarman.acorn.navigation.StackNavigator.execute(StackNavigator.kt:216)
        at com.nhaarman.acorn.navigation.StackNavigator.push(StackNavigator.kt:127)
        at com.nhaarman.circleci.CircleCINavigator$DashboardListener.onBuildClicked(CircleCINavigator.kt:49)
        at com.nhaarman.circleci.dashboard.DashboardScene$onStart$5.accept(DashboardScene.kt:53)
        at com.nhaarman.circleci.dashboard.DashboardScene$onStart$5.accept(DashboardScene.kt:28)
        at io.reactivex.internal.observers.LambdaObserver.onNext(LambdaObserver.java:63)
        at io.reactivex.internal.operators.observable.ObservableSwitchMap$SwitchMapObserver.drain(ObservableSwitchMap.java:297)
        at io.reactivex.internal.operators.observable.ObservableSwitchMap$SwitchMapInnerObserver.onNext(ObservableSwitchMap.java:374)
        at io.reactivex.internal.operators.observable.ObservableHide$HideDisposable.onNext(ObservableHide.java:67)
        at io.reactivex.subjects.PublishSubject$PublishDisposable.onNext(PublishSubject.java:308)
        at io.reactivex.subjects.PublishSubject.onNext(PublishSubject.java:228)
        at com.nhaarman.circleci.dashboard.widget.RecentBuildsRecyclerView$RecentBuildViewHolder$1.onClick(RecentBuildsRecyclerView.kt:110)
        at android.view.View.performClick(View.java:5198)
        at android.view.View$PerformClick.run(View.java:21147)
        at android.os.Handler.handleCallback(Handler.java:739)
        at android.os.Handler.dispatchMessage(Handler.java:95)
        at android.os.Looper.loop(Looper.java:148)
        at android.app.ActivityThread.main(ActivityThread.java:5417)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:726)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:616)

Is your feature request related to a problem? Please describe.
I am trying to make use of Acorn for Navigation in a fully Jetpack Compose screens. This is hard because AcornActivities are coupled with the ViewGroup with the ViewControllerFactory.

Describe the solution you'd like
It would be nice to have the whole ViewController abstracted to just a View and not attached to Android's ViewGroup.

Describe alternatives you've considered
I did a naive implementation of this but haven't figured out the right abstractions yet. Especially with transitions.
https://github.com/gumil/talan/tree/master/app/src/main/java/dev/gumil/talan/acorn

Currently, the ViewFactory allows the user to provide the View and the Container separately, so there is a choice whether or not to let a View subclass implement the Container interface, or to wrap the View in a class that implements the Container interface.

The result is a very verbose and clunky API when creating ViewResults that use the wrapper method:

val view : View = ...
ViewResult.from(view, MyViewWrapper(view))

When letting the root View implement the Container interface, the compiler doesn't assist in checking whether the View actually implements a Container interface, deferring the check to runtime.

Furthermore, using the View subclassing way also impedes with custom scene transitions: when using a custom View root, manipulating a ViewGroup to adhere to the destination Container becomes impossible as well.

Proposed is to replace the ViewResult by a ViewController interface that always contains a reference to the root view. This ViewController then implements the specialized Container interface and 'controls' the Android View:

interface ViewController : Container {

  val view: View
}

interface MyContainer : Container {
  var name: String?
}

class MyViewController(override val view: View) : ViewController, MyContainer {

  override var name: String? = null
    set(value) {
      view.findViewById<TextView>(R.id.textView).text = value
    }
}

This greatly simplifies the ViewFactory and Transition API, and results in a uniform structure in the app: ViewController becomes a mandatory component when working with the default ext-bravo-android artifact.

Navigators should be able to have results, such as selecting a picture or logging in.

A single ViewFactory declaration can become enormous. It shouldn't be that hard to make these composable.

Cannot resolve reference com.nhaarman.acorn.presentation.Container class when using in pure java module.

Steps to reproduce:

1. Create empty activity project using Android Studio wizard
2. Create module Java Library
3. Add implementation 'com.nhaarman.acorn.ext:acorn:1.2.0' to the java module in build.gradle file
4. Create any interface that extends com.nhaarman.acorn.presentation.Container will see the error.

Found a similar issue on StackOverflow. Maybe it is gradle issue then.

The library requires a minSdk of 21. How safe is it to ignore that? Are there functions we can avoid once ignoring the minSdk requirement that will make it safe to use still? Thanks.

For some Scenes and Navigators it may not make sense to restore for, and it may be easier/more fitting to just not save them.

The existing navigators in the extension artifacts currently always save their internal state, and if possible (if the child supports state saving) save the childrens' state with it.

For the StackNavigator for example, it may not make sense to preserve the entire stack but only up to a certain point. Say Scene C does not want to preserve its state in a stack [A, B, C, D] one could argue that only [A, B] should be restored.

Currently, the documentation talks about 'active' and 'inactive' Scenes and Navigators with respect to the onStart and onStop methods.

However, there is also the notion of an 'active Scene` within a Navigator, which may cause confusion: A Scene may be 'active' in a Navigator, but be in the stopped state.

In Scenes section

‘started’ : The Scene is dormant, waiting to be started or to be destroyed.
‘stopped’ : The Scene is started.

In Navigators section:

During the lifetime of a Navigator it can go from ‘inactive’ to ‘inactive’ and vice versa multiple times, until it reaches the ‘destroyed’ state.

In Usage section:

Acorn is tactically divided in several modules to be able to separate different concerns from eachother.

It may be possible to show a Scene while the previous scene is still visible, such as with dialogs.
How would this be handled?

Is it possible to push a scene on top of another without removing other views from the parent?

I use a custom horizontal draggable view and while sliding users must see the view below

Is your feature request related to a problem? Please describe.
For example:
Scene1 pushes to Scene1
They are the same scenes but can have different data. So during transition there's no way to identify if the scene is going back one screen since it is still the same scene class.

An example of a transition that affects this is sliding from left to right. When going back it should be from right to left.

Describe the solution you'd like
Have a way in transition factory to know if the transition happening is going back or forward.

Describe alternatives you've considered
Is it also possible for a more granular transition mechanism where it can be specified on specific navigators?

This is an edge case it seems as I am trying this out in a sample application with unrealistic backstack and screens.