Zoltan Demant

    Zoltan Demant

    1 year ago
    Has anyone had success when using
    SaveableStateHolder.SaveableStateProvider()
    and
    AnimatedContent
    together? Whenever I try, the ui-state just seems to be lost after navigation; e.g. I can navigate between screens A/B/C while they keep their individual scroll-positions, but as soon as animated content enters the picture the scroll-positions are reset on each navigation. Id love to understand why this happens, if theres a way to work with it, or if I should just use
    Animatable
    and do the work myself?
    Doris Liu

    Doris Liu

    1 year ago
    Could you share your code?
    Zoltan Demant

    Zoltan Demant

    1 year ago
    Sure! Ive also attempted to use it the other way around, where AnimatedContent comes first and its content calls SaveableStateProvider instead - but it always results in duplicate key issues (even though the keys are unique):
    java.lang.IllegalArgumentException: Key X was used multiple times
    Dominaezzz

    Dominaezzz

    1 year ago
    Duplicate key!! I've seen that one before! It basically means you're not respecting the key that AnimatedContent has given to you.
    You've omitted the offending code so I can't tell you exactly where you when wrong but just make sure you're not using
    key
    inside
    content
    of
    AnimatedContent
    . You should be using
    render
    in there.
    Zoltan Demant

    Zoltan Demant

    1 year ago
    Thank you, your answer pointed me in the right direction! The input to my composable changes after a while as data is delivered, resulting in another call to
    AnimatedContent
    , since the
    SaveableStateHolder
    is remembered outside of the
    AnimatedContent
    scope the same key is technically used twice - the
    content
    parameter has different data between the render passes, but uses the same key (its the same screen). I dont really know what to do with this information though, if I were to remember the holder inside the animated-content call, it would only last for that animation scope which renders it useless, similarly if I were to use different keys for the different versions of the screen, the saved state wouldnt be applicable. Any ideas? 🙂
    Dominaezzz

    Dominaezzz

    1 year ago
    AnimatedContent
    (haven't used it but I imagine it's similar to Crossfade which I've used) works by rendering the content twice. One for the previous state and one for the new state. It does this so it can animate between them. To compose the content in the two different states, it has to pass which state to render to
    content
    which is what the
    render
    param is.
    similarly if I were to use different keys for the different versions of the screen, the saved state wouldnt be applicable
    What does this mean?
    Why isn't the saved state applicable?
    You should remember the holder outside
    AnimatedContent
    and then call
    SaveableStateProvider
    inside
    AnimatedContent
    .
    Zoltan Demant

    Zoltan Demant

    1 year ago
    Why isn't the saved state applicable?
    I just meant that, if I were to create different keys for the screen based on its state, the keys would also be worthless.
    You should remember the holder outside 
    AnimatedContent
     and then call 
    SaveableStateProvider
     inside 
    AnimatedContent
     .
    Good, thats what Im doing but I wasnt 100% certain that it was the way to go prior due to the crashes: good to know 🙂 Ill share some more details on my use case, hopefully it makes sense and can provide some more insights! The composable Im working with accepts a
    List<T>
    , where each entry has a unique key which is used in the
    SaveableStateHolder
    in order to preserve the screen state. Its like a backstack where previous screens are part of the list and the last/top one is what actually gets rendered (plus animations between previous/current). Its pretty similar to this, but obviously uses
    AnimatedContent
    instead of the lower level API😒, I do wonder if the same scenario would happen if they were to use animated-content? I think the problem lies in the fact that if the list contains Screen A, and a new list is later on provided with a different version of Screen A (with different data); the key is identical between the two, but animated-content still runs due to the change in data. This is pretty much where Im at now in my thinking at least. I want the screen to have the same key so that its state is retained, but simultaneously I want to (potentially) animate its content as well.
    Dominaezzz

    Dominaezzz

    1 year ago
    You'll have to calculate the right key to do what you want, yeah this sounds pesky. Don't think I could help much without seeing code but you're definitely on the right track.
    Zoltan Demant

    Zoltan Demant

    1 year ago
    More code it is! 🙂 The render function is called twice (crashes the second time). BackStackScreen contents are as follows:1. ExploreScreen(page, loading) 2. ExploreScreen(page, data) (page is unchanged between render calls)
    @JvmInline
    value class BackStackScreen(
        val stack: List<Any>
    ) {
    
        init {
            require(stack.isNotEmpty()) {
                "BackStack cannot be empty!"
            }
        }
    
        inline val top
            get() = stack.last()
    }
    @Composable
        override fun Render(render: BackStackScreen) {
            val holder = rememberSaveableStateHolder()
    
            AnimatedContent(
                targetState = <http://render.top|render.top>,
                content = { top ->
                    val key = KeyedScreen.findKey(top) // top.javaClass.simpleName
    
                    LogHandler.log { "Render $key" } // Prints ExploreScreen
    
                    holder.SaveableStateProvider(key) {
                        // Content really doesnt matter, it crashes regardless
                        Text(
                            text = key
                        )
                    }
                }
            )
        }
    Dominaezzz

    Dominaezzz

    1 year ago
    Interesting, that shouldn't crash assuming
    findKey
    is doing the right thing and returning unique keys for each top.
    Zoltan Demant

    Zoltan Demant

    1 year ago
    The top is always
    ExploreScreen
    while Im testing this, and the key is always "ExploreScreen" as well, as it should be. It seems to be an issue with the animation specifically, e.g. placing the
    SaveableStateProvider
    call outside
    AnimatedContent
    works even if it gets called twice as well.
    Dominaezzz

    Dominaezzz

    1 year ago
    Oh, I think you should pass key to animatedcontent
    Zoltan Demant

    Zoltan Demant

    1 year ago
    Oh no. I'll have to try that, it does make sense. I'll let you know how it goes when I'm back in my office! 🌟
    Doris Liu

    Doris Liu

    1 year ago
    The crash indicates that content lambda has been invoked multiple times with the same key. The only reason that I can think of for that to happen is if AnimatedContent considers the
    targetState
    to be different, even though the keys/class names are the same. Seems like that's likely the case since both ExploreScreen(page, loading) and ExploreScreen(page, data) would have the same key but are probably different instances. I'm assuming you would want to treat the loading and data screens as different contents and animate the content swap. If that's the case, I would recommend creating different keys for each of the screen for SaveableHolderProvider. If the loading/data screens are intended to be the same screen (i.e. no content transform from AnimatedContent), then I'd recommend using the key as targetState for AnimatedContent.
    Zoltan Demant

    Zoltan Demant

    1 year ago
    Thank you both. Using the key as target state solves the duplicate key crash, and it makes perfect sense. The state is no longer restored between the screens though, I'm not sure why? Code is identical to above, but uses the render.top value to obtain and pass in its key as target state, and then renders using the same 'render.top' value inside the animated content block. State is restored properly if I ditch animated content. Any ideas?
    Doris Liu

    Doris Liu

    1 year ago
    Could you file a bug including a sample of your latest code? I can't think of any reason why the states don't restore, but I'm happy to investigate. 🙂
    Zoltan Demant

    Zoltan Demant

    1 year ago
    Its not perfect yet, but I think Ive got it working. The issue was really a combination of failing to use the key as
    targetState
    , and then rendering "static" content inside the
    content
    call instead of basing it on the
    targetState
    (key). I omitted my rendering code in the samples thinking that it didnt have any impact, it was crashing regardless at the time after all. If my understanding is correct, heres what went down:1. By supplying the
    <http://render.top|render.top>
    as the targetState, the
    AnimatedContent
    would animate between them as it should, in practice this meant an animation from Foo -> Foo due to it containing different data, and the resutling duplicate key error due to the keys being identical. 2. In the
    content
    call I would use
    <http://render.top|render.top>
    since thats the resulting screen, this rendered fine after (1) was resolved, but the animation was not 100%, and the state was not restored. I believe this was happening due to the wrong content being rendered, hence if the key was meant for Foo, which declared some state, but Bar was actually rendered, declaring its own state; none of them would be saved since each would be forgotten between the render passes. The its not perfect yet part: Since the incoming backstack is already in its final state when the
    @Composable
    is invoked with it, it no longer contains all entries required for the animation when navigating backwards (the animation always requires the last two entries to navigate forward/backward between). There are many ways to solve this, do you have any recommendation? 🙂 This was a harsh but good learning experience! Im very grateful for all your help! ❤️
    Since the incoming backstack is already in its final state when the 
    @Composable
     is invoked with it, it no longer contains all entries required for the animation when navigating backwards (the animation always requires the last two entries to navigate forward/backward between). There are many ways to solve this, do you have any recommendation? 🙂
    Adding to this, Ive often found myself having to keep previous renderings around in order for backwards animations to be able to happen. Is there a better way to approach this?
    AnimatedContent
    will still call
    content
    with the previous-renderings key, and I dont see a fitting way to keep that around (there are several instances like this, BackStackScreen just being one of them). • I can keep all renderings around, but that quickly adds up to a lot.
    SideEffect
    could be used to clear the old ones, but it runs after the composition completes, and the animation continues for a while thereafter, still requiring the previous value. • I can clear renderings using a
    DisposableEffect
    , this works for some cases, however the BackStackScreen in my example above is the top-level screen and lives for as long as the app does. • I can wrap key and content in a class, override the equals method so that it only takes the key into account - this partially works, but breaks when the same screen should be re-rendered with new data.
    Doris Liu

    Doris Liu

    1 year ago
    Thanks for your use case. I'm currently investigating a way to support some form of custom equality for targetState. For now I'd recommend something like this as a work around:
    @Composable
        override fun Render(render: BackStackScreen) {
            val holder = rememberSaveableStateHolder()
            val screenKey = KeyedScreen.findKey(<http://render.top|render.top>)
            val targetScreen = key(screenKey) rememberUpdatedState(<http://render.top|render.top>)
    
            AnimatedContent(
                targetState = targetScreen,
                content = { top ->
                    // top.value would be the BackStackScreen that AnimatedContent will
                    // remember for you. So you don't have to keep the rendering around explicitly
                    val key = KeyedScreen.findKey(top.value) // top.javaClass.simpleName
                    LogHandler.log { "Render $key" } // Prints ExploreScreen
    
                    holder.SaveableStateProvider(key) {
                        // Content really doesnt matter, it crashes regardless
                        Text(
                            text = key
                        )
                    }
                }
            )
        }
    This recreates a new
    State<BackStackScreen>
    instance as the targetState when the screen changes, and update the BackStackScreen when the key stays the same. Since the
    BackStackScreen
    would be kept around in the AnimatedContent, you don't have to track them yourself. And instead, you could just render the content based on
    target.value
    where
    target
    is the state passed to the content lambda
    Zoltan Demant

    Zoltan Demant

    1 year ago
    Thanks, that makes sense and seems better/simpler than my previous implementation! Im seeing some strange behavior where the state isnt restored the first time around - I think Ive nailed it down to my loading indicator; e.g. if I navigate from A to B, and B shows a
    CircularProgressIndicator
    and then its contents, A will have lost its state by the time I navigate back. If I navigate to B again, the data is ready and no loading indicator is shown, hence navigating back will properly restore the state of A. I cant say that I understand why this is happening, but my best bet is that B leaves the composition during the loading phase and it somehow affecting the saved state of A? Ill leave my code down below, hopefully it can provide some insight! Ive also attempted this without the animations without any luck. My screen hierarchy pretty much consists of a BackStackScreen [A, B] where both A & B go through the same loading then content procedure using the code below. BackStackScreen which holds the state of A and B never leaves the composition though.
    @Composable
    override fun Render(render: PendingScreen) {
        AnimatedContent(
            targetState = render.content != null,
            transitionSpec = {
                val animationSpec = tween<Float>(
                    durationMillis = 300
                )
    
                ContentTransform(
                    targetContentEnter = fadeIn(
                        animationSpec = animationSpec
                    ),
                    initialContentExit = fadeOut(
                        animationSpec = animationSpec
                    )
                )
            },
            content = { available ->
                if (available) {
                    val content = checkNotNull(render.content)
                    Content(content)
                } else {
                    Loader(
                        extended = true
                    )
                }
            }
        )
    }
    After experimenting more with this, I realized that using your suggested solution never actually restored the state for me. Ill link my implementation down below; this works for restoring state, but is broken in the aforementioned scenario (loading, then content).
    @Composable
    override fun Render(render: BackStackScreen) {
        //This is just a wrapper around SaveableStateHolder which also keeps track of the keys & clears out old ones in the SideEffect down below.
        //In this scenario, Screen B is cleared on back-navigation by way of the SideEffect at the end of this method.
        val controller = rememberSaveableStateController()
    
        val current = KeyedRender(
            key = KeyedScreen.findKey(<http://render.top|render.top>),
            content = <http://render.top|render.top>
        )
    
        val tracker = remember {
            RenderTracker(
                current = current
            )
        }
    
        tracker.update(page)
    
        AnimatedContent(
            targetState = BackStackState(
                key = tracker.current.key,
                size = render.size,
            ),
            transitionSpec = {
                val difference = targetState.size - initialState.size
    
                when {
                    difference > 0 -> {
                        ContentTransform(
                            targetContentEnter = slideInVertically(
                                initialOffsetY = { fullHeight -> fullHeight },
                                animationSpec = animationSpec()
                            ),
                            initialContentExit = fadeOut(
                                animationSpec = animationSpec()
                            )
                        ).apply {
                            targetContentZIndex = 1f
                        }
                    }
                    difference < 0 -> {
                        ContentTransform(
                            targetContentEnter = fadeIn(
                                animationSpec = animationSpec()
                            ),
                            initialContentExit = slideOutVertically(
                                targetOffsetY = { fullHeight -> fullHeight },
                                animationSpec = animationSpec()
                            )
                        )
                    }
                    else -> {
                        ContentTransform(
                            targetContentEnter = EnterTransition.None,
                            initialContentExit = ExitTransition.None
                        )
                    }
                }
            },
            content = { (key) ->
                val currentRender = tracker.forKey(key)
    
                controller.SaveableStateProvider(
                    key = key,
                    content = { current ->
                        Content(currentRender.content)
                    }
                )
            }
        )
    
        SideEffect {
            controller.updateKeys(
                currentKeys = render.stack.map(KeyedScreen::findKey)
            )
        }
    }
    
    private data class RenderTracker<T : Any>(
        var previous: KeyedRender<T>? = null,
        var current: KeyedRender<T>
    ) {
    
        fun update(render: KeyedRender<T>) {
            if (current != render) {
                previous = current
                current = render
            }
        }
    
        fun forKey(
            key: String
        ): KeyedRender<T> {
            val current = current.takeIf { page ->
                page.key == key
            }
    
            return checkNotNull(current ?: previous)
        }
    }
    
    private data class KeyedRender<T : Any>(
        val key: String,
        val content: T,
    )
    
    private data class BackStackState(
        val key: String,
        val size: Int,
    )
    Update: Its working! 🙂 The issue was even further up the chain! Ill share some details and my understanding of how its all connected. To begin with, the entire app is composed of screens like BackStackScreen above and driven by a multiplatform setup I built a while back. Each screen is coupled with a
    Renderer
    that just turns it into a composable function basically; renderers are stored in a map and looked up by the screens type in the
    Content
    composable. This is where I think the issue really started, I think it will make sense with a code snippet - primary difference being the introduction of key at the top level, and a
    SaveableStateHolder
    that contains the ui-state for that rendering. Please let me know if this is a bad idea! My understanding is that the state is now saved "above" the renderer, and each renderer can instead opt to specify child-states it will handle itself. BackStackScreen lives until either the key changes or its parent is destroyed. I still couldnt get it to continue functioning using your suggestion with key+rememberUpdatedState, so the "rendering tracking" is done like in the code snippet I posted earlier today. Without key:
    @Composable
    fun <Render : Any> Content(
        render: Render,
        modifier: Modifier = Modifier,
        type: Class<Render> = render.javaClass
    ) {
        val resolver = checkNotNull(LocalResolver.current)
    
        val renderer = remember(type) {
            resolver.renderer(type)
        }
    
        Box(
            modifier = modifier,
            propagateMinConstraints = true,
            content = {
                renderer.Render(render)
            }
        )
    }
    With key:
    @Composable
    fun <Render : Any> Content(
        render: Render,
        modifier: Modifier = Modifier,
        type: Class<Render> = render.javaClass
    ) {
        val resolver = checkNotNull(LocalResolver.current)
    
        val holder = rememberSaveableStateHolder()
    
        val renderKey = KeyedScreen.findKey(render) 
    
        key(renderKey) {
            val renderer = remember {
                resolver.renderer(type)
            }
    
            holder.SaveableStateProvider(
                key = renderKey,
                content = {
                    Box(
                        modifier = modifier,
                        propagateMinConstraints = true,
                        content = {
                            renderer.Render(render)
                        }
                    )
                }
            )
        }
    }
    Doris Liu

    Doris Liu

    1 year ago
    Hey @Zoltan Demant, sorry for the late reply. Seems like it's an issue with AnimatedContent. For content to be restored properly the chain of
    key
    s has be consistent.
    AnimatedContent
    internally uses the
    targetState
    object as key: https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/[…]in/kotlin/androidx/compose/animation/AnimatedContent.kt;l=676 As a result, when the object is re-created, the key changes. Inside that content, you still provide the correct key for save and restore. But because the outer key has (incorrectly) changed, your content can't be restored.
    Thanks for bringing this to my attention. It will be fixed in an upcoming release. 🙂
    Zoltan Demant

    Zoltan Demant

    1 year ago
    Hey! No worries, thanks for getting back to me! 🙂 That makes perfect sense and everything is now up and running, Ill just move over to your solution when thats been fixed! Is there a bugtracker for it? If not, no worries; Im sure Ill pick it up in the changelogs once it does land!
    Doris Liu

    Doris Liu

    1 year ago