-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Improve the Reusable Blocks UI by relying on multi entity save flow. #27887
Conversation
This is great, so much code cleanup! Let's see what design improvements can be made and shared between template parts and reusable blocks. |
/> | ||
) ) } | ||
|
||
{ <div { ...innerBlocksProps } /> } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this div necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, that's the actual inner blocks.
Size Change: -1.25 kB (0%) Total Size: 1.28 MB
ℹ️ View Unchanged
|
This and the fact the inspector now works for individual blocks contained within is amazing. There are some things that pop up design wise as worth doing, but we can follow up on them afterwards — like indicating when a reusable block has had changes in the reusable block toolbar (has unsaved changes). We should also move the reusable block name next to the block type icon (like we are doing for template parts) and update the name of the block in things like breadcrumb and block navigation to use the actual title of the block. |
That list of "closes" is epic |
Fantastic to see; thanks for tackling @youknowriad! |
Nice PR 👍 I can't duplicate the block or edit it, if a reusable block is used twice in a post. reusable_block.mov |
d3ad654
to
1f00883
Compare
Without the in-canvas name this feels alright to move forwards and we can follow up for a better presentation in the toolbar of the name. There's something odd in the flow of creating these when the name is set to undefined because you don't get a chance to modify it before creation. It's probably time to add a modal requiring you to input a name before a reusable block is created. |
We are having increased reports of broken reusable blocks (and posts using them) on WordPress.com, apparently started after this change (see original issue). The issue is technically caused by reusable blocks referencing themselves, which if I'm not mistaken, is currently being addressed in #27012 (which is not limited to reusable blocks, but anything where such infinite loop could happen). Regardless, the infinite loop was possible even before this PR. Before: users had to explicitly edit (and save) a reusable block directly in the block UI itself. What seems to happen is that folks might want to add multiple instances of the same reusable block "in sequence", but they accidentally get "stuck" inside the reusable block group.
As I said, the infinite loop issue is already being addressed (although, we might probably want to expedite the fix!), but we might also want to reconsider if this very light UI is the right approach for reusable blocks, or any kind of nested blocks. I consider myself an expert user so I know where to look to figure out the nesting situation. cc @mtias |
See #27890 for some design considerations and possible improvements |
EDIT: |
@paaljoachim there are related tasks in #27890 |
closes #18560
closes #22832
closes #26824
closes #26670
closes #19436
closes #16614
closes #12255
closes #12079
closes #11784
closes #22405
closes #19372
This PR refactors the reusable block to use a controlled innerBlocks. This means several things:
For an initial version, I just show the "title input" when the block is selected and hide it otherwise but this creates "jumps" in the UI, we need better design here.