Add amp-wistia-player in order to load Wistia videos by chen-anders · Pull Request #12654 · ampproject/amphtml

chen-anders · 2018-01-03T22:00:42Z

Closes #12644 .

googlebot · 2018-01-03T22:00:44Z

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 on your commit. Check your existing CLA data and verify that your email is set on your git commits.
If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot. The email used to register you as an authorized contributor must be the email used for the Git commit.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

honeybadgerdontcare · 2018-01-03T22:03:12Z

extensions/amp-wistia-player/validator-amp-wistia-player.protoascii

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


replace 2017 with 2018

honeybadgerdontcare · 2018-01-03T22:03:40Z

validation changes look good.

chen-anders · 2018-01-03T22:21:08Z

Signed the CLA.

googlebot · 2018-01-03T22:21:10Z

CLAs look good, thanks!

cathyxz · 2018-01-19T06:35:18Z

extensions/amp-wistia-player/0.1/amp-wistia-player.js

+        this.element.dispatchCustomEvent(VideoEvents.ENDED);
+      }
+    } else if (data['method'] == 'mutechange') {
+      isMuted = (data['args'] ? data['args']['_isMuted'] : undefined)


const isMuted = ?

aghassemi

Looks very good, just couple of small requests. Thanks for the PR!

aghassemi · 2018-01-20T01:06:00Z

examples/wistiaplayer.amp.html

+    <script async src="https://cdn.ampproject.org/v0.js"></script>
+  </head>
+  <body>
+    <amp-wistia-player


needs a layout

added here: 5838fe1

aghassemi · 2018-01-20T01:07:41Z

extensions/amp-wistia-player/0.1/amp-wistia-player.js

+            'for <amp-wistia-player> %s',
+        this.element);
+    const iframe = this.element.ownerDocument.createElement('iframe');
+    iframe.setAttribute('title', 'Wistia Video Player');


page author should control this. Maybe

iframe.setAttribute('title', this.element.getAttribute('title') || 'Wistia Video Player');

aghassemi · 2018-01-20T01:08:25Z

extensions/amp-wistia-player/0.1/amp-wistia-player.js

+    iframe.setAttribute('scrolling', 'no');
+    iframe.setAttribute('allowtransparency', '');
+    iframe.setAttribute('allowfullscreen', 'true');
+    iframe.src = '//fast.wistia.net/embed/iframe/' + encodeURIComponent(


no relative protocol, must be https://

aghassemi · 2018-01-22T20:09:12Z

@chen-anders To fix the Travis CI failure, you players needs to be added to the whitelist file here: https://github.com/ampproject/amphtml/blob/master/build-system/dep-check-config.js#L175

chen-anders · 2018-01-23T16:12:33Z

CI failures have been resolved.

chen-anders · 2018-01-23T20:27:22Z

Is there some additional work that needs to be done in order to be listed on the following page: https://www.ampproject.org/docs/reference/components#media ?

aghassemi · 2018-01-23T22:10:00Z

@chen-anders We periodically update that page. You can also submit a PR against this file: https://github.com/ampproject/docs/blob/e7696149ce0c231525e07a8ea2a099e1a3a23a99/content/docs/reference/components.md to get it there faster.

* Add an amp-video test that shows which tagspec is picked for errors. * Revision bump for #12654 * Revision bump for #12836 * Make sure the light validator doesn't run amp4ads tests.

* Add an amp-video test that shows which tagspec is picked for errors. * Revision bump for ampproject#12654 * Revision bump for ampproject#12836 * Make sure the light validator doesn't run amp4ads tests.

googlebot added the cla: no label Jan 3, 2018

honeybadgerdontcare reviewed Jan 3, 2018

View reviewed changes

googlebot added cla: yes and removed cla: no labels Jan 3, 2018

chen-anders force-pushed the amp-wistia-player branch from d40135a to dc351f3 Compare January 5, 2018 16:09

aghassemi requested review from aghassemi and cathyxz January 16, 2018 03:29

cathyxz reviewed Jan 19, 2018

View reviewed changes

chen-anders force-pushed the amp-wistia-player branch from 51a6cd8 to b5dc127 Compare January 19, 2018 14:30

aghassemi reviewed Jan 20, 2018

View reviewed changes

chen-anders added 10 commits January 22, 2018 09:18

gulpfile.js - Add amp-wistia-player

687a6e0

Add AMP support for Wistia player

d26b60e

Add amp-wistia-player example page

7bc67d2

Fix lint errors

658fe35

Listen to and handle events from player

f54f6c8

Prefix state/isMuted with const in handleWistiaMessages

97533b3

Fallback to "Wistia Video Player" title attribute if it does not exist

59e1da8

Use https for iframe.src

223f894

Add layout=fixed into Wistia Player example

5838fe1

chen-anders force-pushed the amp-wistia-player branch from b5dc127 to d0b8ae2 Compare January 22, 2018 17:18

Make message handling consistent with non-AMP iframes

9e601e4

chen-anders force-pushed the amp-wistia-player branch from d0b8ae2 to 9e601e4 Compare January 22, 2018 17:20

Fix lint errors

cee6a24

Add amp-wistia-player to whitelist in dep-check-config

5cdb67e

Update amp-wistia-player test for iframe.src being HTTPS only

b6f2ba7

aghassemi approved these changes Jan 23, 2018

View reviewed changes

aghassemi merged commit 8b701cf into ampproject:master Jan 23, 2018

Gregable pushed a commit that referenced this pull request Jan 24, 2018

Revision bump for #12654

c180307

Gregable added a commit that referenced this pull request Jan 24, 2018

Validator Rollup (#12999)

6e55c75

* Add an amp-video test that shows which tagspec is picked for errors. * Revision bump for #12654 * Revision bump for #12836 * Make sure the light validator doesn't run amp4ads tests.

gzgogo pushed a commit to gzgogo/amphtml that referenced this pull request Jan 26, 2018

Add amp-wistia-player in order to load Wistia videos (ampproject#12654)

65cfa68

ampprojectbot added the PR use: In Stable label Jan 31, 2018

RanAbram pushed a commit to RanAbram/amphtml that referenced this pull request Mar 12, 2018

Add amp-wistia-player in order to load Wistia videos (ampproject#12654)

53e82dc

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

Conversation

chen-anders commented Jan 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

googlebot commented Jan 3, 2018

Uh oh!

honeybadgerdontcare Jan 3, 2018

Choose a reason for hiding this comment

Uh oh!

honeybadgerdontcare commented Jan 3, 2018

Uh oh!

chen-anders commented Jan 3, 2018

Uh oh!

googlebot commented Jan 3, 2018

Uh oh!

cathyxz Jan 19, 2018

Choose a reason for hiding this comment

Uh oh!

aghassemi left a comment

Choose a reason for hiding this comment

Uh oh!

aghassemi Jan 20, 2018

Choose a reason for hiding this comment

Uh oh!

chen-anders Jan 22, 2018

Choose a reason for hiding this comment

Uh oh!

aghassemi Jan 20, 2018

Choose a reason for hiding this comment

Uh oh!

aghassemi Jan 20, 2018

Choose a reason for hiding this comment

Uh oh!

aghassemi commented Jan 22, 2018

Uh oh!

chen-anders commented Jan 23, 2018

Uh oh!

chen-anders commented Jan 23, 2018

Uh oh!

aghassemi commented Jan 23, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

chen-anders commented Jan 3, 2018 •

edited

Loading