1858020 - Bring back Crossfade animations for Review Checker shadows

Status: NEW

(Fenix :: Shopping, task, P3)

Description

[Verdi [:verdi] Best to slack me]

Attachment: shadows.mp4

Steps to reproduce

1. Open the review checker

Expected behavior

When the the cards load, they appear fully rendered.

Actual behavior

When the cards load, the shadows on the sides of the cards (and the shadow on the top of the first card) are missing. Then they pop in after everything else is loaded.

Device information

• Firefox version: 120.0a1 (Build #2015978850)
• Android device model: Samsung Galaxy S9
• Android OS version: 10

Any additional information?

Comment 1

[Noah Bond [:007]]

Hey Verdi, I think this is because of a crossfade transition we're currently applying between the loading animation placeholder and the product analysis. This effectively causes the loading animation to still be present for ~300ms as the UI transitions to the product analysis.

Should we instead opt for an instantaneous state transition (i.e. no animation) between these two states?

Let me know if you would like to do some paired prototyping.

Comment 2

[Verdi [:verdi] Best to slack me]

Why does the transition not affect the shadows on the top and bottom? I don't know what happens in code but it appears as if the transition is limited to a box that is the with of the cards without a shadow but includes the space between cards. Then once the transition is done we go, "oh I have to expand the box to show the full shadows (the stuff on the side)." I'd like to see if we can fix that instead of removing the transition.

Comment 3

[Noah Bond [:007]]

After doing ~8 hours of investigating into this so far, this is likely an issue with the Crossfade component from Google causing unnecessary recompositions (full re-draws) of the UI. (Crossfade is an animation wrapper API that's supposed to make it seamless to animate state changes) An additional hypothesis could be that this component does not play well with animating state changes involving elevation (shadows), further exacerbating the recomposition issue.

Technical notes can be found here

Comment 4

[Noah Bond [:007]]

Hey Chris and Verdi, after spending a few more hours investigating this, I wanted to post my notes here for whoever picks this up.

Findings

It is indeed an issue of using Crossfade combined with some weirdness of having it synced into our Redux pattern.

Expanding either the settings or grade info cards causes a full recomposition here.

• To handle this, the crossfade should ignore state changes that are simply data model updates (e.g. a card expansion boolean was changed).

As a sanity check, I removed the elevation from all of the the cards and confirmed that there aren't any other weird visual artefacts appearing as a result of the Crossfade. So it is for sure a weird interaction of the Crossfade wrapper transitioning something that has elevation (elevation is the UI property on Android that drives the rendering of shadows).

Local branch I was using to debug

Recommendations

• We should still fix the recompositions (the redraws of the UI) to help with performance as part of Bug 1859984.
• Two options for the short term are in front of us for this particular issue: 1) Gut the Crossfade 2) Ignore it until we have time for a proper fix.
  • Option 1 would at least hide the UI weirdness (if the defect is highly undesirable) until a better animation solution can be found.

Either short term fix won't fix the unnecessary recomps, so that should still be a priority.

Comment 5

[Chris Peterson [:cpeterson]]

We should still fix the recompositions (the redraws of the UI) to help with performance as part of Bug 1859984.

Will fixing bug 1859984 resolve this shadow issue? Or just make it less obvious?

Two options for the short term are in front of us for this particular issue: 1) Gut the Crossfade 2) Ignore it until we have time for a proper fix.
Option 1 would at least hide the UI weirdness (if the defect is highly undesirable) until a better animation solution can be found.

Does this Crossfade issue only affect the Review Checker? Does this issue all Android OS versions?

Does "gutting the Crossfade" mean removing the component? Do we use it in UI components besides Review Checker?

Comment 6

[Noah Bond [:007]]

Will fixing bug 1859984 resolve this shadow issue? Or just make it less obvious?
It could help make it better, but I believe they're two distinct issues.

Does this Crossfade issue only affect the Review Checker?
Yes; the Crossfade component is only being used in the Review Checker.

Does this issue all Android OS versions?
Yes (maybe not Android 5, as the shadows rendered on that version aren’t the same as 6+).

Does "gutting the Crossfade" mean removing the component? Do we use it in UI components besides Review Checker?
Yes. It is not being used anywhere outside of Review Checker (it's used in two locations).

Comment 7

[Chris Peterson [:cpeterson]]

Crossfade animations were removed in bug 1865452.