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

Allow style[i-amp-layout-style] for transformed AMP to contain rules extracted from attributes for media, sizes, and heights #27468

Closed
westonruter opened this issue Mar 28, 2020 · 4 comments

Comments

@westonruter
Copy link
Member

westonruter commented Mar 28, 2020

Currently AMP SSR is blocked when an AMP page contains AMP components that make use of the attributes sizes, heights, or media. While the AMP Optimizer does not currently implement a transformer for this, there is a transformation which can be made as noted in Future Work of Server-side rendering for AMP caches.

Given the AMP markup:

<amp-img width="150" height="150" src="https://example.com/foo.jpg" layout="fixed" sizes="(min-width: 320px) 320px, 100vw"></amp-img>

The sizes attribute needs to be added to CSS style rule for SSR to be able to remove the AMP boilerplate. Thus it should be transformed as:

<style i-amphtml-layout-style>
#a1 {
  width: 100vw;
}
@media (min-width: 320px) {
  #a1 {
    width: 320px;
  }
}
</style>
<!-- ... -->
<amp-img width="150" height="150" src="https://example.com/foo.jpg" layout="fixed" layout="fixed" id="a1" class="i-amphtml-layout-fixed i-amphtml-layout-size-defined" style="width:150px;height:150px;" i-amphtml-layout="fixed"></amp-img>

Similar transformations would need to be done for media and heights attributes. The a1 element ID is an auto-generated unique ID.

The style rules appearing in this style[i-amp-layout-style] would need to be heavily restricted.

While the styles appearing in style[i-amp-layout-style] could just be added to style[amp-custom], a risk here is that it could cause the total CSS to go over the 75KB limit. Therefore, it would be safer if there was a new style element dedicated for the purpose of containing the CSS rules extracted for transformed AMP.

The style[i-amp-layout-style] element should be added to the head, presumably after style[amp-runtime] but before style[amp-custom].

See also ampproject/amp-wp#4439 a transformer is being developed to do the above, though it will begin by amending the style rules to style[amp-custom] as long as the new rules do not overrun the CSS limit.

@westonruter
Copy link
Member Author

cc @ampproject/wg-caching @sebastianbenz

@westonruter westonruter changed the title Introduce style[i-amp-layout-style] for transformed AMP to contain rules extracted from attributes for media, sizes, and heights Allow style[i-amp-layout-style] for transformed AMP to contain rules extracted from attributes for media, sizes, and heights Mar 28, 2020
@westonruter
Copy link
Member Author

As opposed to adding a single new style[i-amphtml-layout-style] in the head like so:

<style i-amphtml-layout-style>
#a1 {
  width: 100vw;
}
@media (min-width: 320px) {
  #a1 {
    width: 320px;
  }
}
</style>
<!-- ... -->
<amp-img width="150" height="150" src="https://example.com/foo.jpg" layout="fixed" layout="fixed" id="a1" class="i-amphtml-layout-fixed i-amphtml-layout-size-defined" style="width:150px;height:150px;" i-amphtml-layout="fixed">
   <!-- ... -->
</amp-img>

What if instead this style element were added as an SSR'ed child of the AMP element? So like:

<amp-img width="150" height="150" src="https://example.com/foo.jpg" layout="fixed" layout="fixed" id="a1" class="i-amphtml-layout-fixed i-amphtml-layout-size-defined" style="width:150px;height:150px;" i-amphtml-layout="fixed">
    <style i-amphtml-layout-style>
        #a1 {
            width: 100vw;
        }
        @media (min-width: 320px) {
            #a1 {
                width: 320px;
            }
        }
    </style>
    <!-- ... -->
</amp-img>

That could perhaps be easier to validate, and it would reduce the amount of CSS in the head.

@westonruter
Copy link
Member Author

I'm closing this as obsolete since transformed AMP pages now allow 112.5KB of CSS per #32004. So this should no longer be needed.

@westonruter
Copy link
Member Author

Optimizers that create transformed AMP pages can just amend the CSS rules to style[amp-custom].

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

3 participants