amp-social-share causes layout shift on upgrade

[kevinkimball]

What's the issue?

When .i-amphtml-unresolved class is removed on component upgrade, overflow is changed from hidden to visible, which causes subsequent elements to shift (both sibling inline element and following image).

This issue was surfaced by this issue from the Page Experience tool (ampproject/wg-performance#48).

This may be related to #27228 but data-mode=replace is not used in the example.

How do we reproduce the issue?

Visit the URL below with "Layout Shift Regions" enabled in Rendering tab of Chrome Dev Tools to visualize the layout shift.

https://tasty.co/amp/recipe/garlic-bread-bruschetta

What browsers are affected?

Chrome 87

Which AMP version is affected?

2009252320001

[nainar]

This is a high priority if this is causing CLS issues - it is a highly used AMP component.

@zhouyx to assign and @caroqliu for context.

[rbeckthomas]

On potential solution for this would be to permanently add the overflow: hidden; style to this component.
However this change would be backwards incompatible and would require a new revision of the social-share component.

@alanorozco, @kristoferbaxter, @krdwan Do you have any thoughts the correct path forward?

[caroqliu]

Discussed offline with @rebeccanthomas:

The "print" button appears to have display: inline-block which aligns itself to the baseline of the amp-social-share icons. It looks like applying overflow: hidden; vertical-align: top; to the amp-social-share selector will fix it for this particular document, not sure about others. Alternatively, the developers for could applyvertical-align: top; to their Print button and fix that part of the page's CLS without us making any changes to amp-social-share.

Do we have other sample documents @kevinkimball?

Short term recommendations:

• Apply overflow: hidden; vertical-align: top; on the repro case.
• Investigate if the overflow: hidden from .i-amphtml-unresolved can be overridden to be overflow: visible for amp-social-share.i-amphtml-unresolved in the ampshared.css as @rebeccanthomas mentioned above.

Long term recommendation:

• This further strengthens the case for a no-CLS v2 redesign. :)

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.