[Block Editor]: Conditionally show Styles if block is in Placeholder state
#33823
Conversation
ntsekouras
added
[Block] Image
ntsekouras
requested review from
ajitbohra,
chrisvanpatten and
ellatrix
as code owners
|
Size Change: +305 B (0%) Total Size: 1.03 MB
ℹ️ View Unchanged
|
|
Thanks for the PR Nik. It sounds like a valid approach but I'd like to take some time to reflect on what it means more holistically. Right now, block objects can have a
Some questions raised by this PR, they don't need to be answered or solved all in this PR, just food for thoughts. |
|
Thanks for the PR, @ntsekouras. It never occurred to me that we can store placeholder state this way. 🙇 My ideas always leaned towards
{
"apiVersion": 2,
"name": "core/image",
"attributes": {},
"supports": {
"anchor": true,
"placeholder": [ "url" ]
}
} |
|
I've left a comment about this at #33466 (comment) I think something like this is our best shot, although it still has drawbacks for third-party tools. |
|
@youknowriad I know your questions were meant to be rhetorical, but I've been thinking about this for a bit now, so I have some answers for some of them anyway.
I wasn't sure about this either, but I asked during the #core-editor chat and others seemed to think that it is. Looking around, more than a few blocks already have some from of a setup state where nothing will be displayed unless some attributes are set. In these cases, the block usually doesn't want to give a full set of controls until those required attributes are set.
One of my ideas would fit well with a state machine type implementation. Given that setup state is a thing that a block developer wants to use for their block (probably not too uncommon based on the above), from a block developer perspective, what would the easiest way to handle a setup state be? One of my ideas is adding a |
|
Thanks for the feedback all!
I believe we could - there are many blocks that use Placeholders. Also, with the state approach we leave blocks (objects) intact.
I'm not sure, but probably an
If we don't find/agree on another possible state for now, I'd be reluctant to implement a generic solution. I think it would be better to test it out as
@Mamaduka this can easily fall short for many blocks that need more complex calculation for this state. For example, lots of blocks are in Placeholder state when there are no
@ajlende since a |
I'm questioning if placehoders between blocks are different enough? And if it has to be tied to the edit function? What would be easier for a block developer to understand? Having a separate function for when you have required props that need to be set or adding additional wiring and branching within the edit function and having two different returns in one function? (I don't really know for sure) Having two different returns is what we already have—it's the more flexible solution, and it's able to do the things that we want to do. However, the additional wiring (the And I just thought of this, but we want the edit function to be visually as close to what is saved, right? In the case of things like images where we've completely replaced the UI in this setup case, having a separate setup function could help support that goal too. |
|
There are many core blocks that use the placeholder state. Let me share some examples so we have a better understanding of their complexity. Columns blockgutenberg/packages/block-library/src/columns/edit.js Lines 271 to 280 in 62b80f9 Query Loop blockgutenberg/packages/block-library/src/query/edit/index.js Lines 157 to 163 in 62b80f9 Gallery blockgutenberg/packages/block-library/src/gallery/edit.js Lines 412 to 414 in 62b80f9 RSS blockgutenberg/packages/block-library/src/rss/edit.js Lines 59 to 82 in 62b80f9 Table blockgutenberg/packages/block-library/src/table/edit.js Lines 343 to 345 in 62b80f9 gutenberg/packages/block-library/src/table/edit.js Lines 503 to 538 in 62b80f9 |
I'm speaking here from a pure design and user experience perspective, and I might be missing some nuance of the development side of things, so what I'm about to suggest might or might not be completely relevant. Nevertheless the
In many blocks, the In other cases, we use the Placeholder component even though we arguably shouldn't, for all the reasons listed above. For example the Table block allows you to configure the number of columns and rows in the Placeholder itself, but arguably we could just default to 2x2 and then let you change those from the block toolbar directly. This isn't meant as a critique of the choice, often the Placeholder was chosen because the block toolbar wasn't yet a good enough user experience. But it needs to be, and we need fewer of those Placeholders. In short, what we've learned from the user experience side is that designing a good block setup state requires careful consideration of its responsive behavior, consideration of whether it's even necessary in the first place, and thoughts about how to best pass the "squint" test. We might still be able to create some smart behaviors that handle these things in a mostly generic way, or at least provide props that help attune the component to its block type. I've revisited a few recently, though important to note I'm still not entirely happy with these:
But all that is to say: the Placeholder is not as simple as it might seem, and is likely going to vary a great deal from block to block. |
|
Thanks @gziolo and @jasmussen for the examples!
It seems that the placeholders are simpler than I expected—driven almost entirely by attributes and things tied to the block clientId. Only RSS requires internal block state, and that could easily be avoided by entering the url in the toolbar like the image url in the replacement flow currently is done.
Yeah, the design of the Placeholder is not simple—I don't think we could narrow down to even a few categories of placeholder to render. However, it seems that choosing if a block should be in a placeholder state is pretty simple—it's not so simple that we could encode it in JSON, but a single function seems like it could work. |
ntsekouras
force-pushed
the
try/add-in-placeholder-state
branch
from
9b40823
to
8faa838
Compare
|
I'm closing this for now, as this seems to be subjective about user's workflow. Might be better to address the shortcomings and usage of Placeholders in the first place, as explained very well by @jasmussen in the above comment:#33823 (comment) |
Description
Partially addresses: #23122
Related: #33466
There is the need to show/hide some controls or other things like
Stylesbased on whether the block is inPlaceholderstate or not. For example when we have anImageblock with no image set, there is no need to show theStylestab or theStylesinBlockSwitcher.This PR adds to the
block-editorstore aninPlaceholderStatewhich tracks which blocks are in such state. This value can be consumed to conditionally show/hide things. In order to be a complete solution forblock.supportsit will need one more property to the state persupport.For start I've implemented this for
Imageblock and will wait for feedback for the approach. If there is a consensus, I'll follow up on this by writing tests and add it to more blocks that can utilize this like Cover block(issue: #23198)Testing instructions
Imageblock and observe that if you have not set an image, theStylesare not shown.Stylesare shown.Checklist:
*.native.jsfiles for terms that need renaming or removal).