Show default fallback when no ad is available to serve #3817

Open
jasti opened this Issue Jun 28, 2016 · 14 comments

Projects

Current - Core AMP in AMP Ads

8 participants

@jasti
Collaborator
jasti commented Jun 28, 2016 edited

On AMP pages when an adserver doesn't respond with an ad, the AMP runtime will collapse the ad as long as it doesn't affect the user scroll position. amp-ad also supports a fallback attribute which is shown when ad isn't available for the slot.

In cases where the ad slot can't be collapsed and the publisher hasn't provided a [fallback] we show blank boxes, which is not good UX. Instead, we should show a default fallback image which has the following components:
a) fallback image with a grey background, similar to Google Contributor
image
b) Text : "We don't have an ad to show you. Enjoy!"
Text up for debate :)

CC @cramforce @kashyapnitin

@jridgewell
Member

Should show prioritize the provided [fallback] element, falling back to one of the defaults you've suggested, when we can't resize?

@cramforce
Member

@jridgewell Yeah, the idea is to make a fallback for the fallback if the page does not provide one.

@jasti
Collaborator
jasti commented Jun 28, 2016

Yup, I've also updated the issue with case. Priority is
Try to show ad > Publisher provided fallback > Try to collapse > Default fallback (this issue)

@jridgewell
Member

Whoops, I misread.

@cramforce
Member

It is worth reconsidering whether collapse should have higher priority over
pub-supplied fallback.

On Tue, Jun 28, 2016 at 3:41 PM, Justin Ridgewell notifications@github.com
wrote:

Whoops, I misread.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3817 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAFeT7vVzs1wySJuOUXm4CIwJT-5cRPHks5qQaMKgaJpZM4JAmFW
.

@rudygalfi rudygalfi added this to the Current milestone Jun 29, 2016
@zhouyx zhouyx self-assigned this Jul 6, 2016
@zhouyx
Collaborator
zhouyx commented Jul 6, 2016

One thing I find is attempchangeHeight to <amp-ad-3p-impl> won't trigger a resize to its parent <amp-ad>? that's a bug right? @cramforce @jridgewell

@cramforce
Member

Right. One we need to fix before we ship A4A

@cramforce
Member
@michaelkleber
Collaborator

@tdrl See comment about attemptchangeHeight not playing well with <amp-ad-3p-impl>?

@cramforce
Member

Tracking in #3936

@keithwrightbos
Collaborator

Definitely appears to be related to A4A change. Please feel free to re-assign to me.

What is the best way to pass reference to the parent element (amp-ad) to allow for calling attemptchangeHeight? Or would it be sufficient to call the following (after verifying parent element is amp-ad):

resourcesFor(this.getWin()).attemptChangeHeight(
    this.element.parentElement, 0, /* newWidth */ undefined, () => {
     this.element.parentElement.style.display = 'none';
 });
@jasti
Collaborator
jasti commented Jul 28, 2016

@zhouyx in light of your new PR, here is a lo-fi version of how was thinking this should work overall. I'll make a better version tomorrow, but wanted to get this out for your feedback.
img_0470

@lannka @jridgewell FYI.

@tdrl
Collaborator
tdrl commented Jul 28, 2016

In the wake of Dima's PRs to make A4A upgrade-compliant and Keith's
followup, A4A should implement the same fallback behavior as 3p-impl at
this point, at least for ads rendered via the iframe fallback path. I
think we'll need to examine behavior for the shadow DOM rendering path.

On Thu, Jul 28, 2016 at 12:33 PM, Vamsee Jasti notifications@github.com
wrote:

@zhouyx https://github.com/zhouyx in light of your new PR, here is a
lo-fi version of how was thinking this should work overall. I'll make a
better version tomorrow, but wanted to get this out for your feedback.
[image: img_0470]
https://cloud.githubusercontent.com/assets/2366341/17220893/5ada5c9a-54bf-11e6-8459-fcf39fdade8f.JPG

@lannka https://github.com/lannka @jridgewell
https://github.com/jridgewell FYI.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3817 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AQ4oaCS2BCPVTSeLTiL62x3-mjdQuDwyks5qaNnCgaJpZM4JAmFW
.

Terran Lane email: tdrl@google.com
Google Cambridge, MA Knowledge Graph
"But I don't want to go among mad people," Alice remarked.
"Oh, you can't help that," said the Cat: "we're all mad here. I'm
mad. You're mad."
"How do you know I'm mad?" said Alice.
"You must be," said the Cat, "or you wouldn't have come here."

@jasti jasti assigned jasti and unassigned zhouyx Sep 16, 2016
@lannka lannka referenced this issue Sep 27, 2016
Open

Ad Tech Developers, please support render-start #5234

9 of 65 tasks complete
@rudygalfi
Contributor

Part of #5918

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment