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

Dynamic reusable blocks: technical details #2081

Closed
westonruter opened this issue Jul 29, 2017 · 18 comments
Closed

Dynamic reusable blocks: technical details #2081

westonruter opened this issue Jul 29, 2017 · 18 comments
Labels
[Feature] Blocks Overall functionality of blocks [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks)

Comments

@westonruter
Copy link
Member

I'm splitting this out from #1516 (Turning a static block into a dynamic reusable block) since that issue is long and primarily focused on the UI aspects. Here's what I've been thinking of in terms of how such reusable blocks would work at a technical level:

  1. There would be a new block post type.
  2. The block post would store in its post_content one serialized block. Potentially we could store the block's attributes individually in postmeta instead, and just the contents of the block in post_content. That would improve the ability to do queries on the blocks. If a block type is registered with PHP, it could have serialize/deserialize logic that would dictate whether the attributes get stored in postmeta instead of in content.
  3. A block would be identified by a UUID which would get saved in post_name. Using a UUID instead of auto-incremented post ID would eliminate the requirement that a block post be created up-front to reserve its ID. The auto-incremented post ID would be for internal use only and not exposed in the API.
  4. A block_type taxonomy would exist and each block would be associated with one and only one term that would store the block type as the term's slug. The sanitization logic for such terms could be overridden to allow slashes in the slug, e.g. core/text.
  5. The block post type would be exposed in the REST API with endpoints such as:
  • /gutenberg/v1/block-types - Collection of registered block types.
  • /gutenberg/v1/blocks - Collection of all blocks.
  • /gutenberg/v1/blocks?type=text/core - Collection of blocks of the core/text type.
  • /gutenberg/v1/blocks/358b59ee-bab3-4d6f-8445-e8c6971a5601 - Single core/text block.
  1. A block resource would look like:
{
	"id": "358b59ee-bab3-4d6f-8445-e8c6971a5601",
	"type": "core/text",
	"attributes": {
		"dropCap": true,
		"align": "center"
	},
	"content": "<p style=\"text-align:center\" class=\"has-drop-cap\">Hello World!</p>"
}
  1. External blocks would be embedded in a post via a core/block block type which has a single attribute that contains the UUID (REST API resource id) for the external block, for example: <!-- wp:core/block { "ref": "358b59ee-bab3-4d6f-8445-e8c6971a5601" } /-->
  2. When a core/block block initializes, it looks up the from the REST API via the UUID supplied in the ref attribute, and then it creates a block component for the type returned for that block resource.
  3. Modifications to such an external core/block's attributes get written not into the containing post's content, but rather get written into edits for that external block resource.
  4. Saving the editor updates not only the main post resource, but also any edits to the block resources that were modified.
  5. The core/block block would be registered in PHP, and its render_callback would look up the block post by its UUID ref, and embed the content from that block post.
  6. When a non-reusable block (attached block) is converted into a reusable (detached) block it would go through a transformation that would result in a new REST API block resource being drafted among the edits, including a newly-generated UUID and all of the attributes from the block would be moved into this detached block resource, with the transformed block in content then getting the generated UUID as its sole ref attribute.
  7. Likewise, a reusable/detached could be re-attached to the content by reversing the transformation in the previous step.

Thoughts?

@westonruter
Copy link
Member Author

Also: If a PHP-registered block had schema and validation logic defined, the endpoint could validate the incoming attributes upon updating. Without a block being registered in PHP or without any schema defined, then it would just have to accept any attributes and content blindly.

@mtias
Copy link
Member

mtias commented Aug 1, 2017

This is more or less what I had in mind, thanks for diving into the details!

Modifications to such an external core/block's attributes get written not into the containing post's content, but rather get written into edits for that external block resource.

And also separately from the action of saving a post. Modifications to a global block would likely have to offer the ability to convert to regular block, or to edit globally. Maybe on blur the user is asked this.

I'd call this core/global-block, perhaps, for clarity.

@noisysocks
Copy link
Member

noisysocks commented Aug 19, 2017

This approach makes a lot of sense! I'm going to take a crack at implementing this, starting with the REST API that you describe.

/gutenberg/v1/blocks?type=text/core - Collection of blocks of the core/text type.

