Shared transitions #1550

stagg · 2024-08-02T18:38:24Z

Shared Element API

Creating two base elements for using shared transitions SharedElementTransitionLayout and SharedElementTransitionScope
This is done to be able to indirectly provide a SharedTransitionScope to child elements (ie Screen) without having to pipe the scope all the way down into Circuit Ui's
Circuit foundation is updated to provide a Navigation scoped AnimatedVisibilityScope via a new AnimatedNavDecoration variant of NavDecoration
Circuit overlays were also updated to provide an Overlay AnimatedVisibilityScope

`SharedElementTransitionScope`

Provides a SharedTransitionScope for the standard shared elements/shared bounds animations
The SharedElementTransitionScope instance can then be access via the composable SharedElementTransitionScope
The SharedElementTransitionScope composable should be used like SharedTransitionScope to access the sharedElement and sharedBound modifiers
Based on where/how the shared element will be shared getAnimatedScope or requireAnimatedScope can be used to access a needed AnimatedVisibilityScope
AnimatedVisibilityScopes can be provided with the ProvideAnimatedTransitionScope composable
By default Circuit provides Navigation and Overlay scopes when a SharedElementTransitionScope is available

`SharedElementTransitionLayout`

SharedElementTransitionLayout is a layout that creates and provides a SharedElementTransitionScope
Normally SharedElementTransitionLayout should be setup around the a standard circuit setup, between CircuitCompositionLocals and before NavigableCircuitContent or ContentWithOverlays
A PreviewSharedElementTransitionLayout is also provided to help with @Preview use

Update star to use SharedElementTransitions

Wired the shared elements into Star as a demonstration for the Navigation and Overlay use cases
Gesture predictive back (for Android) has been updated to use a seekable transition to drive shared element transitions

Scenario	Demo
No shared elements	none.mp4
Navigation Scope	navigation.mp4
Overlay Scope	overlay.mp4
Seekable Predictive back	predictive-back.mp4

