Editor: Bring back Inspector Advanced Controls

[gziolo]

Description

Fixes: #5884.
Related PR: #3475.

From @DannyCooper:

Has it been considered to visually separate the 'Additional CSS Class' setting from the above Panel? By default, it could be mistaken as part of the previous panel.

From @mtias:

It was supposed to be inside an "Advanced Settings" panel, together with "anchor" setting.

I tried to do the similar in #3475 but we decided to defer this decision. This PR introduces revised changes to make it happen.

This PR also introduces createSlotFill factory method to simplify the process of creating a pair of Slot and Fill components.

How Has This Been Tested?

1. Open Gutenberg editor.
2. Add a Paragraph block.
3. Open block settings.
4. Make sure there are no advanced settings.
5. Add a Heading block.
6. Open block settings.
7. Make sure there are advanced settings in the collapsed state.
8. Make sure they open and work properly after you click toggle icon.
9. Add an Image block.
10. Make sure there are advanced settings in the collapsed state.
11. Make sure they open and work properly after you click toggle icon.

Screenshots (jpeg or gifs if applicable):

Before

Note: this screenshot is old but should be enough to show the difference :)

After

Note: this one is up to date 👍

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows has proper inline documentation.

[gziolo]

I added missing documentation and unit tests.

[mtias]

@jasmussen does this look good to you? I believe we discussed it before we just never got around to doing it.

[jorgefilipecosta]

Minor: Given that we are changing this zone, hasAnchor may be confusing has it includes the isSelected condition. Maybe we can have a condition that checks this hasBlockSupport( props.name, 'anchor' ) && props.isSelected and if true returns what the fragment we have without other checks.
Otherwise we just return <BlockEdit { ...props } />.

[gziolo]

Updated, good idea. Thanks 👍

[jorgefilipecosta]

Nice helper method 👍 It simplifies a lot the slot fill creation.

[jasmussen]

This looks good to me except for a few small things.

The margins on the top panel collapse now causing the text settings header to be a bit snug up against the border:

I think this is due to the addition of a div to group all the non-advanced content. But this div allows margins to collapse, because it has no border and no padding. You could hack your way out of that, but given another change I'd love to see, perhaps there's a different approach. More in a second.

Can you add a top border to the Advanced section, so it looks like this?

As you can see, there's a "separator" between the block type description at the top, and the advanced section below. Right now that description has a border, and in the mockup above I added a border to the bottom section as well.

If you remove those borders, and instead create a "separator" component or something in that vein, we can insert that between divs, and not have to worry about collapsing margins. Make sense? Otherwise hit me up on chat and we'll walk through it.

Nice work! 👍 👍

[jasmussen]

With latest changes, looks good designwise 👍 👍

[gziolo]

I applied the following changes:

• Fixed the regression in the master with 78cbb9a and rebased branch.
• Added border to the Advanced panel as suggested with 0d9961d.
• Refactored hooks to work as @jorgefilipecosta recommended with 36ca30c.

[jorgefilipecosta]

Thank you for your @gziolo, the code looks good to me and is working well.
I just noticed a possible improvement, that in my option does not block the PR. When the block has advanced controls but hasn't normal controls we show two bars:
.
The fix for this is not easy because even when normal inspector controls and advanced ones are empty, we render empty container divs, so using nth-child or first/last is not possible. We can either remove this container divs if possible. Or add classes to them with empty/not empty state and we could add the border when advanced controls with content follow normal controls with content (using + selector).

[jorgefilipecosta]

There is also a pure CSS solution which should work well which is use the selector
.edit-post-block-inspector-panel > div:nth-child(2):not(:empty) + div:not(:empty) to apply the border. The big downside is that it assumes that normal inspector controls are the second div and advanced controls are the third div, which sounds hacky.

[gziolo]

@jorgefilipecosta, good catch. I think we can also try to apply border-top to the inspector controls instead of having border-bottom applied to the block details. I'm wondering now what happens where there are no controls at all :)

[gziolo]

I will open a follow up because I have a fix that touches a half of the core blocks. The good part is that it greatly improves UI for all of them 👍

[aduth]

Why do we filter here?

[gziolo]

To get rid of empty elements. Some fills render nothing based on Redux state. If you want to prevent rendering a slot wrapper it it has no fill which render something this is the way to go. In retrospective, I think this could be also done outside. Should we move it out or improve here and add some comments?

[aduth]

Comments would be good.

It was also strange to me that we filter only if the children is passed as a function, but not otherwise.

[aduth]

Also, filter( Boolean ) doesn't sound like a sufficient way to imitate React's treatment of "empty". For example, a child of 0 would be omitted here, but is a valid / non-empty child to render.

[gziolo]

Yes, I will open PR with all your comments addressed.

[gziolo]

Done #9371.