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

Inconsistency in placement of overflow button when SSR prepends sizer element #1155

Open
westonruter opened this issue Mar 4, 2021 · 5 comments

Comments

@westonruter
Copy link
Member

In ampproject/amp-wp#5926 I noticed that the button[overflow] element was being hidden due to being positioned outside of the parent element:

Screen Shot 2021-02-25 at 21 31 17

On non-SSR'ed pages, the preview button appeared at the top inside of the element.

The issue is that on SSR'ed pages, a responsive amp-iframe will get:

<amp-iframe src="https://embed.tumblr.com/embed/post/2JT2XTaiTxO08wh21dqQrw/92003045635" layout="responsive" width="540" height="280" resizable sandbox="allow-scripts allow-popups allow-same-origin" class="i-amphtml-layout-responsive i-amphtml-layout-size-defined" i-amphtml-layout="responsive">
    <i-amphtml-sizer style="display:block;padding-top:51.8519%"></i-amphtml-sizer>
    <button overflow>See more</button>
    <a href="https://ifpaintingscouldtext.tumblr.com/post/92003045635/grant-wood-american-gothic-1930" placeholder>https://ifpaintingscouldtext.tumblr.com/post/92003045635/grant-wood-american-gothic-1930</a>
</amp-iframe>

Whereas on the non-SSR'ed page, this is absent:

<amp-iframe src="https://embed.tumblr.com/embed/post/2JT2XTaiTxO08wh21dqQrw/92003045635" layout="responsive" width="540" height="280" resizable="" sandbox="allow-scripts allow-popups allow-same-origin">
    <button overflow="">See more</button>
    <a href="https://ifpaintingscouldtext.tumblr.com/post/92003045635/grant-wood-american-gothic-1930" placeholder="">https://ifpaintingscouldtext.tumblr.com/post/92003045635/grant-wood-american-gothic-1930</a>
</amp-iframe>

The i-amphtml-sizer is pushing the computed top for the button to be the same as the height of the sizer, which causes it to move out of view.

We worked around the issue by adding this style rule to consistently place the absolutely-positioned button at the bottom of the container:

button[overflow] {
    bottom: 0;
}

Nevertheless, I think SSR should consider appending the i-amphtml-sizer element rather than prepending in order to ensure consistency when SSR'ed.

Playground examples:

Without SSR With SSR
image image
@sebastianbenz
Copy link
Collaborator

Good one! Have you also checked how the AMP Cache handles this?

@westonruter
Copy link
Member Author

@sebastianbenz
Copy link
Collaborator

sebastianbenz commented Mar 4, 2021 via email

@westonruter
Copy link
Member Author

westonruter commented Mar 4, 2021

Done: b/181895985

Interestingly, I'm also now seeing the AMP runtime prepending the i-amphtml-sizer on the non-SSR'ed page… sometimes(?). Maybe I was seeing an experiment with aspect-ratio which bypassed the runtime from adding i-amphtml-sizer and in that case the overflow button was positioned within view.

@westonruter
Copy link
Member Author

westonruter commented Mar 4, 2021

Maybe I was seeing an experiment with aspect-ratio which bypassed the runtime from adding i-amphtml-sizer and in that case the overflow button was positioned within view.

Seems I was seeing layout-aspect-ratio-css which is being unlaunched: ampproject/amphtml#33050.

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

2 participants