Freecodecamp: AMP support for YouTube is not working/implemented?

[raisedadead]

AMP support for YouTube is not working/implemented in Ghost. Needs investigation.

Context as reported by @AbbeyRenn:

https://twitter.com/carlfwalsh/status/1272198642313515009

[raisedadead]

Yep, it seems like its a special tag: https://amp.dev/documentation/components/amp-youtube/

Ahmad, can you investigate changes that the handlebar templates would need.

[simarpreetsingh-019]

Maybe I could help in this and do my first good issue. Where to see the code and can you provide youtube video links to be added in the article?

[ahmadabdolsaheb]

Thanks @simarpreetsingh-019,
There is our theme repo. https://github.com/freeCodeCamp/news-theme

I highly recommend starting from here
https://ghost.org/docs/api/v3/handlebars-themes/google-amp/

and checking out the ghost forum for a solution.
https://forum.ghost.org/

[simarpreetsingh-019]

@ahmadabdolsaheb thanks for this. Could you also attach links of video that should be added to that page?

[ahmadabdolsaheb]

it could be any video. the idea is to have a template that inserts amp-youtube for imbedded youtube videos when viewed on amp.

[simarpreetsingh-019]

ohh , i didn't know much about working on for templates, as i have just started with AMP, but, will try my best to do something

[simarpreetsingh-019]

Pull request have been made , with some suggestions. Please review .

[scissorsneedfoodtoo]

Thanks for this contribution @simarpreetsingh-019.

This is working locally for be, though I had to update Ghost and restart to get the AMP version of the YouTube embed to render.

While digging more into this, it seems like the amp-youtube script controlled by Ghost in more recent versions: https://github.com/TryGhost/Ghost/pull/11058/files

Not sure if it's best to add it in the theme here, or to either add those lines to our Ghost install or wait until we update to a more recent version.

Since we're still running an older version of Ghost, we'll need to update /current/core/frontend/apps/amp/lib/helpers/amp_content.js directly. It seems like we just need to add to the 'amp-youtube' to the allowedAMPTags here: https://github.com/TryGhost/Ghost/blob/23bf432514c67ac7714cb51846f4ffc8353afd41/core/frontend/apps/amp/lib/helpers/amp_content.js#L25

Then update allowedAMPAttributes with 'amp-youtube': ['src', 'width', 'height', 'layout', 'frameborder', 'autoplay', 'loop', 'data-videoid', 'data-live-channelid'] like on this line: https://github.com/TryGhost/Ghost/blob/23bf432514c67ac7714cb51846f4ffc8353afd41/core/frontend/apps/amp/lib/helpers/amp_content.js#L114

[simarpreetsingh-019]

@scissorsneedfoodtoo That's great news that it worked, and thanks for referring to allowedAMPAttributes with amp-youtube. I was suggesting about that. Well if it's already implemented, I had to see other things for any other suggestion in which I could also help to contribute.
Just one doubt, will this contribution be added to repo? or I would have to wait for some more time with another issue to be merged as my first contribution to any org?

[scissorsneedfoodtoo]

@simarpreetsingh-019, thanks for your patience.

I don't think everything mentioned in my last post is implemented in our production Ghost instance, though I haven't been able to check on it directly.

Either way, someone on the team will need to implement the fix, either by updating our Ghost instance or adding those values to the /current/core/frontend/apps/amp/lib/helpers/amp_content.js file. Once we do that, I don't think we'll need to update the handlebars template, though we'll need to do more testing.

That said, there's still a lot of issues we need help with over on /news or here in the main repo. Feel free to look for other issues and submit PRs!