Add nested blocks inside cover block #13822
Previous attempt: #10639, #12405
How has this been tested?
I verified the block worked as expected.
requested review from
Feb 11, 2019
Feb 11, 2019
Technically this PR allows to nested multiple blocks, which is an enhancement in comparison to the previous version which allows using only text which would translate to a single block. It might be a good idea to allow multiple blocks but at the same time, this might be very close to how Section block is going to be shaped. We should decide whether we should leave it limited to only one block on top of the image or video. Fun fact is that implementation-wise it might be easier to allow multiple blocks to be inserted :)
mapk left a comment
The functionality works as I expected. The nested block limitations for the Heading, Paragraph, and Button blocks made sense.
The only problem I saw was the Heading block isn't centered vertically.
You can see this difference when I switch back from the Heading to the Paragraph. Can we get this aligned better?
@mapk, I think that's because there's an invisible default appender there.
If that's the case, we should probably clarify that. Maybe it requires the grayed-out "Start typing or..." text. Or perhaps some variation of the appender @jasmussen created for #10136 would make sense in this context?
Hi @mapk and @kjellr thank you for reviewing this PR.
But this message only appears if the user clicked and then did not wrote anything we can change to show the message all the time.
Independently of the path we choose we need to make sure we have different colors borders, backgrounds and text/icon colors with high contrast so if the user chooses a cover image/video with colors similar to one of our choices the user still notices the design element because of the other colors. Similar to what @jasmussen did on the block movers that now are always noticeable independently of the background.
I'm not sure about what is the best option to follow, and I would love some thoughts on this.
Part of what makes this block really shine is the ability to see it all together right in the editor. If the block appender is a part of that causing the heading shift too far upwards, then it causes a visual problem. But if there's a way to show the appender when the block is focused, and hide the appender when the block is not focused, that sounds like a plan!
In the case of which appender to show? I like the text "Start writing or type..." to be visible when the block has focus. Is the text have enough contrast?
The option of an outlined appender doesn't seem visible enough.
Any other thoughts, @kjellr ?
I think this is important to tackle in general as it affects all blocks with nesting, e.g. the Columns or eventual Container block.
I made #13571 suggesting some changes to the sibling inserter that could solve this problem.
This makes sense insofar as the selected block is okay to show additional UI. However it might be worth discussing this over a GIF, because there's a little more to it:
The cover title really is only offset if the text appender is there. And it isn't unless you actually press Enter first to create one. So I would argue that if the cover title is offset, it's because the user added a linebreak. Which they should be allowed to.
Going into review this, I was completely ready to suggest that nested blocks inside the Cover block might make less sense given we will have a Section block, as Gzregorz suggested. However, after trying this PR out, I'm not of a different mind again.
This is a cool block, it's a cool PR, and I love it for the same reason I love the Media & Text block. It's a highly specific block that's intended to be decorative. While the Section block might have overlapping features, that's okay, because the section block is likely going to be more advanced and complex to configure in other ways — for example the Section Block is not likely to have a placeholder/setup state, it's likely to just be a box that you configure from the sidebar. But the cover block starts with a setup state, it won't work at all unless you add a background image. We can consider it a "section block with opinionated defaults" — hey if we convert it to be that technically too, that would be fine. But the point I'm trying to make here is that it's helpful for a casual user to have this block, because it's a really easy way to create a decorative post. Let a thousand blocks bloom, it's okay that you might be able to accomplish the same with the section block. This one just makes it easier.
Going to the specifics of this PR, it feels really solid. The block even expands if you add enough linebreaks. I love it.
As far as I'm concerned, the challenge we have to solve here is that when the background is dark, it's impossible to see the block inserter.
Gutenberg already supports dark backgrounds on a global level: https://wordpress.org/gutenberg/handbook/designers-developers/developers/themes/theme-support/#dark-backgrounds — does it make sense to leverage those styles to invert the UI inside the Cover Block if a dark image is used? To keep this implementation relatively simple, it might be a matter of figuring out whether the overlay color is darker than
It fixes most of the contrast issues. Even if we can't guarantee contrast always — a user could choose a really sub-optimal contrast ratio themselves — this would be a vast improvement.
This is totally correct. After testing again, the appender didn't show up until I hit return. And when it did show up, I was able to hit delete in there to remove it. This seems reasonable to me. As long as the appender is visible (so that you know it's not just a weird empty space), I think we're all set.
This seems like a solid direction to me.
referenced this pull request
Feb 15, 2019
With the whole thing I wonder if it doesn't make more sense to work on Add Block: Section #4900. I have the feeling that a lite-section is being worked on here, even though the section issue that we've been wanting for such a long time has been left unfinished.
I don't understand why this is being given so much priority.
The other issue (#4900) would also solve this problem or make the cover block completely unnecessary.
And if I translate that into the Lego language: why don't we build the basic building blocks before we get to the trees, cars and bicycles?
Thank you all for the inputs
Hi, I will try to give a try to this solution proposed by @jasmussen. But there are some cases where I expect this solution to not work correctly, namely an image that is dark in some parts and very light in other parts e.g. some flags, and in videos that have frames that are dark and other frames that are white.
@jasmussen shared some thoughts around this subject:
I think the cover block has very specific functionality that the section may not have like the focal point selector.
Agreed with @jasmussen — this works well. I think there are minor refinements we can make, for instance, fine-tuning the appearance of the insertion point inserter on dark backgrounds:
Right now, the white background seems a little jarring to me.
But that's really minor, and can definitely be iterated on separately.
I did notice that on some screen widths, the inner block toolbars get cut off, but I'm not sure if that issue is specific to this PR or not:
Also worth noting: I think Twenty Nineteen may need a small patch to ensure front end text colors are working correctly with these new changes. I can get a patch going to fix that though.
Update: Turns out this Twenty Nineteen issue effected all nested blocks.