Skip to content

Gutenberg: Use Blocks from staging dir if proxied#10713

Merged
lezama merged 4 commits intomasterfrom
update/set-blocks-dir-conditionally
Nov 23, 2018
Merged

Gutenberg: Use Blocks from staging dir if proxied#10713
lezama merged 4 commits intomasterfrom
update/set-blocks-dir-conditionally

Conversation

@ockham
Copy link
Copy Markdown
Contributor

@ockham ockham commented Nov 23, 2018

Mostly relevant for the WP.com side. See pMz3w-9fO-p2 for context.

Changes proposed in this Pull Request:

  • Move blocks directory definition to a method
  • Use Blocks from staging dir if proxied

Testing instructions:

Verify that Jetpack's Gutenberg blocks still work as before, with no errors throw.

Proposed changelog entry for your changes:

  • Use Gutenberg Blocks from staging dir if proxied

@ockham ockham added [Status] In Progress [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Nov 23, 2018
@ockham ockham added this to the 6.9 milestone Nov 23, 2018
@ockham ockham self-assigned this Nov 23, 2018
@ockham ockham requested a review from a team November 23, 2018 17:00
@matticbot
Copy link
Copy Markdown
Contributor

D21283-code. (newly created revision)

@jetpackbot
Copy link
Copy Markdown
Collaborator

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: November 26, 2018.
Scheduled code freeze: November 19, 2018

Generated by 🚫 dangerJS

@ockham ockham force-pushed the update/set-blocks-dir-conditionally branch from 9a2497f to ef1d4e5 Compare November 23, 2018 17:21
@ockham
Copy link
Copy Markdown
Contributor Author

ockham commented Nov 23, 2018

Hmm, the Fusion-produced patch is wonky. Looks like the file has gone out of sync 🙁

@ockham
Copy link
Copy Markdown
Contributor Author

ockham commented Nov 23, 2018

Looks like it's because of #10537.

* @return string The Gutenberg extensions directory
*/
public static function get_blocks_directory() {
if ( function_exists( 'wpcom_is_proxied_request' ) && wpcom_is_proxied_request() ) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of doing this can we just add a filer for this?
This way we can just filter it on .com and not have to reply on function exiting check

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! 👍

@ockham
Copy link
Copy Markdown
Contributor Author

ockham commented Nov 23, 2018

Looks like it's because of #10537.

Fixed, thanks to @enejb's D21287-code.

if ( apply_filters( 'jetpack_blocks_load_staging', false ) ) {
return '_inc/blocks-staging/';
}
return '_inc/blocks/';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking more like

public static function get_blocks_directory() { 
/**
filter doc block
*/
return apply_filters( 'jetpack_blocks_directory', '_inc/blocks-staging/' );
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea

Copy link
Copy Markdown
Member

@enejb enejb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this and it works as expected. Nicely done

Copy link
Copy Markdown
Contributor

@oskosk oskosk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@oskosk oskosk added the [Status] Ready to Merge Go ahead, you can push that green button! label Nov 23, 2018
@oskosk oskosk removed the [Status] Needs Review This PR is ready for review. label Nov 23, 2018
@lezama lezama merged commit 43885c9 into master Nov 23, 2018
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Nov 23, 2018
@lezama lezama deleted the update/set-blocks-dir-conditionally branch November 23, 2018 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack Touches WP.com Files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants