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

Framework: Extract the block editor from the editor chrome to make it reusable #2065

Closed
wants to merge 3 commits into from

Conversation

youknowriad
Copy link
Contributor

@youknowriad youknowriad commented Jul 28, 2017

At this stage, this is more a proof of concept than a mergeable PR but this deserves early feedback in order to decide if we should continue this way or not.

The idea here is to extract a configurable/generic block editor from Gutenberg in order to reuse it later for more than the post content.

The editor module provides generic React Components usable with the following API:

Import { Provider, Inserter, FullInserter, Editor, FullInspector } from 'editor';

const config = {
  blockTypes: [],
  categories: [], 
  defaultBlockName: 'core/text',
  fallbackBlockName: 'core/free-form',
}

<Provider config={ config } value={ value } onChange={ onChange }>
   <div id=“header”>
      <Inserter />
   </div>
   <div id=“sidebar”>
      <BlockInspector />
   </div>
   <div id=“main”>
      <Editor />
      <FullInspector />
   </div>
</Provider>

How do we achieve this:

  • Split the current editor into two separate modules a generic editor module and a editor-chrome module making use of the editor module to build the WP editor page but the difficulty here is to achieve this without rewriting all our components. This PR tries to solve this by splitting the store (selectors, actions, reducers) into two separate store and use a specific storeKey=“editor” for the editor’s store (which is basically the local state of the editor)
  • This PR also means, parsing and serializing shouldn’t rely on the global block registration but take the available block types and the fallback block type as parameters
  • Same for other APIs: switching blocks, creating Blocks. We can’t rely on the global blockTypes
  • The registerBlockType, unregisterBlockType, setUnknownTypeHandler are still here but their purpose changes from “Registering the block to be used in the editor” to “Register a block in WP” and later we need to create a config object (see above) by retrieving the registered blocks.

Not done yet:

  • Multiselection (it’s basically their, we just need a onMultiSelect or onSelect prop to the Provider in order to show/hide the MultiSelectHeader
  • The block switcher is broken because the transformations make extensive use of the createBlock function which accept the blockType instead of the blockName now (because we can’t rely on the globally registered blocks)
  • Tests are broken (some needs to be moved, some needs to be rewritten)
  • The block inspector toggle is broken (probably needs a prop onToggleInspector or something)
  • Documentation
  • CSS: We already have this issue, not specific to this PR but we should modularize our CSS instead of relying on the same variables across modules. (Maybe a common css module)
  • This is buggy and I took a lot of shortcuts


I suspect we’ll need this (or similar) soon. I think we should solve this as soon as possible because it’s obviously a very big change and the more we add API, the more difficult it will be.

@youknowriad youknowriad added the Framework Issues related to broader framework topics, especially as it relates to javascript label Jul 28, 2017
@youknowriad youknowriad self-assigned this Jul 28, 2017
@youknowriad
Copy link
Contributor Author

@aduth @mtias @iseulde @nylen I'd love to have some thoughts on this, even if it's "we do not want/need this"

@aduth
Copy link
Member

aduth commented Jul 31, 2017

My initial reaction is: This sounds like something we would need if we want a common editor foundation elsewhere in the editor (e.g. Customizer) without bundling all the chrome pieces that currently reside in editor.

That said, I think it may be a little premature to start splitting at this point. Because we don't really know what other editors will need or look like, it's hard to determine what of the current editor we want to split. We don't even necessary know that the pieces that live in the editor currently are ones we want to keep, so trying to expose a common interface might hold us back from iterating quickly in this area.

This PR also means, parsing and serializing shouldn’t rely on the global block registration but take the available block types and the fallback block type as parameters

This is an interesting point to raise, because even if we weren't to pursue splitting the editor into reusable pieces right now, maybe this is something we want in the editor as it stands?

Some previous discussion at: #336, #1132 (comment)

cc @jasmussen @westonruter

@jasmussen
Copy link
Contributor

I can't speak to the most technical of aspects, but it sounds like there's a lot of healthy thinking around this.

This thing we've built, currently, consists of both an editor bar (for document level operations, save, preview, publish), and a sidebar (for metadata), in addition to the "pure content". If I understand the purpose of this PR right, it is to make it easier to take the part that is just content, and make it re-usable elsewhere.

If that's right, then it's an idea I really like. Imagine enabling it on a comment field, and simply passing an array of allowed blocks (if not all).

We'd want to figure out how the block inspector would work, but it seems like it should be a matter of popoing up a modal, perhaps one that floats if we really need to see the content for a live preview.

@youknowriad
Copy link
Contributor Author

If I understand the purpose of this PR right, it is to make it easier to take the part that is just content, and make it re-usable elsewhere.

That's exactly what this PR is about

Because we don't really know what other editors will need or look like

Fair point, but IMO splitting the "block" editor is something we'd want no matter what features the other editors will have, the idea is simple: A component having a "value" corresponding to a list of blocks and an "onChange" callback. It's UI could be composed as needed using the different UI components (inserter, list of blocks, inspector).

Also was taking about nesting with @mtias and this could be useful to enable this we could embed a block editor inside a block.

@nylen
Copy link
Member

nylen commented Jul 31, 2017

👍 I think we should do this, however it seems too big to be viable. What are your thoughts on breaking it up into steps? Maybe a good first step would be the block API changes to rely less on a global registry.

@nylen
Copy link
Member

nylen commented Jul 31, 2017

Also, we need to be sure that we follow up on this refactoring by enabling the editor in some other field and making sure it works reasonably well.

@youknowriad
Copy link
Contributor Author

What are your thoughts on breaking it up into steps? Maybe a good first step would be the block API changes to rely less on a global registry.

Sounds like a good idea to break it into smaller steps, but even "not relying on the global registry" and providing a "config" is a big change on its own. I'll see what I can do about this later.

* WordPress dependencies
*/
import { parse, getBlockTypes, getCategories, getDefaultBlock, getUnknownTypeHandler } from 'blocks';
import { render } from 'element';
Copy link
Member

Choose a reason for hiding this comment

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

Note with #2172, these need to be updated to prefix the dependencies with @wordpress/. You will need to perform a rebase against the latest version of master and apply your changes:

git fetch origin
git rebase origin/master

*/
export function switchToBlockType( block, name ) {
export function switchToBlockType( block, sourceType, destinationType ) {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this change needed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we don't rely on the globally registered blocks anymore to ensure genericity of the block editor.

By the way, #2182 is an attempt to split the current PR into smaller steps. It's intended as the first iteration to achieve this POC.

*
* @return {Object} Action object
*/
export function autosave() {
Copy link
Member

Choose a reason for hiding this comment

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

Why are these actions of the chrome? Does it mean if I embed an editor I don't get autosave, etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this PR decouples the "post" from the "blocks". Meaning the editor-chrome module handles all the Post Save, Update, Creation ... and the editor module is a generic block editor module used by the editor chrome to fill the post-content (for now, metaboxes, title ... could come later as a block editor too)

@youknowriad
Copy link
Contributor Author

closing as what a "generic editor" means has changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

5 participants