Allow the column block in the inserter #20502
Conversation
youknowriad
requested review from
ajitbohra,
jorgefilipecosta,
Soean and
talldan
as code owners
youknowriad
force-pushed
the
update/allow-column-in-inserter
branch
from
fe8729a to
2ccb0e4
Compare
|
Size Change: +238 B (0%) Total Size: 821 kB
ℹ️ View Unchanged
|
|
At a glance, it seems reasonable to me. It raises a question to me whether we even want the "Columns" slider at all, if we're starting to go down this path of letting users rearrange, add, and remove columns in-place. |
|
Yeah I had the same though but adding columns with the inserter is still a bit painful (you need to have the "columns" or "column" selected. There's also something else, the range is limited to 6 columns I believe, do we need a "maxInnerBlocks" API or something? or is it ok to keep it as is. People can still add an illimited number in HTML. |
|
That's an interesting point to highlight. It feels like something where, if we feel the need to have some restriction, we would want to allow blocks supports to be changed dynamically†. So, for example, the Columns block could dynamically set † Maybe it already is? Is there something to dispatch to the |
I'm pretty sure it's possible but we don't have dedicated actions. |
|
Actually, no, it's not possible because we only want to do this change of a single instance. |
|
Ah, I see how that's an issue. I could possibly imagine some hierarchy supporting optional per-instance block supports, but aside from this use-case, I'm not sure how useful that would be. |
|
I tested this and it feels a little strange to me. Intuitively, I felt that I would add a Column to a Columns block. Accordingly, I selected the Columns block and clicked the inserter, but was presented with all block options, and not the Column block. Reflecting on this and how the inserter works in general (you're inserting relative the selected block in the context of the parent or root), I understand the technical reasons for why this is. But I wonder if my experiences will be a common source of confusion. On the other hand, it could be argued that even if it's not optimized to insert this way, there will always be an inserter for when the Column is selected, so it may as well present something useful, rather than "No blocks found" (#19354). In that sense, I think it can be an okay change. |
There was a problem hiding this comment.
Code-wise, this looks correct. I'll leave it to your discretion on whether to proceed, given the thought process I shared at #20502 (comment).
I agree with this confusion and I had a similar feeling. The inserter works consistently no matter which block is selected: "insert after the block". We could potentially consider changing that into:
Even with this, there's a potential for confusion though. |
|
Together with @koke, @pinarol and @iamthomasbishop we were discussing how to approach Columns block on mobile. This PR help us a lot to handle Inserter behaviour. However I also wonder about other two related topics.
What do you think about this ? |
This is already the case. |
|
Ok, great ! Thanks for answer |
|
If we want to limit the column number to the 2-6 range, then any sort of direct manipulation makes the UX confusing. We could allow inserting columns and have that new API to limit the number of child blocks, but then it would not be clear to a user why sometimes they can insert and sometimes they can't. Similarly, if there are two remaining columns, it wouldn't be clear why they can't remove columns now. The slider might not be the most obvious way to manipulate the column number, but at least makes these limits explicit. I'm wondering if there's a way to relax that restriction, making the 2-6 range more of a suggestion than a limit. That way, you would be able to insert as many columns as you like, and remove them all, leaving an empty columns block that would show the initial placeholder.
We came across another potential use case in wordpress-mobile/gutenberg-mobile#1607 (comment), where it would be interesting to refactor media-text so it uses inner blocks for the media part. I believe the current API doesn't allow both limiting to either image or video, and a maximum of one child block. You can either lock the template to an explicit block, or allow multiple children. |
It seems that the max limitation is not necessary - it seems you can put as many Column as you want. Avoiding minimum Column limitation is more tricky because at first glance it was difficult to me to think about Columns with only one Column inside. However second thought was that there might be some use case and user would like to have Columns block with only one Column so why we should block it ? If we're going to remove the limitation I also wonder do we still want to show slider in Sidebar settings ? |
youknowriad
force-pushed
the
update/allow-column-in-inserter
branch
from
1959715 to
d69b767
Compare
|
Added a warning message if the columns count exceeds the maximum. What do you think @aduth ? I believe it's a bit better than trying to force a columns count. |
|
Taking a fresh look at this, I find the previously-noted concerns interesting to consider. I agree that it feels odd to allow for this sort of direct manipulation where a "cap" exists (even if it's now a soft-cap). For me, I think a cap could exist, but the current options for giving feedback to the user are non-ideal. I think the ideal case for me would be one where maybe the inserter option becomes grayed out with an inlined or tooltip message about how the maximum number of columns has been reached. It's obviously a bigger task than we might want to take on here, but it could be a useful inserter feature generally for blocks to leverage. The notice is okay as far as providing feedback, and there's some prior art here with things like the Contrast Checker (give a user the freedom to make a poor choice, but inform them about it). But it could be strange that we only inform a user of a mistake after the fact (could be missed). I guess it comes down to a question of "How bad of a problem is it?". It may not really be that big of a problem for there to be many columns. Or at least, the user can use their own judgment to decide based on how it appears on their site. I think the options I see here are one of:
Might also be good to get some design/usability feedback as well. |
One of the reason I don't want to abandon the PR is because it will allow the inbetween inserter to trigger to add columns when we introduce the horizontal alignment support. I believe @ellatrix is working on some of that. |
Co-authored-by: Andrew Duthie <[email protected]>
|
Yes, I've been thinking it could be useful to have the block list appender visible for columns. Not sure what's the best conditions for it to be visible. When the last column is selected? When any block is selected? When the inner block list is selected as well? We also need to think about this together with other blocks like the navigation block, social icons, buttons... which should have the UI. |
|
Let's give this a try, it's easy to revert anyway. |
closes #19354
Since the "column" block already has a "parent" property defined, we should just let it appear in the inserter if a sibling is selected, allowing you to insert another column to the columns block without using the "columns count" range.
Thoughts?