Skip to content
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

Collapse empty ads in situation where iframe resizing is allowed #1089

Closed
cramforce opened this issue Dec 7, 2015 · 9 comments
Closed

Collapse empty ads in situation where iframe resizing is allowed #1089

cramforce opened this issue Dec 7, 2015 · 9 comments

Comments

@cramforce
Copy link
Member

If we are allowed to resize the iframe of the ad and there is no ad to serve, remove the ad from the page.

CC @kashyapnitin

@ds0001
Copy link

ds0001 commented Dec 11, 2015

👍

@cramforce cramforce assigned camelburrito and unassigned cramforce Dec 15, 2015
@camelburrito
Copy link
Contributor

Does this need animation?

@cramforce
Copy link
Member Author

No.
On Dec 29, 2015 12:02 PM, "Sriram Krishnan" [email protected]
wrote:

Does this need animation?


Reply to this email directly or view it on GitHub
#1089 (comment)
.

@camelburrito
Copy link
Contributor

@cramforce We only collapse if there is no placeholder right?

@cramforce
Copy link
Member Author

Right

@camelburrito
Copy link
Contributor

@cramforce we dont support resizing on amp-ad iframes yet, should I implement resizing support as well?

@cramforce
Copy link
Member Author

Yes, but sync with Dima. He might have a pending CL. Doing this special
case is a good first step, though.
On Dec 29, 2015 1:23 PM, "Sriram Krishnan" [email protected] wrote:

@cramforce https://github.com/cramforce we dont support resizing on
amp-ad iframes yet, should I implement resizing support as well?


Reply to this email directly or view it on GitHub
#1089 (comment)
.

@camelburrito
Copy link
Contributor

adding @dvoytenko to the conversation

@dvoytenko
Copy link
Contributor

Yeah, let's roll it in this PR. Most of the implementation is already done in the custom-element.js - which just need to trigger it.

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

No branches or pull requests

4 participants