Amphtml: Introduce resize message adapters for `amp-iframe` by implementing one for Storify

Created on 27 Jul 2017  Â·  18Comments  Â·  Source: ampproject/amphtml

Motivation

Following the discussion in #10467 and #9838 , we would like to introduce a list of supported embed URLs for which we automatically read the iframe's resize postMessage and interpret it then resize our iframe's height accordingly.

As determined in #10467 , unfortunately most of the popular iframe embeds don't send any resize message of any sort. However, Storify is one of the most requested embeds and it sends a resize message in this format:

 MessageEvent {isTrusted: true, data: "
 {
 "method":"resize",
 "value":6809,
 "sourceName":"sacb…-relievers-sean-doolittle-ryan-madson-traded-to"}", 
 origin: "http://storify.com", 
 lastEventId: "", 
 source: Window…
 }

For this GFI, you will be implementing an adapter that reads the message and updates the iframe's height accordingly.

The issue

amp-iframe currently requires a specific resize message to be able to resize itself per the embed's request. The resize postMessage has to be in this format:

window.parent.postMessage({
  sentinel: 'amp',
  type: 'embed-size',
  height: document.body.scrollHeight
}, '*');

Of course, most websites won't support this format unless they have an AMP-specific implementation.

Implementation Strategy

  • [ ] Implement a postMessage listener in amp-iframe
  • [ ] Implement a method that given a URL, listens for the appropriate resize message (determined through running regex on the URL) and update's the iframe's size accordingly using this.updateSize_()
  • [ ] Make the method work with Storify's embed URLs (determine the appropriate regex and the appropriate fields to read from the postMessage)
  • [ ] Unit test the changes

Step by step

  • [ ] Claim this issue by adding a comment below. Please only claim this bug if you plan on starting work in the next day or so. (If you join the AMP Project we'll be able to assign this issue to you after you've claimed it.)
  • [ ] If you aren't too familiar with Git/GitHub, see the Getting Started End-to-End Guide for an intro to Git & GitHub, and how to get a copy of the code. You can also refer to the Quick Start Guide for the necessary setup steps with less explanation than the End-to-End guide.
  • [ ] Follow the instructions for building AMP.
  • [ ] [Create a Git branch](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#create-a-git-branch) for making your changes.
  • [ ] [Sign the Contributor License Agreement](https://github.com/ampproject/amphtml/blob/master/CONTRIBUTING.md#contributor-license-agreement) before creating a Pull Request. (If you are contributing code on behalf of a corporation start this process as early as possible.)
  • [ ] Get familiar with the life of an AMP element by reading the comments in src/base-element.js
  • [ ] Follow the implementation strategy tasks
  • [ ] [Commit your changes](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#edit-files-and-commit-them) frequently.
  • [ ] [Push your changes to GitHub](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#push-your-changes-to-your-github-fork).
  • [ ] [Create a Pull Request](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#send-a-pull-request-ie-request-a-code-review). Mention Closes Issue #10661 in the description and request a review from @wassgha.
  • [ ] [Respond to your reviewer's comments](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#respond-to-pull-request-comments) (if any).

Once approved, your changes will be merged. ⚡⚡⚡Congrats on making your first contribution to the AMP Project!⚡⚡⚡ You'll be able to see it live across the web soon!

Thanks, and we hope to see more contributions from you soon.

Questions?


If you have questions ask in this issue or on your Pull Request (if you've created one) or see the How to get help section of the Getting Started guide.

When Possible Feature Request

Most helpful comment

hey, i'd like to claim this issue, it'll be my first OSS contribution even though i've been working as a front-end developer for years.

All 18 comments

@aghassemi @mrjoro Does this qualify as a GFI or is it too much work?

I think it's fine for to have GFIs that cover a range of backgrounds--e.g. some people may be very advanced web developers who haven't contributed to open source before, so they use GFIs as a way to do that. Check out the docs for creating a GFI and make sure to use the template.
You can fill in the "What you need to know" section with background knowledge someone should have when tackling the GFI, and break down the tasks into work that people who have that background knowledge but who haven't worked on AMP before will be able to tackle (with pointers if needed).

@wassgha If we already had one adapter, this would be a great First Timer GFI. (idea for subsequent GFIs)
Still fine as a GFI, maybe just add a "Implementation Strategy" section and include a bit more details (and move the existing implementation-related bullet points from the Step by step into the "Implementation Strategy" and just add a "Follow the implementation strategy" bullet in the Step by step section)

hey, i'd like to claim this issue, it'll be my first OSS contribution even though i've been working as a front-end developer for years.

Great! @kboluk I sent you an invite to join ampproject; once you accept we can assign this issue to you.

Hey @kboluk is there any progress on this issue?

Since there haven't been updates in a while we'll make this available for anyone who is interested in working on a good first issue.

@wassgha @mrjoro I would like to claim this issue as i reviewed it and i feel i can do it.

Great ! @mrjoro will invite you :)

@wassgha @FadySamirSadek is there any updates on this? If not, I'd like to claim it.

Update:

Just visiting storify, they announced their end-of-life on December 12th.

(...) Unfortunately, we are no longer allowing new users to sign up and will end existing users' access to Storify.com on May 16, 2018.
Storify End-of-Life

Additionally, I see it will be a paid feature of Adobe's Livefyre, they call it storify 2.
Using google amp with storify 2

should we update the target embed to something else for this specific GFI?

@juanlizarazo I am stuck so I do not mind if you claim it but please keep me updated because I want to know how you did it

@aghassemi can you comment on @juanlizarazo's question when you get a chance?

After thinking about this a for a bit and looking at several embed types, I think we should pivot and create a amp-embedly component instead as it would cover more cases and is actually simpler as a GFI than this.

@juanlizarazo would you be interested in creating an <amp-embedly url=<whatever embed.ly supports> component for AMP?

@aghassemi definitively!

Awesome, please see https://github.com/ampproject/amphtml/issues/12908. It is a bit more involved than other good first issues in AMP, so depending on your comfort level you may want to start with something else first and then move on to it.

Thanks @aghassemi! I can get started on it this weekend.

@aghassemi it sounds like we aren't going to go forward with this issue now and we should close it?

Closing in favour of #12908

Was this page helpful?
0 / 5 - 0 ratings