When would this endpoint be used? I don't think it would be necessary to build a flow like the one that @jasmussen described here.

@noisysocks
Copy link
Member

I experimented a little with implementing reusable blocks as a self-contained block type over in this branch. Here's a little demo:

reusable-blocks-2

This isn't a workable approach, though, because:

  • The user can't undo/redo changes made to the referenced block's attributes, since those attributes are stored in the edit component's local state and not in the redux state tree.
  • A transform function can't be written to switch to/from a reusable block, since it doesn't have access to the referenced block's attributes which are stored within that reusable block's edit component.
  • Reusable blocks, implemented this way, persist their changes to the REST API on blur. This is a confusing UX—I would expect that any changes I make to a post aren't persisted whatsoever unless I click Save.

Here's a sketch of the approach I'm now considering:

  • We have a core/reusable-block block type that doesn't define a save or edit function—it exists solely so that the parser can interpret a <!-- wp:core/reusable-block { "ref": "358b59ee-bab3-4d6f-8445-e8c6971a5601" } /-->.
  • We add reusableBlocks to the redux state tree, under editor so that changes made to reusable blocks are undoable. Each reusable block in this object is keyed by its ID, and would look like:
    {
        type: 'core/paragraph',
        attributes: { content: '<p>Hello</p>' },
        isLoading: false,
        isDirty: false,
    }
  • FETCH_REUSABLE_BLOCK is an action that causes an effect to fetch a reusable block from the API. The reusable block is marked as loading during this time. On success, the fetched block inserted at reusableBlocks[id].
  • UPDATE_REUSABLE_BLOCK is an action that updates the block at reusableBlocks[id]. This also marks the block as dirty.
  • PERSIST_REUSABLE_BLOCK is an action that causes an effect to save a reusable block to the API. On success, the block at reusableBlocks[id] is marked as clean.
  • On RESET_BLOCKS, an effect runs that dispatches FETCH_REUSABLE_BLOCK for every reusable block in the post.
  • On REQUEST_POST_UPDATE, an effect runs that dispatches PERSIST_REUSABLE_BLOCK for every reusable block that is marked dirty.
  • We change <VisualEditorBlock> so that, if block.name === 'core/reusable-block:
    • It renders the <BlockEdit> for reusableBlocks[ ref ] instead of for block.
    • It passes <BlockEdit> a setAttributes prop that dispatches UPDATE_REUSABLE_BLOCK.
    • If the reusable block is marked as loading, it displays a spinner.
  • Converting a reusable block to a static block can be done by dispatching replaceBlock(id, createBlock(reusableBlocks[id].type, reusableBlocks[id].attributes)).
  • Converting a static block to a reusable block can be done by dispatching updateReusableBlock(newUuid, block.name, block.attributes) and replaceBlock(block.uid, createBlock('core/reusable-block', { ref: newUuid }).

Thoughts?

@mtias
Copy link
Member

mtias commented Aug 30, 2017

That's a great overview and plan, thanks for thinking it through.

The user can't undo/redo changes made to the referenced block's attributes, since those attributes are stored in the edit component's local state and not in the redux state tree.

This is an interesting design consideration, but I wouldn't label it a blocker for an initial implementation.

Reusable blocks, implemented this way, persist their changes to the REST API on blur.

I agree this is not ideal, but global-blocks are different and ought to be saved separately from the post. (There are other previous examples of metaboxes in WordPress doing similar operations separately from post save actions.)

The proposed designs so far show the following dialog when attempting to edit a global block. I think this could be shown on blur the first time you edit instead, making it clear something will be saved right then and there:

reusable blocks 04

Or we could have an "update globally" button within the block UI.

We have a core/reusable-block block type that doesn't define a save or edit function

This is interesting and it makes sense—as a "reusable block" doesn't really have specific UI but that of the original block it was created from—so it works pretty much as a pointer to another block. That said, the options shown in the above screenshot (or whatever iterations we do from there) could be considered the edit render definition of the reusable block. In that case, I might expect these to de declared on the edit method of the reusable block.

Pseudo code:

edit() {
    return (
        <div>
            <GlobalBlockUI />
            <OriginalBlockEdit />
        <div>
    );
}

Granted, this won't work given how VisualBlock operates, but just throwing it out there, as we could start considering global blocks as a kind of parent-child block, with parent being the global wrapper, and child being the original block.

@youknowriad
Copy link
Contributor

An alternative could be to display the "save" output of the original block by default and have an "edit" button to enter Edit mode of this block. This makes it more obvious that it's separate from the post flow.

@noisysocks
Copy link
Member

noisysocks commented Aug 30, 2017

That said, the options shown in the above screenshot (or whatever iterations we do from there) could be considered the edit render definition of the reusable block. In that case, I might expect these to de declared on the edit method of the reusable block.

Agree that it'd be nice to have the UI live in the edit component. Although it would mean that we have to import selectors and action creators from within blocks/reusable-block/index.js, which feels pretty weird.

An alternative could be to display the "save" output of the original block by default and have an "edit" button to enter Edit mode of this block. This makes it more obvious that it's separate from the post flow.

This is a neat idea! Would be great to get a design made for it.

@andreiglingeanu
Copy link
Contributor

andreiglingeanu commented Sep 20, 2017

There are other previous examples of metaboxes in WordPress doing similar operations separately from post save actions.

@mtias Could you please point me into the direction of them? I'm looking for some patterns/examples for doing that?

This feature looks very nice, by the way. Nice work @noisysocks!

The idea of having the reusable block as a block type is very nice, but I'm thinking it would fail shortly with the arrival of groups of blocks (and maybe with nested blocks). #1516 (comment)

@mtias
Copy link
Member

mtias commented Sep 20, 2017

Could you please point me into the direction of them?

Yes, core itself :) When you enter a new custom field, you don't have to save the post.

