New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Gutenberg] Extend existing core blocks for AMP specific functionality #1008

Closed
postphotos opened this Issue Mar 9, 2018 · 10 comments

Comments

@postphotos
Copy link
Contributor

postphotos commented Mar 9, 2018

As a WordPress site running Gutenberg, my core/native blocks should have extended AMP-HTML markup features.

  • AC1: Extend core blocks which have validation errors so that the appropriate AMP markup is output to faithfully render the block when the page is served as AMP. This would be done via some combination of the render_callback that integrates with a new block sanitizer/preprocessor.

  • AC2: Further extend core blocks to leverage AMP functionality, such as layout.

@westonruter

This comment has been minimized.

Copy link
Member

westonruter commented Mar 9, 2018

@MackenzieHartung MackenzieHartung added this to the v1.0 milestone Mar 12, 2018

@MackenzieHartung MackenzieHartung added this to To do in v1.0 Mar 12, 2018

@kienstra kienstra moved this from To do to Definition in v1.0 Mar 15, 2018

@kienstra

This comment has been minimized.

Copy link
Collaborator

kienstra commented Mar 15, 2018

Update On Related Work

Hi @postphotos,
You probably saw @westonruter's comment above, linking to #902:

  1. Extend core blocks which have validation errors so that the appropriate AMP markup is output to faithfully render the block when the page is served as AMP.

In #1010, @miina is now working on a block that doesn't support AMP: 'categories' with a dropdown.

  1. Further extend core blocks to leverage AMP functionality, such as layout.

That would be good for this issue.

@postphotos

This comment has been minimized.

Copy link
Contributor

postphotos commented Mar 15, 2018

Thanks for calling it out, @kienstra - I believe the addition of the story and AC at the top of this issue pulls the items from #902 clearly.

@miina

This comment has been minimized.

Copy link
Collaborator

miina commented Mar 16, 2018

Just FI that started looking into this and will add a possible "prototype" of the approach for discussing (based on adding AMP Layout setting) in the beginning of the new week.

@westonruter

This comment has been minimized.

Copy link
Member

westonruter commented Mar 17, 2018

Awesome. I'm looking forward to seeing that!

@miina

This comment has been minimized.

Copy link
Collaborator

miina commented Mar 19, 2018

Created a PR with initial take on the approach: #1026. Would be great to receive some feedback before continuing.

@miina miina moved this from Definition to In progress in v1.0 Mar 20, 2018

@MackenzieHartung MackenzieHartung added Sprint 2 and removed Sprint 1 labels Mar 26, 2018

@postphotos postphotos added Sprint 3 and removed Sprint 2 labels Apr 5, 2018

@westonruter

This comment has been minimized.

Copy link
Member

westonruter commented Apr 18, 2018

We've talked about this before, but as part of this we'll need to make sure that any AMP components and attributes that get added to post_content will need to get whitelisted for Kses.

@westonruter

This comment has been minimized.

Copy link
Member

westonruter commented Apr 18, 2018

Regarding amp-carousel see #1065 for how it should be integrated into the existing Gallery block.

In other words, there needn't be a new amp-carousel block in #1039.

@postphotos postphotos added the Sprint 4 label Apr 20, 2018

@miina miina moved this from In progress to Ready for review in v1.0 May 3, 2018

miina added a commit that referenced this issue May 5, 2018

miina added a commit that referenced this issue May 21, 2018

@westonruter westonruter moved this from Ready for review to Ready for QA in v1.0 May 21, 2018

@kienstra

This comment has been minimized.

Copy link
Collaborator

kienstra commented Jun 6, 2018

Request For Testing

Hi @csossi,
Could you please test this?

  1. Go to https://native-ampconfdemo.pantheonsite.io/
  2. Create a new post
  3. Add an image block
  4. In the "Block" panel, choose a different value for "AMP Layout"
  5. Select "AMP lightbox effect"
  6. Click "Save Draft"
  7. Expected: those settings still display
  8. Click "Preview"
  9. Click the image
  10. Expected: It expands to fill the screen, in a "Lightbox Effect"
  11. Repeat steps 3-10, but with the "Gallery" block
@csossi

This comment has been minimized.

Copy link

csossi commented Jun 8, 2018

verified in QA

@csossi csossi moved this from Ready for QA to Ready for Merging in v1.0 Jun 8, 2018

@kienstra kienstra moved this from Ready for Merging to In Production in v1.0 Dec 11, 2018

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