-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Intent to implement: amp-ad-banner #8334
Comments
This looks great!
|
Instead of introducing a new element, I tried to leverage the |
Just a note that this should also be tested when delivered from the DFP server. Sometimes, I've seen that the ad gets served into multiple cross domain iframes. Not sure if it matters. |
That example doesn't seem to work? The banner expands to the full-size of the frame which is exactly what we don't want. I think I didn't explain myself properly in the description. The banner does not specify the lightbox content. The markup would look like: <body>
<amp-ad-banner>
This is what is displayed when the lightbox is closed.
</amp-ad-banner>
<amp-lightbox>
This is the lightbox content
</amp-lightbox>
</body>
Would the friends not be "friendly frames" if it's A4A inside an AMP doc? @lannka? For the inabox use case we would need to put the iframe resizing logic somewhere, but that's unrelated to this FR.
The use case is for ads. Not sure in which other contexts we would like to anchor the position of an element in an iframe that expands. |
Obsolete. We went with the body element solution: #10699 |
/cc @lannka
Background
We want to support lightboxes in AMP ads (#7015). The lightbox element must be sandboxed as part of the ad frame, but should visually cover the entire viewport when opened. In order to do this, the iframe containing the ad must be set to
position: absolute
and cover the entire viewport before the lightbox is opened. If we resize the frame as-is, the following problems occur:The
body
tag could be styled as part of the creative. This means that if we change the size, background, padding or other properties, the creative will look broken.The content of the creative will shift in position in the page.
The lightbox component has an opacity transition, which causes the intermediate state to be visible.
Illustration of problem
Proposal
An intermediate element that contains the visual components of the creative banner must be inserted in the ad's root. This element will be repositioned when in lightbox mode so that the creative won't shift visually.
Desired result
The iframe manager could potentially create the intermediate element when entering lightbox mode, but this is finicky since it would have to copy a certain set of
<body>
children and could have unintended side effects for the author of the creative due to conflicts in style properties.A better solution is to introduce a
<amp-ad-banner>
element which would be responsible for delimiting the visual area of the banner.This element would be sized and positioned by the iframe manager when a lightbox is opened. The author would then freely style the
amp-ad-banner
element as the visual root of the ad, potentially applying the styling that would otherwise be added to thebody
tag.Details
amp-ad-banner
must be a dummy extension without much (if any) component logic.It's not necessary for the component to have logic besides resizing.
Layout must be set to
fill
This is so that it's clear that sibling elements won't be visible in the ad.
Validation rules must specify that only one
amp-ad-banner
element can appear in the document.Same as above.
Validation rules for
amp-ad-banner
must specify that onlyamp-lightbox
elements are valid siblings.Same as above.
Validation rules for
amp-lightbox
must specify that anamp-ad-banner
element must exist as its sibling.Otherwise the runtime would not be able to reposition the creative properly.
Open questions
The text was updated successfully, but these errors were encountered: