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

The <amp-iframe> 600px / 75% rule - based on what width? #1639

Closed
jpettitt opened this issue Jan 28, 2016 · 8 comments
Closed

The <amp-iframe> 600px / 75% rule - based on what width? #1639

jpettitt opened this issue Jan 28, 2016 · 8 comments

Comments

@jpettitt
Copy link
Contributor

The <amp-iframe> spec, and the validation code, reference the 600px/75% rule. However there is no description of what width that applies to. For example 600px down on a iPhone 4 is a completely different proposition to the same content on an iPad pro or a desktop. What width does the CDN use when validating? Do publishers have to guess based on the user agent what width they are dealing with and adjust content? Always use worst case?

@rudygalfi
Copy link
Contributor

I believe this is measured independently of width. In the context of a device, the restriction has different ramifications, but the enforcement will be the same across screen sizes.

/cc @cramforce

@dvoytenko
Copy link
Contributor

This restriction can be removed by adding a placeholder as described here: https://github.com/ampproject/amphtml/blob/a5b3a0f6153f71fbec7e5648bd46b79f6b4013e1/extensions/amp-iframe/amp-iframe.md

@jpettitt
Copy link
Contributor Author

Since you're asking for feedback I think the rule is pretty unusable as defined. What rule effectively says is "always use a placeholder or your code will fail randomly when some user has a screen wider than you anticipated". Perhaps just changing it to make placeholders mandatory would be simpler all round?

@cramforce
Copy link
Member

@jpettitt yeah, we just actually changed the error message to give clear advice how to fix it. Switching to no-rule-but-mandatory-placeholder is generally a good idea. Added to #1356

@mjangda
Copy link
Contributor

mjangda commented Jan 29, 2016

Since you're asking for feedback I think the rule is pretty unusable as defined. What rule effectively says is "always use a placeholder..."

That's what we ended up doing for the WordPress plugin. There isn't a super
easy, consistent, reliable way for us to determine element position when
generating the AMP files so we just include a placeholder for all iframes.

@cramforce
Copy link
Member

@mjangda What do you put into that placeholder?

@mjangda
Copy link
Contributor

mjangda commented Jan 29, 2016

What do you put into that placeholder?

Just a <div> with a background image via CSS. (We could get fancier but wanted to get something in place that validates.)

@rudygalfi
Copy link
Contributor

Closing since we've captured mandatory placeholder in #1356

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

5 participants