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 75 forks source link

Consider a way to connect a "child nested stack" to a "parent stack" similarly to `setParentComposition` #239

Closed Zhuinden closed 1 year ago

Zhuinden commented 3 years ago

While multi-stack is not the focus of the library by default, if it were possible to integrate against Compose using actual composable functions instead of externally stored state (see the behavior of OnBackPressedDispatcher for why this is actually potentially impossible without manually hijacking onBackPressed in Activity), then it would be possible to introduce a backstack at any "nesting level". After all, what needs to be propagated is back events and service lookups.

Although can you truly keep alive a child composition's "belongings" (read: scoped services / global services) in a retained scope? Not even Compose has figured this out without the use of Fragments to host ViewModelStores.

Zhuinden commented 3 years ago

Actually if I take my own simple-stack multi-stack sample as inspiration, as long as the children can hook themselves into "a parent" (eventually the global) scope's scoped services it'd get everything it needs 🤔

But that's heavily mutable so a bit shady

Zhuinden commented 3 years ago

Until this is resolved, see https://github.com/Zhuinden/simple-stack/issues/247#issuecomment-910227261 for more info

matejdro commented 1 year ago

What if we just pass all currently resolvable services to GlobalServices of the child backstack, when we create the child?

From what I can see, services are immutable, so this should technically work? Services are not enumerable at the moment, but we could add a method for that.

Only issue I can see is with ScopedServices.Activated, ScopedServices.Registered and Bundleable - those callbacks would be triggered twice (for the child and for the parent). Maybe we could add suppressCallbacks to GlobalServices.Builder.addService method?

Zhuinden commented 1 year ago

@matejdro i don't think that's a good idea. It actually makes more sense for the child to have its own "globals" and own scopes.

I think I overthought this and we could just expose something like ˙backstack.setParentBackstack() and just alter the behavior of lookupService()/canFindService() to also check the parent if it's null in the current. Then there is no duplication of any sort between scopes. It is also a behavioral change in Compose-Integration (which has nested stacks) but that's not 1.0 so we can do it.

matejdro commented 1 year ago

Considering #274, would it also make sense to have backstack.setParentBackstack(backstack, scope)?

Zhuinden commented 1 year ago

@matejdro I actually don't see how that would work 🤔 Although I do admit it raises the question of what happens if the one performing the lookup is not part of the top-most chain. Maybe passing in a scope tag is unavoidable.

matejdro commented 1 year ago

The way I see it working is:

  1. Child performs lookupService/lookupFromScope normally
  2. If service is not found, it starts looking into parent:

a) If setParentBackstack was set with explicit scope, it would search inside parent using parent.lookupFromScope(). b) Otherwise, it would search using parent.lookupService()

Zhuinden commented 1 year ago

I probably should have actually written the code for this like 2 months ago, huh

matejdro commented 1 year ago

This is how we do it at the moment: https://github.com/inovait/kotlinova/blob/611b9f9c2719e01583760b25df82e7d84d97bfc8/navigation/src/main/kotlin/si/inova/kotlinova/navigation/simplestack/ServiceInheritance.kt

When nested backstack is created, registerParentBackstack() is called on its global services. Then we use above linked lookupFromScopeWithParentFallbackto lookup services.

Zhuinden commented 1 year ago

Added in 2.8.0. Please check if it solves your usecase (assuming you can update https://github.com/inovait/kotlinova/blob/611b9f9c2719e01583760b25df82e7d84d97bfc8/navigation/src/main/kotlin/si/inova/kotlinova/navigation/simplestack/ServiceInheritance.kt to use it instead).

One thing I'm uncertain about is that even if you use EXPLICIT for the current stack, it will still pass ALL to the parent stack, as all lookup done to the parent stack is effectively unified in behavior. I hope this won't cause any issues, but I also felt like it might not make sense to pass EXPLICIT to the parent, because you can't really know who's there. We'll see if this ever comes up as a request to change... if it ever does, i'll consider it a patch version update (like x.x.1). https://github.com/Zhuinden/simple-stack/issues/277

matejdro commented 1 year ago

Can confirm, it solves my use case. Thanks!

I'm not sure I understand the EXPLICIT issue though, so sorry, can't offer much insight.

Zhuinden commented 1 year ago

My plan is to add a new overload which is "propagateExplicit=true" which would search with explicit even in all the parents.

Not sure if it's important, but I feel it makes sense to have it.

I'm glad it solves your issue and sorry it took a bit long 😅