Skip to content
This repository was archived by the owner on Feb 11, 2021. It is now read-only.

Added support for AMP HTML#188

Open
neildaniels wants to merge 7 commits intogetkirby:developfrom
thestreamable:nvd-amp-html
Open

Added support for AMP HTML#188
neildaniels wants to merge 7 commits intogetkirby:developfrom
thestreamable:nvd-amp-html

Conversation

@neildaniels
Copy link

@neildaniels neildaniels commented Dec 18, 2019 •
edited
Loading

Describe the PR

This pull request introduces AMP HTML support. Tags such as <img> and <iframe> are strictly forbidden in AMP, so this adds compatible versions of the image and video blocks. The HTML from other blocks should already be AMP compatible.

Usage

echo $page->text()->blocks()->ampHtml();
// or
echo $page->text()->blocks()->toAmpHtml();

Extending

If a block requires an AMP snippet variant, append an .amp extension to the snippet name. For example:

Kirby::plugin('yourname/custom-block', [
  'snippets' => [
    'editor/custom'     => __DIR__ . '/snippets/custom.php',
    'editor/custom.amp' => __DIR__ . '/snippets/custom.amp.php',
  ]
]);

Videos

If you intend on using YouTube or Vimeo videos, you'll need to include one/both of the following in your AMP's page <head>.

<script async custom-element="amp-youtube" src="https://cdn.ampproject.org/v0/amp-youtube-0.1.js"></script>
<script async custom-element="amp-vimeo" src="https://cdn.ampproject.org/v0/amp-vimeo-0.1.js"></script>

This can automatically provided by using the ampCustomElementScriptTags() method on a Blocks collection.

<head>
  <?= $page->text()->blocks()->ampCustomElementScriptTags() ?>
</head>

Kirbytext

Simple usage of a Kirbytext block will likely generate AMP-valid code, but some Kirbytags (such as (image: ), (video: ), and some custom tags) will generate incompatible AMP code. You should avoid using these kind of Kirbytags if you want valid AMP code.

Ready?

  • Added unit tests for fixed bug/feature
  • Passing all unit tests
  • Fixed code style issues with CS fixer and composer fix
  • Added in-code documentation (if needed)

@bastianallgeier
Copy link
Member

bastianallgeier commented Feb 11, 2020

I really love this (despite hating AMP) but it would be awesome if there could be a more abstract version that is able to convert the blocks into multiple formats. A bit like content representations but for the editor. What do you think?

@neildaniels
Copy link
Author

neildaniels commented Feb 11, 2020

I totally agree. It would be nice to have a more coherent way to expand this to other formats.

I'm happy to to revise this to a more generic pattern (unless you had something in mind), but I suspect the ultimate product will hinge on "Block Plugins" #199. Editor would provide the foundation for "content representations," but a plugin would provide the actual AMP-specific content. Or, did you also want AMP support to be built-in to Editor?

For a Block content representation system, I think it should be flexible enough that:

  • Block output is not necessarily be a string rendering. It should be able to vend array or full-fledged Objects for each Block.
    • For example, at some point I want to add Apple News Format support, which is a JSON file. I would expect that each Block would return an array and then the collection of all those arrays would actually be serialized as JSON at the end.
  • There should be support for "related data." (This could probably be more defined; or make no assumptions and let representation plugins decide how to use.)
    • In the AMP example, this is used for providing what <script> imports to include in the <head>.

@DanielRivers
Copy link

DanielRivers commented Oct 6, 2020 •
edited
Loading

I would LOVE this to be accepted, as much as AMP is not loved my all, its pretty much required for my type of site, hurts not having it amp compatible.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@neildaniels @bastianallgeier @DanielRivers