image

It is not very intuitive (there's a yellow background thing that sort of implies it has been added).

The group of blocks was clarified here: #1516 (comment)

It would use the same system because the group of blocks would always have a block as the parent, and that parent is the one you make reusable.

@westonruter
Copy link
Member Author

Reusable blocks, implemented this way, persist their changes to the REST API on blur.

I agree this is not ideal, but global-blocks are different and ought to be saved separately from the post. (There are other previous examples of metaboxes in WordPress doing similar operations separately from post save actions.)

I really think we should avoid this if at all possible. The “Custom Fields” UI isn't exactly a heavily-trodden cowpath that serves as an ideal model to follow. In fact, until recently the Featured Image was another such metabox that updated instantly upon changing, even without saving the post as a whole. This was brought up in #12922 (Setting featured image does not require the post to be saved) and has been fixed so that now you have to explicitly save the post as a whole in order to get the featured image change to persist.

This is the same model that we should follow for reusable blocks. As a user, after I make a change to a regular block and make a change to a global block, I should then be able to click Preview to see changes to both of those blocks on the frontend. If then I decide I don't like the changes, I should be able to leave the post and the changes I made should be discarded—both to the regular block and the reusable block.

Essentially the redux state as a whole should be what is submitted to the preview, including the changes to the reusable blocks and the local blocks. In the Customizer, previewing would be implemented by putting the state into a changeset. This is what I was referring to in #1516 (comment). Submitting the entire redux state tree for the preview would allow for previewing changes to not only the post but also the postmeta, something which is not possible in core now either (#20299).

@andreiglingeanu
Copy link
Contributor

Yes, core itself :) When you enter a new custom field, you don't have to save the post.
It is not very intuitive (there's a yellow background thing that sort of implies it has been added).

Oh, yeah, I totally forgot about them! Yes, they are a perfect example for that sort of use case and I'll agree that they are not very intuitive. A lot of corner cases and unnecessary code paths usually arise when one starts to go down that path.

But, sometimes, you plainly can't avoid that. And the reusable blocks are one instance of such a case.

@aduth
Copy link
Member

aduth commented Oct 16, 2017

A block would be identified by a UUID which would get saved in post_name. Using a UUID instead of auto-incremented post ID would eliminate the requirement that a block post be created up-front to reserve its ID. The auto-incremented post ID would be for internal use only and not exposed in the API.

Can you elaborate on why, if we were to use a standard post ID, we would need to reserve an ID up-front? This as a follow-on to my review at #2659 (review) where my initial impression was that we're going far out of our way to avoid using IDs, and I'm not sure what we're gaining in doing so.

@andreiglingeanu
Copy link
Contributor

Probably because the frontend code will be able to create reusable blocks that won't collide with other ones, without having to touch the server for doing so.

@aduth
Copy link
Member

aduth commented Oct 16, 2017

In that case, it seems like we could still assign a temporary ID or other identifier to mark the block as not-yet-persisted while manipulating it in the client, then update the local copy once the server-assigned ID becomes known.

In Calypso, we do similar for media where we want to be able to reference and display an item even prior to it being uploaded to the server, using a combination of a boolean flag property and prefixed ID:

https://github.com/Automattic/wp-calypso/blob/e3bca1e/client/lib/media/utils.js#L531-L586

@westonruter
Copy link
Member Author

@aduth @andreiglingeanu A few reasons for why I wanted to go with UUIDs:

  • https://core.trac.wordpress.org/ticket/38072 — In core presently we are using a hack for adding new nav menu items in the customizer, to assign them a negative random integer to indicate they are nav menu items that aren't yet saved into the database. Ultimately we're now thinking of going to use a regular post ID for an auto-draft, but that means we have to do a request to “reserve” the ID so that someone else doesn't snatch it up concurrently. If we could use UUIDs instead, then we could avoid this, and there is no need to swap out the underlying ID when it gets saved into the DB.
  • https://core.trac.wordpress.org/ticket/35669 — Widgets in core aren't currently concurrency safe since they never implemented what was implemented for nav menu items:

Concurrency: Since all widget instances of a given type are stored in a single option, if two users attempt to update two separate widgets at the same time, it is possible that one of the updates will get lost (see #31245). Additionally, the widgets admin page and widgets in the Customizer both get loaded with the max number (array index) for each widget type. When a new widget instance is created, this maximum number is incremented in memory and used in the new widget ID which is then passed to the server for saving. If two users have loaded the UI at the same time, when they both create a widget of a given type and save their widget changes, the one who saves last will overwrite the other user's widget since the two widgets would have the same ID. (See #32183 for more about the widget ID collisions, and see Customize Widgets Plus for a “Widget Number Incrementing” component which uses Ajax to generate new widget IDs in a more concurrency-safe manner.)

  • Customizer changesets use UUIDs so that the ID is known the moment the Customizer loads. One benefit of this is it doesn't have to create a throwaway auto-draft (as is done for the edit post screen). There are other benefits, like having a unique universal identifier that can also be secret preview key.
  • Aren't UUIDs used in the Gutenberg for something?
  • What would the post_name for a Block contain otherwise? It's already a unique indexible field, so storing a UUID is handy here for lookups and we get “for free” portability of such block posts when migrated across WordPress installs, if we only ever reference UUIDs and not the underlying integer IDs.

@joehoyle
Copy link

I might be missing a piece of this puzzle so apologies for wading into this late! The above REST spec and some associated features seem to make the assumption that the registered blocks are available server-side, but as I understand it, it's recommend to write / register custom blocks in JavaScript: https://wordpress.org/gutenberg/handbook/blocks/writing-your-first-block-type/

This is a "limitation" that struck me when reading the handbook - by having all block registration done client side, it becomes impossible to provide server-side functionality and abstractions, such as a /block-types REST API endpoint. Having them available server side allows us to do some more security wise validation, which would presumably be more important when blocks can be used outside of the post_content (which is safe, as already has all the reliability of existing sanitization).

It would also open up the possibility of other block interfaces backed by a REST API, such as mobile apps. Again, if this has been discussed elsewhere, a link would be great. By having block registration as part of the Gutenberg app client side, the blocks and general Gutenberg concept will never be able to escape the confines of the WP Admin. While I don't think it will be possible to make blocks totally portable (UI / front end logic is always going to be more powerful expressed as JavaScript components), having registration happen at the server level (and only the server level) will ensure they are more portable.

@aduth
Copy link
Member

aduth commented Nov 20, 2017

Related: #2751

@mtias
Copy link
Member

mtias commented Dec 7, 2017

Closing as technical implementation is ready.

@mtias mtias closed this as completed Dec 7, 2017
@aduth aduth added the [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) label Dec 19, 2017
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 [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks)
Projects
None yet
Development

No branches or pull requests

7 participants