Change default navigation animations to be animators [167430145]

Fixed

Bug

Status Update

No update yet.

Description

jb...@google.com

created issue #1

Sep 1, 2020 10:32PM

The current Navigation default effects are not only Animations, but they are also inside of sets. This causes issues (like this one) that do not happen when using Animator, Transition, or Androidx Transition. Animator was built as a follow up to Animation, specifically to handle its short comings. We should be using Animator as our default effects instead of Animation.

Because these resources are part of the public API and we don't want to remove or change their directory, we will attempt to have them remain in the same R.anim res location, and just change their underlying implementation.

Comments

ap...@google.com <ap...@google.com> #2Sep 2, 2020 10:50PM

Still crashing with Compose 1.7.4.
I also tried moving sharedElement to BoxWithConstraints (rather than in its content), but did not help.
So the layout hierarchy looks like this (with more details and with "..." I replaced series of "normal" layouts):
NavHost -> ... -> AnimatedVisibility -> ... -> BoxWithConstraints with sharedElement -> some content
\> ... -> AnimatedVisibility -> ... -> LazyList -> ... -> Animated Visibility -> Selection container -> BoxWithConstraints with sharedElement -> some content

Any ideas?

ap...@google.com <ap...@google.com> #3Sep 14, 2020 05:47PM

In your BoxWithConstraints do you do any conditional composition? Things like:

if (constraintsExceedsCertainThreshold) composeSomeContent() else someOtherContent()

ap...@google.com <ap...@google.com> #4Sep 16, 2020 04:26PM

No, it's just a `Text` which has different style depending on constraints.
Also note `sharedElement` is put on `BoxWithContraints` now, not on the content.
Thanks!

jb...@google.com <jb...@google.com> #5Sep 22, 2020 10:39PM

Marked as fixed.

Based on the stacktrace, it seems like the item in question is composed but never placed. Do you use custom precompose or prefetch logic in your lazy list?

ap...@google.com <ap...@google.com> #6Sep 24, 2020 09:34AM

No, nothing custom. Just the default prefetch logic.
There is also one `AnimatedContent` above the `BoxWithConstraints`. Could this cause issues?
Thank you for looking into this.

ap...@google.com <ap...@google.com> #7Sep 24, 2020 09:36AM

The AnimatedContent never skips measuring/placing children after it composes them. So I don't expect that to be the issue.

Since you mentioned the modifier is on BoxWithConstraints rather than on the children of BoxWithConstraints, my guess is it's most likely related to LazyList.

Can you clarify what type of LazyList you are using? Is it LazyColumn/Row or LazyGrid, or something else?

ap...@google.com <ap...@google.com> #8Sep 24, 2020 10:08AM

Makes sense, thanks.
It's a LazyColumn.

ap...@google.com <ap...@google.com> #9Sep 24, 2020 10:10AM

One interesting observation is that this stack trace did not seem to come from the ComposeView with our app's Activity content. Because I did a release with copy pasted setContent + ComposeView subclass that is catching exceptions in dispatchDraw and it did not catch this crash. Is the animations framework doing content composition outside the app's composition context? One note: at the time I tried this exception catching, sharedElement was applied to the content of BoxWithConstraints (rather than to the BoxWithConstraints itself), but I don't think this would have caused the exception catching not to work.

ap...@google.com <ap...@google.com> #10Sep 24, 2020 11:29AM

Hey, sorry to insist - any more ideas here?

ap...@google.com <ap...@google.com> #11Sep 24, 2020 11:32AM

I'm investigating what would cause items to be composed but not placed.

We'll submit a fix in shared element in 1.8 to account for that scenario if that's an intended behavior.

Have you tried a recent 1.8 release and see if it's reproducible?

ap...@google.com <ap...@google.com> #12Sep 24, 2020 11:52AM

Thank you!

Unfortunately we cannot release publicly with 1.8 alpha, and we never reproed this issue locally even with 1.7.x.