Allow insertion of elements before and after a block, when the default block is not supported in the parent container #29490
Conversation
|
Size Change: +148 B (0%) Total Size: 1.06 MB
ℹ️ View Unchanged
|
gwwar
force-pushed
the
add/navigation-insert-before-after
branch
from
db1b5ab to
fd6bb5f
Compare
gwwar
requested review from
ajitbohra,
chrisvanpatten,
ellatrix and
mkaz
as code owners
talldan
added
[Feature] Blocks
gwwar
force-pushed
the
add/navigation-insert-before-after
branch
from
dc9cbc6 to
5274a89
Compare
|
Thanks for the great review @talldan! The summary has been updated with new behavior changes. I tried applying new block defaults to buttons and social links too. Buttons works well, but I noticed that social links in trunk doesn't allow for a variation transform. I played around with social links variation transforms in 7e7c82a but it feels a little awkward. I think we might want to leave enabling this for social icons in a follow up PR. social-link-variations.mp4cc @jasmussen not urgent, but if you wanted to 🤔 about the many variations case. |
|
With regard to the many-variations-issue, maybe clicking the menu items should open the same block library pop-up that the sibling inserter uses? |
|
Good point about Social Icons, I didn't think about that. Definitely not a blocker, this PR can ship without the social icons changes since it's a really good improvement for Buttons and Navigation. The social icons part could be moved to a separate issue/PR. |
Nice work. This is what I see:
Yep, the variations issue is a challenge. As you can see in this GIF I start by inserting 2 page links, i.e. this:
but the before/after controls only allow the insertion of "links", i.e. this:
I know the difference is subtle, and the latter allows linking to the former type. But given it's a challenge we need to tackle for the social links block, it might be worth tackling here. Zebulan mentions it. Perhaps we need to show the quick inserter when choosing the before/after options (when there are more than 1 options to insert). It seems like we might be able to leverage the vertical sibling inserter pattern for this:
That is, not necessarily use the same component, but the blue line that shows up in the margin between horizontal blocks, it could show up as an anchorpoint for the block library. As in: when you click the "insert before" menu item, it will be as if you clicked the sibling inserter between the two items. Does that make sense? Note, the vertical sibling inserter doesn't work in the navigation menu item quite yet, that's a separate issue (#28833). |
gwwar
force-pushed
the
add/navigation-insert-before-after
branch
from
7e7c82a to
3e97524
Compare
|
The vertical inserter looks pretty promising @jasmussen @ZebulanStanphill. How would folks feel about me enabling this for just buttons and navigation, and I can immediately follow up on the variations+vertical inserter case. I'm a fan of having multiple ways of doing something, so I think this is still a good improvement. cc @talldan |
The current behavior where it only inserts a single variation? Could probably be a fine start, but I'd default to "Page Link" rather than "Link" as it does now, if you can. |
🤔 This one is a little interesting since a page link is a block variation of the navigation link, rather than a block. So instead of passing a string we might send an array, similar to block-templates: __experimentalDefaultBlock: [ 'core/navigation-link', { type: 'page' } ],Currently Happy to explore passing through block attributes too, if folks think we'll use this. |
The inserter has a heuristic where if only a single block is available to insert, such as inside Buttons, it skips the popover menu and just inserts that one allowed block directly. It makes perfect sense to pair this "insert before" with Buttons for that reason: it's totally obvious what you're inserting, because it can only be a Button. In the case of the Navigation block, it's not obvious. It could be one of the menu item blocks, it could be "Page List", it could be social links, or search. And you can't really transform one into the other after the fact. So it seems we probably need that block library to open in that before-slot, as if you'd clicked the vertical sibling inserter. Though it seems fine to keep the PR small and default to our "best guess" for what you want to insert (probably a page link), but we'd want to follow up on that, I'd think. |
gwwar
force-pushed
the
add/navigation-insert-before-after
branch
5 times, most recently
from
4e5f6e4 to
f9c6287
Compare
|
cc @jasmussen @talldan @ZebulanStanphill Do folks have preferences on which approach to explore further? I think adding Besides that, the main feedback I'm looking for is if something like adding an editor only block, like Update __experimentalDefaultBlock to pass through block attributesIn f9c6287 I updated __experimentalDefaultBlock to also allow passing through of block attributes: This lets us do things like insert a page link vs a generic link. navigationpageinsert.mp4Experiment with hijacking insertion place inserterBasic idea on this approach is to force the insertion point inserter to open the QuickInserter on "insertBefore/insertAfter" I didn't explore this too far. I could maybe get this working, but it felt really buggy. Some issues
Experiment with leaving a placeholderI also did a very rough experiment in 4e5f6e4 that lets us add a placeholder block to retain space, and pick a block to replace itself at a later time. (This doesn't render anything on the published view). The inserter code is pretty tangled together, so I opted to just punch a hole through to see how this feels. Everything on that commit is throw away. 🙈 Some problems that stand out from this:
PoC.mp4 |
|
🤔 I can also explore setting some internal state on the block list for showing an inserter. |
|
Just pushed up a half example of the "show more" case. The inline quick inserter part is a bit trickier, but I'll maybe try this approach again. There's a good amount of component nesting for the inserter, so the smallest solution here will likely involve adding block-editor store actions. more.mp4 |
|
This is cool work, thanks for the hard work. I think both the proof of concept in #29490 (comment) and the smaller solution in #29490 (comment) can work, the latter perhaps more as an interim. That said, it seems like this is a complicated and difficult task, which while something we need to address, is perhaps not as high a priority as the others on the tracking issue (#27593), so I've lowered the priority there. So in case this keeps being a headache, I'd personally think think it'd be fine to shelve it for a bit and grab other tasks. |
|
The different experiments are really great to see in action. For For the other cases (navigation and social) where there might be multiple blocks/variations I also like the idea of showing the inserter menu (whether the full menu or the quick inserter), though from what you say, seems like a longer term option. |
|
Something else I just spotted. The 'Insert Submenu' button for the Nav Link block suffers from a similar issue. It just inserts the default Link block as a child. |
@jasmussen agreed!
Thanks @talldan I'm starting to come around to this idea too, especially if we choose not to go with a placeholder block approach. The scope of the inserter approach is a bit bigger to explore, so I'll return to this one after we close out a few of the more urgent Navigation issues. This has been a fun exploration, I have certainly learned a lot more about the block list and inserter! Setting this one back to draft, but other folks are welcome to take this on earlier in the meantime. |
|
closing this for now |
…w insertion of elements before and after a block, when the default block is not supported in the parent container.
gwwar
force-pushed
the
add/navigation-insert-before-after
branch
from
4628bb5 to
d283507
Compare
|
In working through #34899 I tried default inserting a Page Link block in the Navigation. The problem is that Page Link limits the links we can insert to pages, which, while it might be the majority of cases, will be very frustrating if we're looking for something else. Whereas if we insert a default Link block, the search brings up everything: pages, posts, categories, and also allows us to insert a custom link. I think we have the same situation here, and given that in #35056 we're also looking at reducing the exposure of different link variations, and in #34899 we're looking at simplifying the flow for inserting Nav links, it makes sense to insert a default Nav link here instead of going for the quick inserter. We could possibly tweak the suggestions in the link control to prioritise pages, but being able to search for any entity makes it much more flexible. The Social Icons problem is a bit different from Navigation, because there we don't expose a default icon type that allows us to search for any social link; we only show the different link variations. This makes sense, because unlike nav links where any old URL and label combination is valid, social networks are a limited set of items; we can't make one up. So if we were to enable insert before/after for Social Icons, it would probably be best to pop up the quick inserter in place instead of inserting a link directly. Imo, the benefit of doing this for Social icons might not be worth the extra work, because it's super easy to just append an icon to the end of the block and then use the move buttons to place it elsewhere. In any case, we can leave them for later 🙂 |
Fixes #23603 where we'd like to insert additional links in navigation links before and after items.
In trunk, "insert before"/"insert after" is only allowed when a parent block supports insertion of the default block (typically this is a paragraph block).
Proposed changes here add a
__experimentalDefaultBlockoption to block list settings. If this is set, we can choose a different default block in allowedBlocks for that parent container.Changes also includes adding defaults for:
If folks like this approach, I can add an e2e test in a follow up. See #29543
defaultinsert.mp4
Testing Instructions