Amphtml: Rendered A4A ad is never assigned the "amp-animate" CSS class

Created on 28 Feb 2017  Â·  29Comments  Â·  Source: ampproject/amphtml

I managed to successfully serve an a4a ad using Cloudflare's network implementation. The ad validates and renders in a friendly iframe, but animations never start because in a4a they only run when the "amp-animate" CSS class is present, and this class is never added by amp runtime.

Searching through amphtml code base reveals that the string "amp-animate" does not appear outside of a4a validator code. Has adding of this class to rendered a4a creatives not been implemented yet?

A4A fast fetch Soon Stale Bug monetization

All 29 comments

to @dvoytenko
CC @ampproject/a4a

What is not clear from the a4a spec is whether the amp-animate class should be added by amp at runtime once the ad has been loaded, or if this class should already be present in the ad html and the runtime just has the option of removing it if it wants to stop animations. It could be just my misunderstanding of intended behavior.

Lets clarify in the spec that this SHOULD be present in the markup.

@lannka While we don't yet remove this, we should at least add it.

@cramforce on which element do we recommend to put this amp-animate class? Actually wouldn't it be better for runtime to insert amp-animate to body element when the creative is ready to animate?

yes, that would be better.

On Wed, Mar 1, 2017 at 8:18 PM, Hongfei Ding notifications@github.com
wrote:

@cramforce https://github.com/cramforce on which element do we
recommend to put this amp-animate class? Actually wouldn't it be better
for runtime to insert amp-animate to body element when the creative is
ready to animate?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/ampproject/amphtml/issues/7846#issuecomment-283551969,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAFeTzU7vgxw4rpGw94KvDIUcGd3aYxgks5rhkMkgaJpZM4MO81f
.

@cramforce to let AMP have full control over what and when to animate, shall we actually enforce "no amp-animate class in any elements" in the document?

An alternative concept:

  1. We can start (e.g. via bolierplate or re-serialization) with always setting * {animaton-name: none !important}. When ready, this CSS will be removed by AMP Runtime.
  2. Similarly, if we wanted to pause all animations at any time (as opposed to reset), we can set * {animation-play-state: paused} and remove it to continue.

We should switch to the concept recommended by @dvoytenko. This is just way better.

@aghassemi mentioned that setting animation-play-state: paused won't work on iOS when also setting -webkit-overflow-scrolling: touch (which we always do).

See this example, which works properly everywhere but on iOS.

Lets UA detect and use animaton-name: none !important where paused is not supported.

Thought that was fixed. Well, that's annoying. Is that still a problem for our embeds in iframes? Can we check against a friendly iframe? The embeds are never scrolled, so we might be in luck there.

Cancelling animation is pretty hard hammer, so let's see if we can reduce % of cases where it happens.

Changing the scroll mode for the frame doesn't seem to work either :( see example

I'd like to discuss this during this week's design review. There are some overall animation changes I'd like to do an wrap them behind amp-animation extension.

@alanorozco Could you please document the final decision and next steps, if any, here?

Will do.

Will be at #10650 design review

@alanorozco @dvoytenko do you want to summarize the conclusion and action items here?

@alanorozco Will test couple more situations. But we are leaning toward wild-card control that's completely automatic.

Apologies for moving slowly on this. As this change involves low-level changes to the runtime, we've been trying to make this decision with care as not to affect usage or performance in a significant way.

We ran benchmarks on the performance of three different types of lock selectors. The methodology for the benchmarks is described here and a full comparison table is here.

We've decided to go for a wildcard lock. This means that no special considerations (like marking elements with a specific classname) need to be made from the document author's side. The validator does not yet reflect this, so it will need to be updated before CSS animations are fully supported.

🥇

@alanorozco would you like to close this and create a new issue for validator changes?
Also, are there any runtime changes for this decision?

May I ask when the validator change will happen?

To @alanorozco

Validator changes are live.

This issue hasn't been updated in awhile. @alanorozco Do you have any updates?

Hi, I'm seeing an issue with A4A animations not running on iOS (#15260) . Can't quite work out whether it is related to this issue, so thought I'd flag it. Thanks

This issue hasn't been updated in awhile. @alanorozco Do you have any updates?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

sryze picture sryze  Â·  3Comments

mkhatib picture mkhatib  Â·  3Comments

samanthamorco picture samanthamorco  Â·  3Comments

radiovisual picture radiovisual  Â·  3Comments

edhollinghurst picture edhollinghurst  Â·  3Comments