…ansitionScope`

ZacSweers · 2024-08-03T23:39:41Z

Some high level early thoughts

Can we add a control to disable the default nav decoration? This way sharedBounds() could be used without colliding with it. I'm almost wondering if we should disable the nav decoration by default if a shared element is present
Related from the above, can we leverage sharedBounds() in one of the samples?
Overlay API definitely seems like something we should get figured out with this

chrisbanes · 2024-08-04T07:11:51Z

Do you have any sense of how much work is involved? I'd like to see a bit more progress before landing this personally, as I think there's going to be tests which are broken.

If only just getting the sample looking right to prove it out fully.

chrisbanes · 2024-08-04T07:08:06Z

...ion/src/commonMain/kotlin/com/slack/circuit/foundation/internal/NoOpSharedTransitionScope.kt

+      layoutDirection: LayoutDirection,
+      density: Density,
+    ): Path {
+      path.reset()


Use rewind() it's cheaper for the next outline

riggaroo · 2024-08-05T12:29:20Z

...ion/src/commonMain/kotlin/com/slack/circuit/foundation/internal/NoOpSharedTransitionScope.kt

+ */
+@InternalCircuitApi
+@OptIn(ExperimentalSharedTransitionApi::class)
+public data object NoOpSharedTransitionScope : SharedTransitionScope {


Curious on the need for this, vs just not adding the modifiers / layout when not required. Hard to really understand all the details of Circuit but maybe you've explored that option already?

Was initially experimenting with this from the view that a Screen can be reused regardless of the context. Thinking on it some more its not really needed, a Screen really shouldn't be getting reused as such.

riggaroo · 2024-08-05T12:46:30Z

samples/star/src/commonMain/kotlin/com/slack/circuit/star/petdetail/PetPhotoCarouselScreen.kt

+        name = state.name,
+        photoUrlMemoryCacheKey = state.photoUrlMemoryCacheKey,
+        modifier =
+          Modifier.sharedElementWithCallerManagedVisibility(


Is there no way to expose an AnimatedVisibilityScope from Navigation framework here, so that you could use the standard shared element modifiers?

I think from a usability perspective for developers, this would be easier to work with than needing to use sharedElementWithCallerManagedVisibility.

I also think you'd need to use it to pass through the correct Transition object, if looking at supporting predictive back correctly.

Take a look at how its used here in Jetsnack for animating rounded corners as the animation progresses - https://github.com/android/compose-samples/pull/1314/files#diff-cd670f26d99bb64790bdbee3a8965fca378714cb988996912f2e0e137931b72dR156-R163
For a simple example of Shared elements with SeekableTransitionState, no NavHost usage, we have this snippet that might be helpful to tie your transitions into - android/snippets#273

You may want to consider using context receivers in Kotlin if your library would use an experimental feature (Kotlin/KEEP#259), you could then expose both the AnimatedVisibilityScope and SharedTransitionScope to one function without needing to pass it around.

This case is a bit different as it's dealing with an element being shared across navigation and again as an Overlay is being shown. Need to really work on the Overlay API for this a bit more, currently missing a AnimatedVisibilityScope from it 😅 Think the goal would be to dynamically switch to the correct AnimatedVisibilityScope based on whats happening.

Thanks for the SeekableTransitionState snippet 🙇🏻 The predictive back implementation/api is definitely going to need to provide some of the progress information.

riggaroo · 2024-08-05T12:59:34Z

...uit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/SharedElementTransition.kt

+ * [SharedElementTransitionScope].
+ */
+@Composable
+private fun staticAnimatedVisibilityScope(): AnimatedVisibilityScope {


This feels like it could potentially introduce some UI inconsistencies, is it possible that a developer somehow relies on the scope to perform a different UI animations unrelated to shared elements? Maybe it'd be less error-prone to rather have a nullable scope instead.

Or have the Nav framework itself always expose the AnimatedVisibilityScope and throw an exception if its not there as something would've gone quite wrong to have that happen?

I second what Rebecca said. This custom scope will always have the target visibility be EnterExitState.Visible. That would potentially prevent the shared elements from identifying the target bounds. More specifically when both sharedElement/sharedBounds call sites of the same key report their target visibility to be visible, we would not know which bounds to use as the target bounds since they are both "entering".

Also, consumers of this API may be confused as to why their Modiifer.animateEnterExit doesn't work. A nullable scope in this case would be more explicit.

Yup, it's pretty broken always being in the single state right now. Going to lean on the nav for providing this.

riggaroo · 2024-08-05T13:15:14Z

Love the visuals to aid the PR description! Thanks for showcasing them 🎉

The animations are looking great too - love the back from full screen to the pager.

Could the Active Scope (layout.mp4) example at 0:07 be improved to have the image shared to that screen too? It seems like there is a blank screen at that point. Maybe its an image loading issue, perhaps it could be corrected by setting a placeholder cache key to the same as the memory cache key? .placeholderMemoryCacheKey("image-key") and having the placeholder be null, similar to this example https://developer.android.com/develop/ui/compose/animation/shared-elements/common-use-cases#async-images

From details -> list of dogs, the final clipping is applied immediately to the dog, consider animating the rounded corners too as it progresses between the states, the bottom corners can animation from rounded to straight - something similar was done here - https://youtu.be/PR6rz1QUkAM?si=DsGg4huAxgbel9c_&t=989

I do agree about getting predictive back working correctly too - as this may heavily influence your API design more, as developers would probably want to tie into the Transition object to do other surrounding animations, as well as it can help with debugging to be able to seek between the states.

stagg · 2024-08-13T01:16:29Z

Can we add a control to disable the default nav decoration? This way sharedBounds() could be used without colliding with it. I'm almost wondering if we should disable the nav decoration by default if a shared element is present

We're going to need some form of AnimatedVisibilityScope from the nav decoration to run the transitions, so would this set the animation to something predefined?

Do you have any sense of how much work is involved? I'd like to see a bit more progress before landing this personally, as I think there's going to be tests which are broken.

Think most of the work is in supporting seekable transitions for predictive back, then maybe building some simpler api's on top of that?

chrisbanes · 2024-08-13T08:10:47Z

Think most of the work is in supporting seekable transitions for predictive back, then maybe building some simpler api's on top of that?

Sounds good to me! I just don't see any tests being ran on CI?

ZacSweers · 2024-08-14T20:11:16Z

Looking better. Def game to make whatever changes we need to nav decoration APIs (and overlays)

ZacSweers · 2024-11-20T22:13:17Z

Some early thoughts from going around the sample before looking through the code

Is there a reason it never works the first time in debug? It seems to only work the second+ time. On release builds it's fine
Anything we can do to improve the transition for the photo viewer?

starsampleone.webm

Do you think it would be possible to, in the photo viewer when it's dragged but not flung, do a shared element transition instead?
The transition from the home list to the detail view feels asymmetric. When going from A -> B, the background content translates away. When going from B -> A, some content translates but the bottom nav bar translates up
I notice when seeking through predictive back that the top of the stack exerts a significant shadow, which I think is causing some of the artifacting I'm seeing in the above bits. Anything we should tweak in NavigableCircuitContent or elsewhere to improve this? What's particularly weird is that it appears behind some content on the previous screen and separate from the top screen too.

starsample2.webm

The sample crashes when I go to detail and rotate 😬. Doesn't crash on just the home screen.
For the card, I wonder if we can animate its color quicker. It looks a little like it blows out/flashes.
If the photo carousel is scrolled in the detail screen, the shared element transition still runs but it blips back to the first image at the start.

starsample3.webm

stagg · 2024-11-30T03:12:16Z

Is there a reason it never works the first time in debug? It seems to only work the second+ time. On release builds it's fine

Not sure I've run into this?

Anything we can do to improve the transition for the photo viewer?

Changed it a bit with 0d1b246

Do you think it would be possible to, in the photo viewer when it's dragged but not flung, do a shared element transition instead?

Nice idea, added with e972508

I notice when seeking through predictive back that the top of the stack exerts a significant shadow

Fixed with 2779634, the drawbacks of dark mode 😅

For the card, I wonder if we can animate its color quicker. It looks a little like it blows out/flashes.

If the photo carousel is scrolled in the detail screen, the shared element transition still runs but it blips back to the first image at the start.

Changed the animation timings for both of these. Jumps directly to the first image which fades in, but now won't do an odd cross fade.

The transition from the home list to the detail view feels asymmetric. When going from A -> B, the background content translates away. When going from B -> A, some content translates but the bottom nav bar translates up

Yaaa this has been the main problem. We don't know if there are shared transitions in time to disable the animated content transition.
We can either have the screens tell us there's going to be a shared animation, which is some of what I was playing with using AnimatedScreen. That or I think we'd need to rebuild AnimatedContent in some way to let us compose both screens ahead of time. Which lets us check for SharedTransitionScope.isTransitionActive, and then build the transition spec 😬

I like the idea of the AnimatedScreen/ AnimatedNavigationOverride as it'd dovetail into a more capable API for changing the enter/exit animation based on the screens involved in the transition. Think that should be a separate PR though.

ZacSweers

left some questions for my understanding in the PR! Can merge and answer them async. Some also here

do we still need the separate artifact now that these are in compose-multiplatform?

All the comments non-blocking or nits, def think there's a lot here and we should roll forward and iterate more in followup PRs. Awesome work!

ZacSweers · 2024-12-02T20:31:22Z

circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/ModifierExt.kt

Before you merge let's push these to com.slack.circuit.foundation.internal

ZacSweers · 2024-12-02T20:33:20Z

backstack/src/commonMain/kotlin/com/slack/circuit/backstack/Navigation.kt

    args: ImmutableList<T>,
    backStackDepth: Int,
    modifier: Modifier,
    content: @Composable (T) -> Unit,
  )
 }
+
+/** Argument provided to [NavDecoration] that exposes the underlying [Screen]. */
+public interface NavArgument {


what's the difference between this and accessing the screen via the args list (of records) that's passed in?

Outside an implementation of NavDecoration we won't be able to access the specific screen as it'd only be visible as T. Its needed for any animation override apis, not really for the current shared elements wok.

ZacSweers · 2024-12-02T20:34:10Z

circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigationDecoration.kt

+import kotlinx.collections.immutable.ImmutableList
+
+@OptIn(ExperimentalSharedTransitionApi::class)
+public abstract class AnimatedNavDecoration(


Let's add a big ol' doc on this and its usage on this

ZacSweers · 2024-12-02T20:35:35Z

circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigationDecoration.kt

+      Content(args, backStackDepth, modifier) { modifier ->
+        AnimatedContent(modifier = modifier, transitionSpec = transitionSpec()) { targetState ->
+          ProvideAnimatedTransitionScope(Navigation, this) {
+            AnimatedNavContent(targetState) { content(it) }


it's hard for me to look at this level of nesting and feel like these new compose animation APIs are supposed to be easy :(

ZacSweers · 2024-12-02T20:35:49Z

circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavigationDecoration.kt

+}
+
+@Stable
+public interface AnimatedNavDecorator<T : NavArgument, S : AnimatedNavState> {


same here re: doc

docs/shared-elements.md

ZacSweers · 2024-12-02T20:44:33Z

samples/star/src/androidMain/kotlin/com/slack/circuit/star/MainActivity.kt

@@ -60,7 +62,7 @@ class MainActivity @Inject constructor(private val circuit: Circuit) : AppCompat
      } else {
        val httpUrl = intent.data.toString().toHttpUrl()
        val animalId = httpUrl.pathSegments[1].substringAfterLast("-").toLong()
-        val petDetailScreen = PetDetailScreen(animalId, null)
+        val petDetailScreen = PetDetailScreen(animalId, null, null)


nit: let's name these args now

ZacSweers · 2024-12-02T20:46:47Z

samples/star/src/commonMain/kotlin/com/slack/circuit/star/transition/PetSharedTransitionKeys.kt

+// SPDX-License-Identifier: Apache-2.0
+package com.slack.circuit.star.transition
+
+interface SharedTransitionKey


do you think this is something worth encouraging in docs/promoting the marker interface to a shipped artifact?

Co-authored-by: Zac Sweers <[email protected]>

- Don't need these as Apis in Circuit proper

Co-authored-by: Zac Sweers <[email protected]>

stagg added 3 commits August 1, 2024 10:15

Compose 1.7 + fixes

1dfb846

Initial setup of SharedElementTransitionLayout and `SharedElementTr…

