Amphtml: Add support for amp-embedly

Created on 19 Jan 2018  路  13Comments  路  Source: ampproject/amphtml

Create a component that uses's Embedly's API to embed any provider supported by Embedly.

Note: Embedly is a paid service and publisher needs to provide their own key.

API

<amp-embedly key=<yourAPIKey> url=<embedUrl> width height layout>

Implementation details:

1- We make a call to embedly's oembed API
2- It returns data including some html to render.
3- We add the html and watch for resize events and resize our components accordingly.

NOTE: we can NOT trust the html return by embedly and just inject it. Therefor this component as a whole needs to live in a 3P iframe similar to amp-gist or amp-twitter

/to @juanlizarazo
/cc @jpettitt @ericlindley-g

Feature Request

All 13 comments

@aghassemi I can start working on this. Would you please assign it to me?

10661 can apparently be closed.

@juanlizarazo Thanks! done.

@aghassemi @jpettitt @ericlindley-g

I created PR #13843 of what I have implemented so far.

As this is my first contribution to this project, I'd like to get some feedback if I am on the right path and on best amp-project practices in case I am hacking around things I shouldn't so I can make those changes early on.

Also, when it comes to writing integration tests and examples, what approach should we take as embedly's api requires a paid api key. I have one there for now so the examples can be rendered.

Thanks @juanlizarazo. Adding @alanorozco who will help you through the contribution and will review your PRs. We can also contact Embedly to get a test key for AMP examples later.

Thanks again for taking this on!

Thanks @alanorozco for the review. I addressed all comments except testing and component documentation/spec. Working my way through those now.

now the component api looks like this:

` <amp-embedly-key` value="someKey" layout="nodisplay"></amp-embedly-key> <amp-embedly data-url layout width height></amp-embedly> <!-- etc... as many <amp-embedly as desired ... -->

/cc @artgibson from Embedly. Art, we are making (PR an #13843) amp-embedly component in AMP. Please let us know if any concerns or feedback from your side.

Also, would it possible for AMP to get a test/developer API key to use in examples and samples on ampbyexample.com?

Thanks!

@aghassemi @alanorozco I need some guidance with resizing. How should I implement that?

These are some questions I have (none of the following is part of the PR yet):

When a resize should occur? (e.g. Is this just with the responsive layout?)

If I add a listenFor event, where does the message originates from?

I created an embedly.js file and I registered it in integration.js and I whitelisted 'embedly' in both the updateDimensionsEnabled_ line and the win.context.updateDimensions = triggerDimensions; block.
From there, I called context.updateDimensions but at that time the iframe is barely created and has no content as that comes from an api call. I tried making the api call from that script but the services are not registered at that time. So I I am thinking this might not be the right approach. Unless, there a way to use that context after the frame source is set? or from the component class itself?

How would we go about reading the actual content height?
Sometimes, embedly returns a width and height but for for rich type (like a twitter embed) the html content has different types of containers and there's no with or height in the response metadata, which makes it hard to read the height. And iframe.contentWindow.document.body.offsetHeight or similar inside the extension causes the

Error: Blocked a frame with origin "http://localhost:8000" from accessing a cross-origin frame.

I was thinking for this case, just stick with the height provided when the layout is not responsive. Or recommend fixed-height and recommend the user to measure the height of embed. I find it hard to handle it dynamically when there's some many providers and container names in the rich html we render.

What do you think would be best for this?


Other questions:

Should I create separate documentation for amp-embedly-key? For now I filled the template as part of the same extension documentation (just in the html example).

How would you like to handle the link resource type? for this, maybe we don't use an iframe and just create a <a> and set the src from the api response metadata?

Thanks! I know it's a lot of questions :smile:

@juanlizarazo what do you think about using Embedly's card API (available in their platform.js) to fully delegate rendering to them onside the 3P embedly.js's iframe.

From what I see here: http://docs.embed.ly/docs/cards#listeners after a Card is rendered, they give you an event that can be used to get the final width and height. Now we can ask their renderer to have width 100% so as soon as we get the card.rendered back, you can call the context.updateDimensions and set the final height.

It may actually make sense to call this component amp-embedly-card if we go with that approach.

@alanorozco @juanlizarazo thoughts?

@aghassemi Sounds great. I can get started on that tonight.

cc @alanorozco

Little update here!

This is the current status, I am aiming to finish the remaining tasks in the next few days.

I closed PR #13843 because the new amp-embedly-card component's implementation is quite different. I will create a new one once this is ready.

  • [x] Create and register extension.
  • [x] Do not trust html, use 3p iframe.
  • [x] Parse url data attributes.
  • [x] Load external script in 3p iframe
  • [x] Render card
  • [x] Listen for resize events
  • [x] amp-embedly-key component, when key provided, branding is removed.
  • [x] Allow customization options
  • [x] Update validator definition
  • [x] Documentation.
  • [x] Unit tests.

@aghassemi @alanorozco

@aghassemi @alanorozco New PR #14819 is up and ready for review :grin:

@alanorozco updated PR with new requested changes (comment in PR with details). Ready for review :D!

Closed by #14819

Was this page helpful?
0 / 5 - 0 ratings