Block Selection Toolbar: Support fixed blocks by flipping toolbar when block goes out of view

[andrewserong]

What?

This PR explores whether we can update the Popover component and the block selection toolbar to support the toolbar displaying appropriately when using fixed or sticky positioned blocks (as in the exploration in #46142). This is achieved by flipping the toolbar to be below the block when a block scrolls out of view, to prevent the block toolbar from obscuring the block.

Why?

When support for sticky or fixed position is added to blocks, the block toolbar is not currently positioned correctly — it continues to scroll off the viewport as if the block were not sticky or fixed. Once the block toolbar is fixed in position, it then needs to be moved so that it does not obscure the fixed/sticky block content.

How?

• Popover: Add support for strategy in floating UI, which allows the consumer to set fixed or the default absolute
• For the block toolbar, use the fixed strategy, which appears to play better with fixed elements as mentioned in the floating UI docs: https://floating-ui.com/docs/computeposition#strategy
• Allow padding on shift and flip, and use this value to flip and shift the toolbar, while factoring in the height of the toolbar
• Update the logic in the useBlockToolbarPopoverProps to simplify things a little — here, we default to having flip always switched on, unless the block fills the viewport — this is a departure from trunk, but as described below, appears to be required in order to support sticky or fixed position blocks. I'm very happy for feedback on this PR, as it is quite a change — unfortunately I couldn't quite work out a way to support sticky/fixed blocks without making this change to toolbar positioning.

Testing Instructions

• Explore how the Toolbar is positioned with regular blocks that do not fill the viewport (should flip as you scroll down the page, if there is enough room to display the Toolbar beneath the block)
• Explore how the Toolbar is positioned with blocks that fill the viewport (should be sticky at the top of the viewport, as on trunk)
• To test in conjunction with sticky positioned blocks, this PR can be tested on top of Design Tools: Add a Position block support (including sticky), decoupled from layout #46142 — however, for the moment, it might be worth considering this change in isolation, and whether or not it is appropriate

Screenshots or screencast

In the below screenshot, a Group block that is set to "sticky" is selected, and its toolbar is flipped to be displayed below the block. It stays there while the viewport is scrolled. (Note: to test this, you also need #46142 applied)

Ignoring sticky or fixed position for the moment, the main change in this PR is that if there is no room above a block, however there is room below the block, then we flip the controls to display underneath. For blocks that fill the height of the viewport, then we retain the sticky behaviour from before. Overall, this is a change from the behaviour on trunk — I couldn't quite work out a way to support sticky/fixed blocks without this change, particularly since we also need to support the children of those blocks, so we can't directly inspect if the current block is sticky or fixed in order to enable the behaviour. Here are a couple more screenshots of the behaviour in this PR:

Enough room above	Not enough above, so flip to bottom	A tall block sticks to the top, as before

Screengrab (note that toolbar flips to be below the block when scrolling past the edge of the viewport, so that it doesn't obscure the currently selected block):

block-selection-toolbar-position.mp4

[codesandbox]

  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

[andrewserong]

Note: the logic here isn't quite right, as it currently alters the behaviour on trunk where for non sticky/fixed blocks, the toolbar stays over the block. In this PR, it appears that the toolbar flips too eagerly, so I'll do some more testing of this in the coming days.

[andrewserong]

Updated, and simplified the logic a little, so that it doesn't require a scroll handler. It is a bit of a departure from behaviour on trunk though, so might need some discussion to determine whether or not this direction is viable.

[andrewserong]

Just noting an issue that will need to be fixed up. In the site editor, the toolbar now displays over the top of the site layout header:

[andrewserong]

In the site editor, the toolbar now displays over the top of the site layout header:

It turns out this appears to be because in #44770 the use of the .interface-interface-skeleton__header classname for the edit site header was replaced with .edit-site-layout__header, but the edit site header didn't receive a comparable z-index. In e3b8c9c, I've added in a new z-index entry for the site editor, which appears to have gotten it working again.

[andrewserong]

@talldan I've just added you as a review in case you get a chance to look at this, since you previously improved the block selection toolbar logic back in #42887. Also @jasmussen just a heads-up, that I believe this PR (or one like it) will be a pre-requisite to landing sticky position support as in #46142.

This PR makes a few changes to attempt to deal with the use case of sticky / fixed position blocks, and the behaviour deviates a bit from what's on trunk. I'm very happy for any feedback or other ideas about how we might deal with blocks that are locked to the top of the viewport/editor canvas, and how to position the toolbar, if anyone has other ideas about how to achieve it! 🙂

[talldan]

As it changes a design element, it'd be good to get some design feedback (I've added the label).

The direction seem well thought through. The only other suggestion I'd have is that it may be possible to explore different toolbar positioning logic only for fixed/sticky blocks within the useBlockToolbarPopoverProps hook. It really depends on what the design feedback is though.

I haven't looked too deeply at the code right now. The change in 'strategy' seems like the main thing that might need some thorough testing. I've learnt there are generally a lot of corner cases to consider with the Popover component 😄

When manually testing the main problem I'm seeing is that site editor toolbars now seem to be positioned on the right instead of the left:

The widget editor also has header issues (might be similar to what you experienced in the site editor):

[jasmussen]

Thank you for thinking ahead, this is an important piece to get right. It's already partially at play for top-aligned blocks in the site editor, such as a header, and it would be nice to have more of a clear system for when this invokes. Fixed-to-top blocks, for example, feels like an obvious use case for this.

Two things about the current implementation, though: I think that in most cases we should not be flipping the position of the toolbar if it's able to start out above the block. Specifically, the act of flipping the toolbar feels a bit jarring, especially for editing text, as shown in this GIF:

So in the case where it starts above, I'd keep the "sticky" behavior. Secondly as is slightly evident in the GIF above as well, it feels like the position of the toolbar is a bit more jittery and elastic in this PR compared to trunk, where the toolbar is locked solidly in place even when scrolling:

In other words, for any blocks that are normal flow blocks, I think we should keep the current block toolbar behavior. Issues of it covering content, I think we can improve by fading out or hiding the toolbar in more cases. At the moment, it disappears when typing, but it could also fade out when hovering various elements, such as sibling inserters, items in the inspector or toolbar, or otherwise in performing actions that do not require the toolbar. And of course, improvements to the top toolbar behavior will bring additional options.

But then for blocks that are fixed, or which start at the top edge of the screen and the block toolbar isn't able to be shown above, I think we can embrace this flipping behavior. Figjam is a good example of how this might work:

What do you think?

[andrewserong]

Thanks for the comments @talldan and @jasmussen, that's just the kind of feedback and help I was looking for! This points to needing to fine-tune the logic, and add a little more smarts.

The widget editor also has header issues (might be similar to what you experienced in the site editor):

Oh, I totally forgot about the widget editor, I'll check it out and see if it too needs a z-index. Thanks! 👍

I think that in most cases we should not be flipping the position of the toolbar if it's able to start out above the block.

This might be a good clue as to how to get the logic to work. I don't think we can reliably test whether or not we're currently dealing with a fixed/sticky block, because we could be dealing with a direct child (or nested child) of a fixed/sticky block, however seeing if there's space when the toolbar is initially rendered could be a good way to identify when to switch on the flip behaviour so that it's a little less greedy. I'll dig into that idea.

it feels like the position of the toolbar is a bit more jittery and elastic in this PR compared to trunk

Good catch, I'll see if I can figure out what's going on there and why the changes here introduce that jitter (and whether it's our code in Gutenberg or a peculiarity of floating UI).

What do you think?

Great feedback! I think overall, it sounds like, "yes, flipping for fixed/sticky blocks is a good idea, but let's make it less greedy and fix the jittery-ness". I'll dig in 🙂

[andrewserong]

Update: the widgets header was a simple fix (turns out it didn't have a background color set for the widgets page, so I've added one in f52091e).

The other issues will need a little more digging, so I'll continue exploring through the rest of the week, and play around with the logic. I haven't been able to reliably reproduce the jittery movement when scrolling the page, but it does appear to happen sometimes, and then also sometimes resolves itself when I click away from a block and then back to it. So it's a little challenging to figure out the cause, but I'll do some more digging!

[andrewserong]

I haven't been able to reliably reproduce the jittery movement when scrolling the page, but it does appear to happen sometimes

[andrewserong]

I haven't been able to reliably reproduce the jittery movement when scrolling the page, but it does appear to happen sometimes

I haven't gotten much further to a solution yet, but I can reliably reproduce it within the first few seconds of selecting a block directly after the block editor loads. After a little while of scrolling the page, it seems to then stick appropriately to the block position for me. I initially wondered if there was a conflict with framer motion's animation of the popover position, but after trying skipping that in the Popover component, it doesn't look like that was the culprit. I'll continue digging in next week!

[andrewserong]

I'm wrapping up for the year shortly, but I thought I'd push an alternate PR as another option to this one. I've opened up a separate PR over in #46565 that attempts a slightly simpler approach to see if that would be viable instead. Some dot points:

• The new PR explores retaining the absolute strategy, and instead forces a re-render whenever the scroll container is scrolled. This appears to avoid a couple of the issues with the fixed strategy that we've run into so far — the jitteryness for non-fixed blocks, and that the fixed strategy meant that we needed to keep track of whether or not the toolbar was going outside of its clipping area.
• The new PR also updates the logic that determines whether or not to flip the toolbar to be beneath the block — it will determine the flip setting when the block is initially selected based on whether or not there is enough space at the top of the visible area of the editor canvas. That seems to fix the issue for sticky/fixed blocks while being less greedy overall.

I'll leave that there for now, but will pick all this up again in the new year. Thanks again for all the feedback!

[jasmussen]

Have a good break!

[andrewserong]

It turns out the larger changes in this PR are no longer required thanks to an improvement to how the popover component is rendered that landed in #46187. We still need to update the flipping logic, so I've reduced the scope of #46565 to solve just that part of the problem.

I'll close out this PR now.