Zhuinden/simple-stack-compose-integration

Replace animation with AnimatedContent

Closed this issue · 5 comments

Looking at ComposeStateChanger, its code seems very complicated, mostly due to animations.

Maybe using AnimatedContent composable in place of custom baked animation system would allow for cleaner code?

I can take a stab of that in the PR if you want.

Hm, now that I think about it, it would be a breaking change, because AnimationConfiguration would no longer accept Modifiers. Is this worth it?

The library is 0.9.X because it was highly likely that breaking changes would occur in the future.

Is AnimatedContent no longer experimental? I cannot accept code in the library that relies on experimental features.

ComposeStateChanger's code is unfortunately complicated (it was meant to be just "DefaultComposeStateChanger" and then it had 200+ lines of code in it to create something simple 🤦 ) because it has animation support that does not rely on experimental APIs, and does what navigation-compose couldn't figure out for the past 2 years.

Ah, you are right, it is experimental. But I feel like half of the animation APIs in Compose are experimental (and it does not look like they are going to get either changed or stabilized any time soon), so it's a practical thing to use it.

It's practical thing to use them in apps, but irresponsible to expose it in libraries (yes, I mean that even for accompanist).

So until they're stabilized, a better course of action is to reimplement ComposeStateChanger in such a way that it uses AnimatedContent, but it shouldn't be part of the library.

If there's something that the library should be doing, it's #15, but I haven't figured out how to do it without creating scoped services, but that depends on whether scoped services are provided (and is otherwise optional).

Merging #17 should have done this.