Zhuinden / simple-stack

[ACTIVE] Simple Stack, a backstack library / navigation framework for simpler navigation and state management (for fragments, views, or whatevers).
Apache License 2.0
1.36k stars 76 forks source link

Recipes #60

Closed Lakedaemon closed 6 years ago

Lakedaemon commented 7 years ago

I'm currently migrating a complex app from Mortar+Flow to Mortar+SimpleStack

I have a key that leads to different screens (a search screen, an introscreen, a progress screen that waits when a database is ready to go to the search screen, etc...) doing some kind of redirection.

I was hoping to do it with simplestack but 1) building my data and when it is ready, calling

Navigator.getBackstack(this).setHistory(HistoryBuilder.from(Navigator.getBackstack(this)).build(), StateChange.REPLACE)

to redirect to the search screen doesn't work as (I guess the same short circuit that in flow) prevents going from a screen with a key to the same screen with the same key (even if the asociated view changed)

so, I tried 2) resetting the view with val top = Navigator.getBackstack(this).top<Any>() val newView = with (it) {newView(top)} L.error("state change top=$top newView=$newView") val container = containerViewGroup() ?: return container.removeViewAt(0) container.addView(newView, 0)

it works, but now, when In this screen, I do a Naviagator.getBackstack(this).goTo(OtherScreenKey())

