[Jetcaster] Fixed to enable SharedElementTransition to run in the Grid list #1492
Conversation
…o jetcaster/remove-redundancy
# Conflicts: # Jetcaster/mobile/src/main/java/com/example/jetcaster/ui/home/Home.kt
simona-anomis
requested review from
riggaroo and
simona-anomis
and removed request for
bentrengrove
| CompositionLocalProvider( | ||
| LocalAnimatedVisibilityScope provides animatedContentScope | ||
| ) { |
There was a problem hiding this comment.
Whats the reasoning for moving this further down the tree?
There was a problem hiding this comment.
I noticed an issue where the animation was being executed in other areas when the AnimationScope was propagated to the SupportingPane. To address this, I modified it so that the scope is passed only to the node where the animation should occur.
Fold_ux.mp4
Jetcaster/mobile/src/main/java/com/example/jetcaster/util/SafeSharedElement.kt
Outdated
Show resolved
Hide resolved
|
LGTM, but I'll let @riggaroo give the final approval before merging :) thanks for doing this @yongsuk44 |
| Box( | ||
| modifier = Modifier | ||
| .background(placeholderBrush) | ||
| .fillMaxSize() |
There was a problem hiding this comment.
I think this is currently part of the design :) So i think it needs to be left in.
There was a problem hiding this comment.
As you can see, the background created in advance made the UX appear awkward during the image animation, so I removed it. Do you think a rollback is needed?
before_img_box.mp4
after_img_box.mp4
|
Thanks @yongsuk44 for working on this and @simona-anomis for tagging! This change may need to wait for the next stable release of Compose, since official Lazy Grid support for shared elements only just got merged (https://android-review.git.corp.google.com/c/platform/frameworks/support/+/3287098) . I'm just checking in with Doris to confirm what issues we'd see. |
We will need to unfortunately wait for Compose 1.8 for this upgrade, as right now even though shared elements look like they are working okay in Lazy Grids, they could cause unknown crashes and issues with scrolling etc. |
|
Spoke to @riggaroo , we will merge this now into my branch #1489 Then, when 1.8 is stable, we'll merge that into main :) thanks for the contribution @yongsuk44 |
simona-anomis
merged commit e25deeb
into
android:anomis/jetcaster-ui-update
The PR for #1489.