Jetpack: Video Block - Upgrade notice broken on Gutenberg v8.1.0

[Addison-Stavlo]

Steps to reproduce the issue

1. Using a 'Free' plan site with gutenberg-edge sticker (for v8.1.0), visit the editor.
2. Insert a video block.
3. Notice the banner has disconnected from the block and the 'upload' button is enabled.

What I expected

[ockham]

@roo2 found this change on the Gutenberg side that seems to have caused this.

I think the problem is specifically that the VideoPress block wraps around the core/video block, and the latter's new extra div wrapper is messing up the styling.

[apeatling]

I'm not seeing any way to easily fix the notice placement issue with the new wrapper. :/

The problem is with this new block wrapper, the notice is rendered outside of the main block div, without any way of getting it back inside via our block wrapping code.

[apeatling]

It's possible to hack it visually with CSS, but I suspect this to be incredibly fragile and not a good idea.

[scruffian]

The UpgradeNotice was meant to be following the same patterns as other notices in Gutenberg - how do other warning notices look in this block with this change?

[creativecoder]

That <Block.div> change comes from an "experimental" component. Let's ask to have it put it back the way it was!

[apeatling]

Left a note on the core issue to see how we can proceed. I don't have any suggestions there at this point, but Riad may have some thoughts.

[apeatling]

The UpgradeNotice was meant to be following the same patterns as other notices in Gutenberg - how do other warning notices look in this block with this change?

Those looks fine and continue to display within the placeholder.

[roo2]

We've noticed that after uploading a video, the video block cannot be selected in 8.1.0. I wonder if that is related p1589943272033100-slack-create-gardeners

[ockham]

That <Block.div> change comes from an "experimental" component. Let's ask to have it put it back the way it was!

Andy asked here: https://github.com/WordPress/gutenberg/pull/22028#issuecomment-631145184
Riad's reply: https://github.com/WordPress/gutenberg/pull/22028#issuecomment-631357407 (tl;dr unlikely to be rolled back)

[ockham]

We're reverting this Gutenberg commit for WP.com as a temporary fix (D43703-code).

However, we'll need to sort this out in Jetpack since that change is here to stay, and the issue would break the video block on self-hosted sites with Gutenberg >= 8.1. Riad left some pointers on how to fix.

[apeatling]

@ockham Based on this being in your backlog now, are you taking this change on in the Create Focus?

[ockham]

I added it to our maintenance board to keep track of it (but wasn't planning to work on it myself). We should get it sorted out tho, I realized that we basically need the Jetpack fix for Atomic sites. @apeatling Do you have any spare cycles to work on this? Otherwise I can look into it.

@youknowriad Do you have some additional pointers for us how to best tackle this within the light wrapper setting? Unlike the block toolbar (that uses a Popover, as you've mentioned in your other comment), the upgrade nudge is supposed to take up some vertical space above the block permanently (well, until the user upgrades their plan).

cc/ @roo2

[youknowriad]

@ockham I think we need some explorations here. The Popover component can potentially be used to cover the anchor I believe using maybe getAnchorRect. that said, it's not something I've tried, the Popover component also changed a lot that I don't know how it works entirely at the moment.

If it doesn't work, or it's too complex, I think it's reasonable to consider a dedicated API to add "children" or content inside the block wrappers which could be used to add an absolute positioned element right.

[ockham]

I played with this a bit. The Popover approach looks very complex (potentially a lot of manual calculations of dimensions, and I'm not sure it'd give us more than the div we have now). I might've found a simpler way tho :slightly_smiling_face:

[ockham]

Fixed by #15883 and #15906.