xwp / travel

WordPress AMP Adventures Themes (Native AMP)
GNU General Public License v3.0
34 stars 6 forks source link

Update the blocks to the latest version of Gutenberg #86

Closed kienstra closed 5 years ago

kienstra commented 5 years ago
kienstra commented 5 years ago

Request For Code Review

Hi @miina, Would you be available to review this?

For the dynamic blocks, this uses ServerSideRender.

If you still have this repo set up, please re-run composer install && npm install && npm run dev

Here's the display in the block editor with this PR:

travel-blocks

Thanks, Miina! No problem if you're really busy, I could see if someone else is available.

miina commented 5 years ago

Sure, @kienstra, I'll review it shortly.

kienstra commented 5 years ago

Thanks For Reviewing

Hi @miina, Thanks for reviewing this.

https://github.com/xwp/travel/pull/86/commits/4ce2b7fe1257973b51832b298045176606075d77 adds <Disabled>, though at least in my local in looks like it's still possible to click the links and go to another page, like an 'Activity' link in the 'Activity List' block.

miina commented 5 years ago

@kienstra Okay, good to know, I had the impression that it would remove pointer events but looks like it might just disable buttons then 🤷‍♀️

Should be ready for merging then.

kienstra commented 5 years ago

Thanks, it might just be my local 😄

kienstra commented 5 years ago

Thanks, @miina! Could you please approve this if you're ready?

kienstra commented 5 years ago

Thanks!