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

Reorganize core supported blocks into top-level blocks directory #399

Merged
merged 5 commits into from
Apr 12, 2017

Conversation

aduth
Copy link
Member

@aduth aduth commented Apr 11, 2017

This pull request seeks to explore a restructuring of blocks to colocate implementations with the API in an effort to provide better control mechanisms over when and how blocks are enabled in an editor. In subsequent pull requests, this will be expanded to revise behavior of registration such that a block will no longer automatically be enabled as an option in all cases, but rather made available in a bucket of all known block types from which the editor can cherry-pick.

Potential cases where this may be useful:

  • Filtering blocks by post type
  • Filtering blocks by role
  • Filtering blocks by custom plugin logic
  • Filtering blocks by editor type (in case of future site building efforts)

The idea being that all core blocks are always registered, and a server-side-filtered array of desired block slugs will be provided for the editor at initialization time.

@aduth aduth added [Feature] Blocks Overall functionality of blocks Framework Issues related to broader framework topics, especially as it relates to javascript labels Apr 11, 2017
@aduth aduth requested a review from mtias April 11, 2017 12:37
@aduth aduth force-pushed the move/blocks-register-optin branch from 6f9fc56 to 2e198ab Compare April 11, 2017 12:41
@aduth
Copy link
Member Author

aduth commented Apr 11, 2017

Need to sort through a failing test at editor/test/state.js, likely related to recursive dependencies. Speaks to a more general concern about how exactly we should handle dependencies within and between directories.


const Editable = wp.blocks.Editable;
const { html, prop } = wp.blocks.query;
const { html, prop } = query;
Copy link
Member Author

Choose a reason for hiding this comment

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

That we can't destructure query here is a little disappointing. Might be worth a rethink on exporting hpq as wp.blocks.query, either renaming the property or simply assuming dependent plugins refer to hpq directly.

Incompatibillity with “export * from” and runtime transform

babel/babel#2877 (comment)

A bit uncertain why this would have no impact on non-test environments
(would it?)
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

We should probably merge this asap to avoid conflicts. The changes look good for me. Curious to know how did you fix the tests? Is it the transform-runtime removal?

@mtias
Copy link
Member

mtias commented Apr 12, 2017

Yes, let's get this in. Thanks @aduth :)

@aduth
Copy link
Member Author

aduth commented Apr 12, 2017

We should probably merge this asap to avoid conflicts. The changes look good for me. Curious to know how did you fix the tests? Is it the transform-runtime removal?

Yes, there's some more detail in the extended description of a71bea2. I'm still not entirely sure why the runtime polyfill works fine in the browser but not in Node.

@aduth aduth merged commit ae5daa3 into master Apr 12, 2017
@aduth aduth deleted the move/blocks-register-optin branch April 12, 2017 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks Framework Issues related to broader framework topics, especially as it relates to javascript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants