Skip to content
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
2 tasks
postphotos opened this issue Mar 9, 2018 · 10 comments
Closed
2 tasks
Assignees
Projects
Milestone

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
Copy link
Member

westonruter commented Mar 9, 2018

See #902 (comment)

@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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Member

Awesome. I'm looking forward to seeing that!

@miina
Copy link
Contributor

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
@westonruter
Copy link
Member

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
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.

@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
Copy link
Contributor

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
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
Projects
No open projects
v1.0
In Production
Development

No branches or pull requests

7 participants