I get an exception, this one : java.lang.IllegalArgumentException: The view [org.lakedaemon.dictionary.flow.JapaneseSearchView{421e1af0 V.E..... ........ 0,0-2560,1550 #7f1000fe app:id/flowSearchView}] contained no key! at com.zhuinden.simplestack.BackstackManager.persistViewToState(BackstackManager.java:244) at com.zhuinden.simplestack.navigator.Navigator.persistViewToState(Navigator.java:288) at com.zhuinden.simplestack.navigator.DefaultStateChanger$NavigatorStatePersistenceStrategy.persistViewToState(DefaultStateChanger.java:75) at com.zhuinden.simplestack.navigator.DefaultStateChanger.performViewChange(DefaultStateChanger.java:556) at com.zhuinden.simplestack.navigator.DefaultStateChanger.performViewChange(DefaultStateChanger.java:541) at com.zhuinden.simplestack.navigator.DefaultStateChanger$2.stateChangeComplete(DefaultStateChanger.java:524) at com.zhuinden.simplestack.navigator.DefaultStateChanger$NoOpStateChanger.handleStateChange(DefaultStateChanger.java:42) at com.zhuinden.simplestack.navigator.DefaultStateChanger.handleStateChange(DefaultStateChanger.java:517) at com.zhuinden.simplestack.BackstackManager$1.handleStateChange(BackstackManager.java:76) at com.zhuinden.simplestack.Backstack.changeState(Backstack.java:306) at com.zhuinden.simplestack.Backstack.beginStateChangeIfPossible(Backstack.java:272) at com.zhuinden.simplestack.Backstack.enqueueStateChange(Backstack.java:254) at com.zhuinden.simplestack.Backstack.goTo(Backstack.java:164) at org.lakedaemon.android.AndroidKt.goToScreenFor(Android.kt:594) at org.lakedaemon.dictionary.UtilsKt.goToScreenFor(utils.kt:25) at org.lakedaemon.dictionary.list.WordViewHolder.onSingleTapConfirmed(WordViewHolder.kt:48) at org.lakedaemon.android.listener.DoubleTapListener.onSingleTapConfirmed(utils.kt:40) at android.view.GestureDetector$GestureHandler.handleMessage(GestureDetector.java:315) at android.os.Handler.dispatchMessage(Handler.java:102) at android.os.Looper.loop(Looper.java:157) at android.app.ActivityThread.main(ActivityThread.java:5350) at java.lang.reflect.Method.invokeNative(Native Method) at java.lang.reflect.Method.invoke(Method.java:515) at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1265) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1081) at dalvik.system.NativeStart.main(Native Method)

What should I do to fix this ?

Lakedaemon commented 7 years ago

I guessed that I created the new view in a bad way, without a contextwrapper holding the key... What is the best way to achieve that ?

Lakedaemon commented 7 years ago

actually, solution 2 looks like it is working once I create the new view with the context of the oldview (instead of the wrapped AppcompatActivity) I wonder why simplestack uses 2 KeycontextWrapper (with the same key) though...

I stumbled on another issue : in a view, I would like to access the fields in the key but it throw an axception when I do something like this : val wordDocId = Navigator.getBackstack(context).top<KeyJapaneseWord>().docId

I guess because the key isn't at the top of the backstack yet. To achive the same thing in flow, I had to use Path.get().docId

What is the correct way to do that in simplestack ?

Lakedaemon commented 7 years ago

Nevermind, I already had the solution (it just took my brain some time to realize) : KeyContextWrapper.getKey<SomeClass>(context)

So far, so good with the migration. At some point, I'll want to go to a screen and to inject some stuff in it's state bundle. Is there some canonic way to do that ? Also, what is the canonic way to test if a screen starts from scratch or if it had state restored ?

Zhuinden commented 7 years ago

Path.get was replaced with KeyContextWrapper.getKey(), but as that is not obvious from an API standpoint that is also accessible from Backstack.getKey(context).

If the view implements Bundleable, then you receive a null state bundle in fromBundle() if that helps.

Normally if you want to provide parameters to a view, it is part of the key.

Sorry about the delay, droidcon totally took all my attention during the day.

Simple Stack shouldn't need multiple key context wrappers to work afaik.

Zhuinden commented 7 years ago

I think the reason why you got the crash initially was because

https://github.com/Zhuinden/simple-stack/blob/master/simple-stack/src/main/java/com/zhuinden/simplestack/navigator/DefaultStateChanger.java#L98

To create a context that wraps the base context with a key parameter, you had to do LayoutInflater.from(stateChange.createContext(activity, key));

Zhuinden commented 7 years ago

Normally the Navigator should do that, but you might have been doing something tricky with your addView method which I didn't really account for.

Possibly just didn't do stateChange.createContext(). It's not really obvious that you have to do that, but there is no better place to put it either. It just creates a new KeyContextWrapper, just like what you ended up doing.


The DefaultStateChanger is a bit tricky in its design because it has a re-configurable default implementation for every single line, but you can replace it with your custom StateChanger implementation. As long as for your view-based app you keep the contract which is:

Then it should work just fine.

A custom implementation can ignore the check for being at the same key.

Zhuinden commented 7 years ago

I have opened https://github.com/Zhuinden/simple-stack/issues/61 to track that I should tell people to use stateChange.createContext() when they create a view they want to persist without their key.

@Lakedaemon any luck?

Lakedaemon commented 7 years ago

@Zhuiden I stripped my app of mortar and flow and it is now partially working with simplestack I managed to fix the few issues that happenend because of my code (and not because of simplestack) I have yet to experiment with going to a view and changing it's state (with the data included in a key, through a previous bookmark)

but so far, I'm happy : the code is much simpler than what I had before. And I also hope to manage view state correctly this time (I wasn't doing it correctly with mortar view presenters)

Lakedaemon commented 7 years ago

Next, I'll look how to make the app fully working the simplestack way

Zhuinden commented 7 years ago

I also hope to manage view state correctly this time (I wasn't doing it correctly with mortar view presenters)

Gah I still have nightmares about that, rotating the screen and navigating away then navigating back used to break things in awkward ways >.>


but so far, I'm happy : the code is much simpler than what I had before. Next, I'll look how to make the app fully working the simplestack way

Well, if you run into any issues, I'm here to help 😄 Hopefully with smaller delay than while I was in berlin

Zhuinden commented 6 years ago

@Lakedaemon so, how's the migration going?

Lakedaemon commented 6 years ago

I have not reimplemented the bookmark feature yet. Appart from that, it looks good. I'm happy to be rid of mortar, flow and aloso gson.

I'll probably try to implement something close to redux, having an app state (the many data containers and the rx observables) in the business/pure kotlin/non-android layer and using custom views to only subsribe/unsubscribe observables and save/restore state view.... while having the possibility to time travel with the ui actions

Lakedaemon commented 6 years ago

It would probably make saving restoring state of views with adapter with scroll to search simpler It will probably require having something close to a backstack in the app layer

Zhuinden commented 6 years ago

@Lakedaemon did it work? I always had some trouble wrapping my head around navigation with MVI. the backstack you think of typically ends up being a merged stream of Observable.

Lakedaemon commented 6 years ago

After evaluating it, I didn't try to migrate my app to a redux-like architechture because

But, my feeling is that UI development for android really really sucks (and is a monstrous waste of time). And, in my future projects, I'll try to implement apps that from the start do navigation in the core/language/business logic part (with list of pojos) and only uses android to display data (dumb custom view, redux like rendering, transitions, saving/restoring view states (but not app state)).

Avoiding android stuff always feel so great....

Zhuinden commented 6 years ago

I'll try to implement apps that from the start do navigation in the core/language/business logic part (with list of pojos)

That wanted to be the original intention of simple-stack and possibly of flow, in its core it's a list of objects that are parcelled out and brought back :smile:

The tricky thing about it is that I generally only use the Backstack to store initial parameters, but maybe there is hidden potential in storing current state in it. Just make sure loading is transient.