0860067

…ansitionScope`

Update star to use SharedElementTransitions

1fc77ae

stagg changed the base branch from 0.24.x to compose-1.7.x August 2, 2024 18:41

Spotless

c4461cc

chrisbanes reviewed Aug 4, 2024

View reviewed changes

riggaroo reviewed Aug 5, 2024

View reviewed changes

stagg added 6 commits August 13, 2024 18:28

Remove NoOpSharedTransitionScope

bc5cadc

Add a sharedBounds transition

00c8c99

Start looking into multiple animation scopes

460d9f0

Overlay testing

242bf20

Pick the animated scope based on overlay state

dcd1716

More shared transitions, make the image viewer animations better

e87b75e

This comment was marked as outdated.

Sign in to view

stagg added 8 commits August 15, 2024 18:29

WIP - AnimatedNavDecoration

656ccae

SharedElementTransitionState

c03f316

Merge branch 'main' into compose-1.7.x

29dc2c8

1.7.0-beta07

ec7f8c8

Merge branch 'compose-1.7.x' into j-shared-transition-scope

5be626b

Merge remote-tracking branch 'origin/main' into compose-1.7.x

35344f6

Bottom sheet compile fixes

10a80f7

Compose 1.7.0-rc01

f23c30d

stagg and others added 7 commits November 22, 2024 16:29

Disable gesture shadow if a transition is active

2779634

Changes to the item <-> detail animation

e6c51e3

Merge branch 'main' into j-shared-transition-scope

177096c

Api updates

5c39d64

tweaks

cc6e3cb

Better fade into image viewer

0d1b246

ImageViewer fling

e972508

ZacSweers approved these changes Dec 2, 2024

View reviewed changes

stagg and others added 15 commits December 5, 2024 17:44

Update docs/shared-elements.md

cbf1814

Co-authored-by: Zac Sweers <[email protected]>

Update docs/shared-elements.md

3cafe6c

Co-authored-by: Zac Sweers <[email protected]>

Update docs/shared-elements.md

05728b5

Co-authored-by: Zac Sweers <[email protected]>

Update docs/shared-elements.md

4f6bf11

Co-authored-by: Zac Sweers <[email protected]>

Move ModifierExt into sample

7f13a22

- Don't need these as Apis in Circuit proper

DelicateCircuitSharedElementsApi -> ExperimentalCircuitSharedElementsApi

5ebd965

Update SharedElementTransitionLayout docs

39336dd

getAnimatedScope -> findAnimatedScope

5eb591c

SharedElementTransitionScope helpers as properties

2f788c3

Start on AnimatedNavDecoration docs

891894e

Named args

995b28b

Make SharedTransitionKey an api

c319d2c

Merge branch 'main' into j-shared-transition-scope

074c9fa

api dump

6748c97

Update docs/shared-elements.md

9a01c29

Co-authored-by: Zac Sweers <[email protected]>

stagg enabled auto-merge December 6, 2024 03:26

stagg added this pull request to the merge queue Dec 6, 2024

Merged via the queue into main with commit f4a0fb7 Dec 6, 2024
5 checks passed

stagg deleted the j-shared-transition-scope branch December 6, 2024 03:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Shared transitions #1550

Shared transitions #1550

stagg commented Aug 2, 2024 •

edited

Loading

ZacSweers commented Aug 3, 2024

chrisbanes commented Aug 4, 2024

chrisbanes Aug 4, 2024

riggaroo Aug 5, 2024

stagg Aug 12, 2024 •

edited

Loading

riggaroo Aug 5, 2024

stagg Aug 12, 2024

riggaroo Aug 5, 2024

doris4lt Aug 5, 2024 •

edited

Loading

stagg Aug 13, 2024

riggaroo commented Aug 5, 2024

stagg commented Aug 13, 2024

chrisbanes commented Aug 13, 2024

This comment was marked as outdated.

ZacSweers commented Aug 14, 2024

ZacSweers commented Nov 20, 2024 •

edited

Loading

stagg commented Nov 30, 2024

ZacSweers left a comment

ZacSweers Dec 2, 2024

ZacSweers Dec 2, 2024

stagg Dec 6, 2024 •

edited

Loading

ZacSweers Dec 2, 2024

ZacSweers Dec 2, 2024

ZacSweers Dec 2, 2024

ZacSweers Dec 2, 2024

ZacSweers Dec 2, 2024

Shared transitions #1550

Shared transitions #1550

Conversation

stagg commented Aug 2, 2024 • edited Loading

Shared Element API

SharedElementTransitionScope

SharedElementTransitionLayout

Update star to use SharedElementTransitions

ZacSweers commented Aug 3, 2024

chrisbanes commented Aug 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stagg Aug 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

doris4lt Aug 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

riggaroo commented Aug 5, 2024

stagg commented Aug 13, 2024

chrisbanes commented Aug 13, 2024

This comment was marked as outdated.

ZacSweers commented Aug 14, 2024

ZacSweers commented Nov 20, 2024 • edited Loading

stagg commented Nov 30, 2024

ZacSweers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stagg Dec 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stagg commented Aug 2, 2024 •

edited

Loading

`SharedElementTransitionScope`

`SharedElementTransitionLayout`

stagg Aug 12, 2024 •

edited

Loading

doris4lt Aug 5, 2024 •

edited

Loading

ZacSweers commented Nov 20, 2024 •

edited

Loading

stagg Dec 6, 2024 •

edited

Loading