Blocks: Improve UI for all Inspector Controls

[gziolo]

Description

Follows up #5952.

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).

This PR also improves the default view of the Custom Size slider:

Before:

After:

How Has This Been Tested?

Manually.

Checking the diff of this PR is much easier without whitespace changes: https://github.com/WordPress/gutenberg/pull/5995/files?w=1.

Screenshots (jpeg or gifs if applicable):

Paragraph

Latest Posts

Shortcode

List

Types of changes

UI improvements.

Checklist:

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

[jasmussen]

Beauuuutiful. So many little things that just improve the whole picture. Thank you.

To me this looks completely shippable as is, experience wise. Code looks good too, though I see a failing test.

[gziolo]

I think I might introduced some regression when uodating RangeControl. I will investigate

[gziolo]

It was a different issue. In line https://github.com/WordPress/gutenberg/pull/5995/files#diff-9e70015597c35b4faecd9a6beae81344R245 fontSize was applied also for the default font size which isn't necessary.

[gziolo]

Let's get this UI fixes for the 2.6 release.

[paulwilde]

Although I like the changes, this has had an impact on my implementation where I've been adding additional controls to core blocks.

As an example with the 'Advanced' Settings disabled (Custom CSS and Anchors), I have added 3 additional controls to the Heading Settings. The last 3 at the bottom:

However, when the panel is closed, these additional settings are no longer contained within the same panel:

I don't want to be limited to where I need to create a separate expandable panel for very basic additional settings such as text colour as it adds more weight to the UI. I'd want them to live in the same main 'Heading Settings' panel.

As there is currently no documentation on how controls can be added to existing blocks I used the implementation that the Custom CSS and Anchors uses in order to learn on how to add these additional controls. Maybe its possible to target the controls to be rendered at the bottom of the 'Heading Text' component, although I couldn't figure it out.

For clarify sake, here is what I'm doing to add new controls:

function onUpdate( BlockEdit ) {
    const WrappedBlockEdit = ( props ) => {
        if ( 'core/heading' !== props.name ) {
            return <BlockEdit { ...props } />
        }

        const { attributes, setAttributes, isSelected } = props
        
        return [
            <BlockEdit { ...props } />,
            isSelected && <InspectorControls key="inspector">
                <ToggleControl
                    label={ __( 'Custom Control' ) }
                    checked={ !! attributes.customControl }
                    onChange={ onChangeEvent }
                />
            </InspectorControls>,
        ]
    }
    return WrappedBlockEdit
}
addFilter( 'blocks.BlockEdit', 'custom/heading/update', onUpdate )

[jorgefilipecosta]

We are setting the default size for font Size as regular making the range control start in some value. But we don't apply the regular class.
This may cause problems, imagine for example I set a CSS rule for paragraphs nested inside the cover image to have size 20px. These changes make the default size for paragraph appear as 16px but we don't set the class so the size will be in fact the one set with CSS for paragraphs inside cover image 20px.
Essentially I think there is a difference of state between not having size specified, no class et all where we use the default css rules depending on the context, and having regular font size where we use the regular class.

[gziolo]

This may cause problems, imagine for example I set a CSS rule for paragraphs nested inside the cover image the have size 20px.

Yes, I didn't think about that. Good catch. If you use fontSize or customFontSize prop it won't be an issue. Updating CSS is tricky. I got your point though.

Any idea how to move the slider to the proper position without updating the state then? :)

At the moment it's a very bad user experience when you start updating the font size and it gets much bigger than it initially was.

[jorgefilipecosta]

I agree the solution we had is not perfect, I think a possible improvement maybe use getComputedStyles to try to understand the actual size of font, and position the initial range slider position in that value, but without setting the value itself (we would have an initial position prop in the slider to set it to initial value when no value exists) the value would only be set if the user changed it. So we would still have a difference between the state user explictly set some value or no value was chosen.

[gziolo]

I opened PR to revert this change. This is more complex than I expected and not that important to spend a significant amount of time on it. See: #6075.