mxalbert1996/compose-shared-elements

Nested SharedElements are misplaced in Overlay during transition

Closed this issue · 8 comments

Dude, you've done a great job maintaining the original library, thanks for that.
I've been exploring its capabilities and found a bug.

The problem

Consider a following use-case: we have a screen with a list of cards and want to animate a clicked card and some of its children to a new screen.
More generally, we want a transition of a container SharedElement with some other transition of its children SharedElements. I found out that this bug occurs when using a navigation-compose library, specifically for me it is androidx.navigation:navigation-compose:2.4.0-alpha10. Compose versions do not affect this bug, but mine is 1.0.3.
Here's some sample code: NavHost is itself inside of a SharedElementsRoot, switching composables for it.

SharedElementsRoot {
    Surface(
        modifier = Modifier
            .fillMaxSize(),
        color = MaterialTheme.colors.background
    ) {
        val navController = rememberNavController()
        NavHost(
            navController = navController,
            startDestination = "First_screen"
        ) {
            composable(route = "First_screen") {
                LazyColumn(
                    contentPadding = PaddingValues(vertical = 100.dp) // Simulating position in the list
                ) {
                    item {
                        SharedElement(
                            key = "Container",
                            screenKey = "First"
                        ) {
                            SharedCard {
                                navController.navigate("Second_screen")
                            }
                        }
                    }
                }
            }
            composable(route = "Second_screen") {
                SharedElement(
                    key = "Container",
                    screenKey = "Second"
                ) {
                    SharedCardContent {
                        navController.popBackStack()
                    }
                }
            }
        }
    }
}

And here is the code for containers:

@Composable
fun SharedCard(
    onClick: () -> Unit
) {
    Card(
        modifier = Modifier
            .fillMaxWidth(),
        elevation = 2.dp,
        shape = RoundedCornerShape(5.dp),
        onClick = onClick
    ) {
        Column(
            modifier = Modifier
                .padding(
                    top = 10.dp,
                    bottom = 8.dp,
                    start = 15.dp,
                    end = 15.dp
                )
        ) {
            Text(
                text = "Card title",
                style = MaterialTheme.typography.h6
            )

            Spacer(modifier = Modifier.height(10.dp))

            Column(
                verticalArrangement = Arrangement.spacedBy(8.dp)
            ) {

                SharedElement(
                    key = "author",
                    screenKey = "First"
                ) {
                    Text("Author name")
                }

                SharedElement(
                    key = "subject",
                    screenKey = "First"
                ) {
                    Text("Subject name")
                }
            }
        }
    }
}

@Composable
fun SharedCardContent(
    onClick: () -> Unit
) {
    Surface(
        modifier = Modifier
            .fillMaxWidth()
            .clickable(onClick = onClick),
        color = MaterialTheme.colors.surface,
        elevation = 1.dp
    ) {
        Column(
            modifier = Modifier
                .fillMaxWidth()
                .padding(20.dp),
            verticalArrangement = Arrangement.spacedBy(10.dp)
        ) {
            SharedElement(
                key = "author",
                screenKey = "Second"
            ) {
                Text("Author: Author name")
            }

            SharedElement(
                key = "subject",
                screenKey = "Second"
            ) {
                Text("Subject: Subject name")
            }

        }
    }
}

It looks exactly like we want during transition forwards, but fails to position nested SharedElements during transition backwards:
1

However, if for some reason the whole tree is recomposed after the forwards transition, transition backwards plays as expected:
2
I assume it's because when SharedElementsRoot is recomposed, it considers the Second screen to be the Start state of the transition and vice versa, so the actual backwards transition becomes the forwards one, which explains its failure to execute the actual forwards transition as intended.
It would be super cool if you could help fix this issue. Thanks!

I experimented further and found out that SharedElementsRoot doesn't have to be recomposed to handle backwards transition correctly. It is enough for the Second screen to leave the composition and come back, for example by means of navigating to unrelated screen and popping the navigation back stack.
After that both transitions are executed correctly until another backwards one.

Thanks for filing this! Should be fixed by 972c0bf.

Some children SharedElements are now inconsistently rendered in the overlay. It was hard to catch and record in test project, but you can see it happening with the last element here:

1

While testing it inside my main project, I noticed that the same children behave the same way inside items with the same position.
Increasing duration revealed that they are an fact rendered, but there's some kind of strange state causing them to have the wrong alpha value depending on the fraction inside MathUtils.kt/calculateAlpha
2
I would say that the fadeFraction in Placeholder composable plays the key role here

Please provide a repro so that I can debug.

I'm getting a Failed to resolve: com.mxalbert.sharedelements:shared-elements:0.1.0-SNAPSHOT error upon building. The needed repo was added to project buildscript:

repositories {
    google()
    mavenCentral()
    maven { url 'https://s01.oss.sonatype.org/content/repositories/snapshots' }
}

Any ideas?

I just copied over the files to make it work. Here is a test repo

Fixed by 2db736a.

Btw, the reason of the unresolved error is that you are putting the repo definition in the repositories of buildscript, which is wrong. You need to put it in the project's repositories. In the project you attached, it's in setting.gradle.