Add support for pages #188
Conversation
|
We'll want a custom template for Pages (e.g. no need to include the author/date/category/tags) and will need to adapt things like the JSON metadata so that it accurately reflects the content (e.g. |
|
Gotcha. Thanks for the clarification |
|
@mjangda, Updated the PR with modifications for the metadata. Note that I decided to go the route of checking for if the post type is hierarchical instead of simply checking for pages. Allows us to roll out CPT support as another issue, or at a later time. |
|
I think we could probably do something around a factory class for the templates and/or a base class that other template types can extend, but that's more out of scope for this issue. |
|
@mjangda, out of curiosity, what's your timeline for 0.4? |
Yeah, that's sort of what I was originally envisioning too.
Aiming for next week or two. |
|
Is there an issue for that? I can work on it. Already been tossing it around in my head how to do it extensibly and properly. Or I could just update the PR with new code that takes that into account for this new template. |
| * | ||
| * @since 0.2 | ||
| */ | ||
| $metadata = apply_filters( 'amp_metadata', array_merge( $metadata, $article_meta ), $this ); |
There was a problem hiding this comment.
We already have a filter further down, so this is unnecessary.
No issue for it, but would love to hear what you're thinking. If it'll majorly change the direction of this PR, then it probably makes sense to close this and just run with that. Overall, though, this PR looks pretty solid; thanks for tackling it! Left a few comments and questions. |
|
Yeah, I'll clean this up this week. My idea would change the template portion of the PR, but I don't think it should block the inclusion of this PR. That would just be a secondary PR that adjusts this. The thought is a class that has things like Thoughts? |
|
Going to do some more quick tests but I think this is ready to ship. |
|
I add support for tags, category, pagination, author, and home page (as archive). Also added some filters to change the URL of the elements when it is "within amp", for paging, tags, etc .. on this repo: https://github.com/gabrielperezs/amp-wp What do you think about? I ask because I made several changes I do not know if they go on line / plan. In my version, I still have clean code, repeat functions for AMP_Archive_Template, AMP_Post_Template and AMP_Common_Template. But it is ready to be tested. Diff https://github.com/Automattic/amp-wp/compare/master...gabrielperezs:maste |
|
What is status of this work? I'd love to use this on a small site. |
|
Again, what's the status with allowing AMP on pages? This looks likes it was ready as of 25th February. Any update? |
|
Hello do you know when or if page support will be ready? It looks like you've already started the work here. This would be really good for website based wordpress sites. I understand that you guys are busy and appreciate that you have put so much time and effort into the project already, So not pressuring just wondering as I imagine a lot of wordpress members would love this. |
|
@mjangda and @technosailor, Which begs the question: What's the hang up here? |
|
I'm wondering as well, why this PR has not been merged already (currently 2 files are conflicting @evintus). Pages support is crucial for this plugin to take off in my opionion. |
|
Hello, do you know when or if page support will be ready? |
|
I wonder if the following Trac ticket has anything to do with why this isn't being merged in to master? When I fork and add support for pages I can confirm that the front-page 404's and does not handle the query/amp canonical redirect correctly, but non-front-page pages work just fine. |
|
See also #619. |
This addresses Issue #176