Add Facebook like button support: amp-facebook-like #8989

eduardogoncalves · 2017-04-27T15:28:04Z

Since AMP already loads facebook sdk, why not have an like-button extension?

googlebot · 2017-04-27T15:28:08Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please let us know the company's name.

eduardogoncalves · 2017-04-27T15:33:18Z

I signed it!

googlebot · 2017-04-27T15:33:20Z

CLAs look good, thanks!

jridgewell · 2017-04-28T00:01:39Z

extensions/amp-facebook-like/0.1/amp-facebook-like.js

+    this.applyFillContent(iframe);
+    // Triggered by context.updateDimensions() inside the iframe.
+    listenFor(iframe, 'embed-size', data => {
+      this./*OK*/changeHeight(data.height);


#attemptChangeHeight, please.

@jridgewell, sorry, I didn't undestand.

#attemptChangeHeight, please.

Change changeHeight to attemptChangeHeight, which won't cause a reflow of content.

this.attemptChangeHeight(data.height).catch(() => { /* ignore failures */ });

jridgewell · 2017-04-28T00:02:15Z

extensions/amp-facebook-like/0.1/validator-amp-facebook-like.protoascii

@@ -0,0 +1,50 @@
+#


/cc @honeybadgerdontcare

@jridgewell validation looks good, asked for some minor tweaks.

jridgewell · 2017-04-28T00:02:32Z

extensions/amp-facebook-like/amp-facebook-like.md

@@ -0,0 +1,94 @@
+<!---


/cc @bpaduch

jridgewell · 2017-04-28T00:02:53Z

gulpfile.js

@@ -75,6 +75,7 @@ declareExtension('amp-dynamic-css-classes', '0.1', false, 'NO_TYPE_CHECK');
 declareExtension('amp-experiment', '0.1', false, 'NO_TYPE_CHECK');
 declareExtension('amp-facebook', '0.1', false);
 declareExtension('amp-facebook-comments', '0.1', false);
+declareExtension('amp-facebook-like', '0.1', false, 'NO_TYPE_CHECK');


You should be able to remove the NO_CHECK_TYPE.

removed 'no_type_check' as

ghost · 2017-04-28T12:15:13Z

extensions/amp-facebook-like/amp-facebook-like.md

+<table>
+  <tr>
+    <td width="40%"><strong>Description</strong></td>
+    <td>Embeds the Facebook like-button plugin.</td>


remove the hyphen

ghost · 2017-04-28T12:16:04Z

extensions/amp-facebook-like/amp-facebook-like.md

+The verb to display on the button. Can be either like or recommend.
+
+**data-colorscheme** (optional)
+The color scheme used by the plugin for any text outside of the button itself. Can be light or dark.


Please insert a blank line in between the attribute and description for all attributes that don't have a blank line in between.

ghost · 2017-04-28T12:17:52Z

extensions/amp-facebook-like/amp-facebook-like.md

+Specifies whether to display profile photos below the button (standard layout only). You must not enable this on child-directed sites.
+
+**data-size** (optional)
+The button is offered in 2 sizes i.e. large and small.


Rewrite as:

The size of the button, which can be one of two sizes, large or small. The default is small.

ghost · 2017-04-28T12:18:55Z

extensions/amp-facebook-like/amp-facebook-like.md

+
+**data-action** (optional)
+
+The verb to display on the button. Can be either like or recommend.


Please put the values in code style: like or recommend.

ghost · 2017-04-28T12:19:12Z

extensions/amp-facebook-like/amp-facebook-like.md

+The verb to display on the button. Can be either like or recommend.
+
+**data-colorscheme** (optional)
+The color scheme used by the plugin for any text outside of the button itself. Can be light or dark.


Please put the values in code style: light or dark

ghost · 2017-04-28T12:19:27Z

extensions/amp-facebook-like/amp-facebook-like.md

+If your web site or online service, or a portion of your service, is directed to children under 13 you must enable this
+
+**data-layout** (optional)
+Selects one of the different layouts that are available for the plugin. Can be one of standard, button_count, button or box_count.


Please put values in code style.

Applied markdown tags, removed `data-` elements with default values from __Example__

ghost

Docs look good.

honeybadgerdontcare

Thank you for doing a great job with the validation!

honeybadgerdontcare · 2017-04-28T15:44:27Z

extensions/amp-facebook-like/0.1/validator-amp-facebook-like.protoascii

+    mandatory: true
+  }
+  attr_lists: "extended-amp-global"
+  spec_url: "https://www.ampproject.org/docs/reference/components/social/amp-facebook-like"


The spec_url is changing again and it's no longer necessary to specify a sub-component. So let's remove social from the URL. It will be this instead:

spec_url: "https://www.ampproject.org/docs/reference/components/amp-facebook-like"

honeybadgerdontcare · 2017-04-28T15:44:42Z

extensions/amp-facebook-like/0.1/validator-amp-facebook-like.protoascii

@@ -0,0 +1,50 @@
+#
+# Copyright 2016 The AMP HTML Authors. All Rights Reserved.


Nit s/2016/2017.

honeybadgerdontcare · 2017-04-28T15:47:44Z

extensions/amp-facebook-like/0.1/validator-amp-facebook-like.protoascii

+  # data-* is generally allowed, but it's not generally mandatory.
+  attrs: {
+    name: "data-href"
+    mandatory: true


Since this is an href let's enforce some restrictions on it's values.

value_url: { allowed_protocol: "https" allow_relative: false }

honeybadgerdontcare · 2017-04-28T15:49:11Z

extensions/amp-facebook-like/0.1/validator-amp-facebook-like.protoascii

@@ -0,0 +1,50 @@
+#


@jridgewell validation looks good, asked for some minor tweaks.

aghassemi · 2017-04-28T15:43:03Z

extensions/amp-facebook-like/0.1/amp-facebook-like.js

@@ -0,0 +1,73 @@
+/**
+ * Copyright 2015 The AMP HTML Authors. All Rights Reserved.


2017 ( and in other files as well)

aghassemi · 2017-04-28T15:48:24Z

extensions/amp-facebook-like/0.1/validator-amp-facebook-like.protoascii

+tags: {  # <amp-facebook-like>
+  html_format: AMP
+  tag_name: "AMP-FACEBOOK-LIKE"
+  disallowed_ancestor: "AMP-SIDEBAR"


Don't see a need to disallow this in sidebar. Actually sidebar is a good place for a like button. @camelburrito ?

aghassemi · 2017-04-28T15:50:17Z

extensions/amp-facebook-like/amp-facebook-like.md

+  </tr>
+  <tr>
+    <td width="40%"><strong>Availability</strong></td>
+    <td>Stable</td>


Update to "In Development" and change to "Stable" 3 weeks after PR is merged (when validator changes are in production)

aghassemi · 2017-04-28T15:56:23Z

extensions/amp-facebook-like/0.1/test/validator-amp-facebook-like.html

+  <script async custom-element="amp-facebook-like" src="https://cdn.ampproject.org/v0/amp-facebook-like-0.1.js"></script>
+</head>
+<body>
+    <amp-facebook-like width=120 height=30


Please also add sample usage to examples/facebook.amp.html

Change changeHeight to attemptChangeHeight (which won't cause a reflow of content) as request by jridgewell

aghassemi · 2017-04-28T22:35:04Z

@eduardogoncalves Thanks for contributing!

aghassemi · 2017-04-28T22:35:35Z

/cc @sebastianbenz new amp-facebook-like component

eduardogoncalves · 2017-04-28T23:24:59Z

Ty I'm pleased to help.

eduardogoncalves · 2017-05-08T02:07:13Z

Hi, how does it works now? I tried to run facebook.amp.html example, but it says: Failed to load resource: the server responded with a status of 404 (): https://cdn.ampproject.org/v0/amp-facebook-like-0.1.js

honeybadgerdontcare · 2017-05-08T04:03:26Z

@eduardogoncalves It looks like your component is still in pre-release. Also it takes two to three week for validation changes to make it into production. So even if the component was released, it wouldn't be valid yet. Hopefully in the coming week or two it'll be ready for use in production.

eduardogoncalves added 2 commits April 27, 2017 10:38

add like-button support

9d2ed2a

add facebook like-button support

c285530

eduardogoncalves added 5 commits April 27, 2017 11:38

try fix "error Line 85 exceeds the maximum line length of 80 max-len"

5afd20c

exceeds the maximum line length of 80 max-len

67d448f

Update gulpfile.js

0f6ab3c

set declareExtension amp-facebook-like argument true to false

856003a

change data-kid_directed_site to data-kd_site

5608a1e

jridgewell approved these changes Apr 28, 2017

View reviewed changes

jridgewell requested review from a user, honeybadgerdontcare and aghassemi April 28, 2017 00:03

jridgewell added the Category: User Input label Apr 28, 2017

Update gulpfile.js

8fcf0ca

removed 'no_type_check' as

ghost suggested changes Apr 28, 2017

View reviewed changes

Update amp-facebook-like.md

2aff73b

Applied markdown tags, removed `data-` elements with default values from __Example__

ghost approved these changes Apr 28, 2017

View reviewed changes

honeybadgerdontcare requested changes Apr 28, 2017

View reviewed changes

aghassemi suggested changes Apr 28, 2017

View reviewed changes

eduardogoncalves added 2 commits April 28, 2017 13:50

Update validator-amp-facebook-like.protoascii

f6cc99f

Update amp-facebook-like.md

a1d9009

honeybadgerdontcare approved these changes Apr 28, 2017

View reviewed changes

eduardogoncalves added 6 commits April 28, 2017 13:58

Update amp-facebook-like.js

ad008cd

Update validator-amp-facebook-like.html

93beb61

Update facebook.js

8924e53

Update amp-facebook-like.js

5e865d8

Change changeHeight to attemptChangeHeight (which won't cause a reflow of content) as request by jridgewell

fix error Line 56 exceeds the maximum line length of 80 max-len

dc62d89

Extension amp-facebook-like (#8989)

f2dc623

aghassemi approved these changes Apr 28, 2017

View reviewed changes

aghassemi merged commit 9a1bfaf into ampproject:master Apr 28, 2017

KenneyE pushed a commit to spotxchange/amphtml that referenced this pull request May 3, 2017

Add Facebook like button support: amp-facebook-like (ampproject#8989)

a7bc0a9

ampprojectbot added PR use: In Beta / Experimental PR use: In Stable labels May 11, 2017

eduardogoncalves deleted the patch-1 branch May 11, 2017 22:42

honeybadgerdontcare mentioned this pull request May 12, 2017

Pull in new components: amp-3q-player, amp-facebook-like, amp-ima-video & amp-timeago ampproject/amp.dev#483

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Facebook like button support: amp-facebook-like #8989

Add Facebook like button support: amp-facebook-like #8989

eduardogoncalves commented Apr 27, 2017

googlebot commented Apr 27, 2017

eduardogoncalves commented Apr 27, 2017

googlebot commented Apr 27, 2017

jridgewell Apr 28, 2017

eduardogoncalves Apr 28, 2017

jridgewell Apr 28, 2017

jridgewell Apr 28, 2017

honeybadgerdontcare Apr 28, 2017

jridgewell Apr 28, 2017

jridgewell Apr 28, 2017

eduardogoncalves Apr 28, 2017

ghost Apr 28, 2017

ghost Apr 28, 2017

ghost Apr 28, 2017

ghost Apr 28, 2017

ghost Apr 28, 2017

ghost Apr 28, 2017

ghost left a comment

honeybadgerdontcare left a comment

honeybadgerdontcare Apr 28, 2017

honeybadgerdontcare Apr 28, 2017

honeybadgerdontcare Apr 28, 2017

honeybadgerdontcare Apr 28, 2017

aghassemi Apr 28, 2017

aghassemi Apr 28, 2017

aghassemi Apr 28, 2017

aghassemi Apr 28, 2017

aghassemi commented Apr 28, 2017

aghassemi commented Apr 28, 2017

eduardogoncalves commented Apr 28, 2017

eduardogoncalves commented May 8, 2017

honeybadgerdontcare commented May 8, 2017


		data-action (optional)

		The verb to display on the button. Can be either like or recommend.

		@@ -0,0 +1,50 @@
		#
		# Copyright 2016 The AMP HTML Authors. All Rights Reserved.

		@@ -0,0 +1,73 @@
		/**
		* Copyright 2015 The AMP HTML Authors. All Rights Reserved.

Add Facebook like button support: amp-facebook-like #8989

Add Facebook like button support: amp-facebook-like #8989

Conversation

eduardogoncalves commented Apr 27, 2017

googlebot commented Apr 27, 2017

eduardogoncalves commented Apr 27, 2017

googlebot commented Apr 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

honeybadgerdontcare left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aghassemi commented Apr 28, 2017

aghassemi commented Apr 28, 2017

eduardogoncalves commented Apr 28, 2017

eduardogoncalves commented May 8, 2017

honeybadgerdontcare commented May 8, 2017