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.

Sign up for GitHub

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

Jump to bottom

Gutenberg: jetpack_set_available_blocks revert to get data only from manifest file #10844

Closed
wants to merge 3 commits into from
Closed

Gutenberg: jetpack_set_available_blocks revert to get data only from manifest file #10844

wants to merge 3 commits into from

Conversation

enejb
Copy link
Member

@enejb enejb commented Dec 5, 2018

Simplifies the logic in what blocks are available by pulling the data from a single place the block-manifest.json.

Changes proposed in this Pull Request:

  • This simplifies how blocks are loaded.

Testing instructions:

cc: @roccotripaldi, @simison

@enejb enejb requested a review from a team December 5, 2018 20:35
@matticbot
Copy link
Contributor

D21879-code. (newly created revision)

@enejb enejb self-assigned this Dec 5, 2018
@enejb enejb added the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Dec 5, 2018
@enejb enejb mentioned this pull request Dec 5, 2018
Closed
@jetpackbot
Copy link

jetpackbot commented Dec 5, 2018 •
edited
Loading

Warnings
⚠️ "Proposed changelog entry" is missing for this PR. Please include any meaningful changes

This is automated check which relies on PULL_REQUEST_TEMPLATE.We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against 3697cf2

@jeherve jeherve added the [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack label Dec 6, 2018
@jeherve jeherve added this to the 6.9 milestone Dec 6, 2018
@jeherve jeherve added [Feature] Contact Form [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Dec 6, 2018
jeherve
jeherve previously requested changes Dec 6, 2018
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

It works well on my end. Small nitpick though, about the docblock you edited.

class.jetpack-gutenberg.php Outdated
@@ -124,7 +124,6 @@ public static function get_preset( $preset ) {
/**
* Filters the results of `apply_filter( 'jetpack_set_available_blocks', array() )`
* using the merged contents of `blocks-manifest.json` ( $preset_blocks )
Copy link
Member

Choose a reason for hiding this comment

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

You'll need to update that comment since there is no merging happening anymore.

@enejb enejb force-pushed the update/jetpack-gutenberg-available-blocks branch from 0b79807 to 4deb036 Compare December 12, 2018 00:35
@enejb enejb dismissed jeherve’s stale review December 12, 2018 00:36

This was resolved

enejb added 2 commits December 12, 2018 11:43
@enejb
Updates
04bbd76
@enejb
Blocks: Any use index.json in development mode
ac38bc3 
Only use the filter that applies index.json blocks in development mode or if the `JETPACK_BETA_BLOCKS` are trying to be used.
@enejb enejb force-pushed the update/jetpack-gutenberg-available-blocks branch from 4deb036 to ac38bc3 Compare December 12, 2018 20:09
@enejb enejb added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Dec 12, 2018
lezama
lezama reviewed Dec 12, 2018
View reviewed changes
class.jetpack.php Outdated Show resolved Hide resolved
lezama
lezama reviewed Dec 12, 2018
View reviewed changes
class.jetpack-gutenberg.php Outdated Show resolved Hide resolved
lezama
lezama approved these changes Dec 12, 2018
View reviewed changes
Copy link
Contributor

@lezama lezama left a comment

Choose a reason for hiding this comment

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

other than a couple of minor format remarks, 🚢 IT

@jeherve
Copy link
Member

jeherve commented Dec 13, 2018

I am sorry, I may be a bit slow, but I am not sure I understand what is happening here.

What do you think about updating https://github.com/Automattic/jetpack/blob/master/docs/guides/gutenberg-blocks.md to mention the different ways a block should be registered:

  • When running Jetpack in Dev Mode.
  • When supplying the JETPACK_BETA_BLOCKS constant.
  • When not using Dev Mode.

If I were to add a new block to Jetpack today, how should I go about it basically? Where should the block be added? I would have the same question for plugins, since it seems they are now treated differently, and appear on 3 lists?

I think some documentation about all this may help me, and maybe others in the future :)

@jeherve jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Jan 2, 2019
@jeherve
Copy link
Member

jeherve commented Jan 2, 2019

Is this still valid? If so, it would need a rebase, and addressing the questions I had above.

@jeherve jeherve removed this from the 6.9 milestone Jan 3, 2019
@sirreal
Copy link
Member

sirreal commented Jan 7, 2019

I'm closing, I believe this was superseded by other work like #11075.

If that's not the case, please reopen.

@sirreal sirreal closed this Jan 7, 2019
@jeherve jeherve deleted the update/jetpack-gutenberg-available-blocks branch January 7, 2019 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lezama lezama lezama approved these changes

@jeherve jeherve jeherve left review comments

@roccotripaldi roccotripaldi Awaiting requested review from roccotripaldi

@simison simison Awaiting requested review from simison

Assignees

@enejb enejb

Labels
[Feature] Contact Form [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! Touches WP.com Files
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@enejb @matticbot @jetpackbot @jeherve @sirreal @lezama