-
Notifications
You must be signed in to change notification settings - Fork 4.1k
I2I: Consolidated 1.0 amp-facebook #34969
Description
Summary
This I2I proposes consolidating amp-facebook:0.1, amp-facebook-comments:0.1, amp-facebook-like:0.1, and amp-facebook-page:0.1 into a single amp-facebook:1.0 component.
Design Document
API impact
The amp-facebook component currently supports a required data-embed-as attribute with value post|video|comment. This proposal involves allowing three more embed types: comments|like|page.
- <amp-facebook-comments>
+ <amp-facebook data-embed-as="comments">
- <amp-facebook-like>
+ <amp-facebook data-embed-as="like">
- <amp-facebook-page>
+ <amp-facebook data-embed-as="page">All other pre-existing attributes on standalone components will be supported in amp-facebook. Invalid combinations of their existing attributes will behave as follows:
- valid AMP, which is not different from the current behavior given that we allow
data-*attributes by default - noop if irrelevant to the embed type, which is also the status quo. Take for example
amp-facebook-page, which will follow the same code path asamp-facebook[data-embed-as=page]:Lines 145 to 160 in c194d57
function getPageContainer(global, data) { const container = createContainer(global, 'page', data.href); container.setAttribute('data-tabs', data.tabs); container.setAttribute('data-hide-cover', data.hideCover); container.setAttribute('data-show-facepile', data.showFacepile); container.setAttribute('data-hide-cta', data.hideCta); container.setAttribute('data-small-header', data.smallHeader); container.setAttribute('data-adapt-container-width', true); const c = global.document.getElementById('c'); // Note: The facebook embed allows a maximum width of 500px. // If the container's width exceeds that, the embed's width will // be clipped to 500px. container.setAttribute('data-width', c./*OK*/ offsetWidth); return container; }
The downside is that document authors will not be able to convert the import scripts for components from 0.1 to 1.0 as the only migration action required. If this is an issue, we can consider publishing valid amp-facebook-{comments,like,page} with 1.0 versions which upgrade to amp-facebook[data-embed-as]. These could also be transformed as such on the AMP cache.
Attribute matrix
The follow table outlines the attributes current supported by each component individually.
amp-facebook |
-comments |
-like |
-page |
|
|---|---|---|---|---|
data-href |
x | x | x | x |
data-locale |
x | x | x | x |
title |
x | x | x | x |
data-allowfullscreen |
x | |||
data-embed-as |
x | |||
data-include-common-parent |
x | |||
data-colorscheme |
x | x | ||
data-numposts |
x | |||
data-orderby |
x | |||
data-action |
x | |||
data-kd_site |
x | |||
data-layout |
x | |||
data-ref |
x | |||
data-share |
x | |||
data-size |
x | |||
data-hide-cover |
x | |||
data-hide-cta |
x | |||
data-show-facepile |
x | |||
data-small-header |
x | |||
data-tabs |
x |
Behavior impact
The primary behavior change is that all embed types will now attemptChangeHeight, which impacts amp-facebook and amp-facebook-comments -- the only two components that would prior forceChangeHeight and cause CLS regardless if it is pushing content down below it, potentially disrupting an end user's engagement with content. To be clear, amp-facebook-like and amp-facebook-page have always used attemptChangeHeight and would continue to do so as <amp-facebook data-embed-as="like"> and <amp-facebook data-embed-as="page">. Examples would be provided in all use cases of how to provide an overflow element to solicit user interaction for resizing.
amp-facebook will also fail if its data-embed-as attribute is not one of the six embed types, whereas prior it would restrict it to the existing three.
Motivation
Without this change, AMP developers are forced to import up to 4 components scripts which all: preconnect to the Facebook SDK, create an iframe, and load our third-party Facebook integration code. They could get the same feature set with only one such component with negligible code additions to ensure compatibility and significant removals of redundant logic.
For publishers who only use one of these components on their document, the difference would be that this configuration object would have 15 more entries:
amphtml/extensions/amp-facebook/1.0/base-element.js
Lines 22 to 31 in c194d57
| BaseElement['props'] = { | |
| ...FacebookBaseElement['props'], | |
| 'allowFullScreen': {attr: 'data-allowfullscreen'}, | |
| 'includeCommentParent': { | |
| attr: 'data-include-comment-parent', | |
| type: 'boolean', | |
| default: false, | |
| }, | |
| 'showText': {attr: 'data-show-text'}, | |
| }; |
Alternative Solutions
We can do nothing and launch the four components as-is. The same features are available regardless, but publishers would see some non-optimized performance when using more than one of these on a single document. Namely, this includes downloading near-identical extension binaries in all cases and also downloading near-identical pre-upgrade CSS for non-AMP (Bento) use cases.
Launch Tracker
No response
Notifications
/cc @ampproject/wg-approvers @ampproject/wg-components