Full Site Editing: Refactor the Template block UI

[Copons]

Changes proposed in this Pull Request

Requires Automattic/themes#1262

Refactor the Template block style, and changes the block selection behaviour.

• Drop all the unnecessary rules that were left over during the months of development back and forth.
• Hide all the block's UI component that aren't used by FSE (breadcrumb, toolbar, borders).
• Move the theme classes (e.g. .site-header) from the block most external wrapper, to one more inside. This should simplify the overrides, as now the theme styles are only applied to the rendered part of the block, and not to its wrapper too.
• And much else! (I'll add to this list later)

SCREENSHOTS GALORE!

Show

Clean Slate
Note: the colophon positioning is off compared to how it's supposed to look in the theme (which is a bit awkward, what with calculated margins that attempt to almost center position it, instead of just text-align: center it and call it a day. tl;dr it's technically text-align: left, but looks more centered)

Where	Current	Refactor
Header
Footer
Page
Front End
Theme Demo

Header with Logo
Note: the header elements spacings are off in both current and refactor, compared to how the theme is supposed to look.

Where	Current	Refactor
Header
Page
Front End
Theme Demo

Header with Full Width Image
Note: in both current and refactor, on the front end, the image has a negative right margin that triggers the horizontal scrollbar.

Where	Current	Refactor
Header
Page
Front End

Footer Alignment
Note: imho this has a very bad UX.
Blocks have an alignment option, but then some of them are forced with a predetermined alignment. Imho, we should not force any alignment via CSS, but instead letting people do their thing with the editor.

Where	Current	Refactor
Footer
Page
Front End

Selecting a Template Doesn't Hide the Sidebar Anymore
Note: the sidebar stays open (if already open), and the "Block" tab gets hidden.

Current	Refactor

Testing instructions

• Build the Modern Business theme with Modern Business: Refactor the Full Site Editing styles themes#1262.
• On an FSE site, edit a page.
• Try poking the header and footer template parts, and make sure that:
  • The Template block overlay is properly aligned to the editor frame.
  • The overlay retains the same background color and blur filter as before.
  • Selecting (clicking) a Template block does not close the sidebar (if it was open), but simply hides its "Block" tab.
  • Everything looks fine!

Fixes #35378

[matticbot]

Test live: https://calypso.live/?branch=try/fse-polish-templates-ui

[matticbot]

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

[gwwar]

@Copons do you mind adding a before/after screen or gifs?

[gwwar]

Can we see if we can add a better selector instead of targeting last-child with the addFilter hook. See example:

wp-calypso/apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/post-content/index.js

Line 32 in 8e51dbe

const addContentSlotClassname = createHigherOrderComponent( BlockListBlock => {

We can also query block selection state, instead of debouncing this to apply another class.

For example we can probably keep track of https://github.com/WordPress/gutenberg/blob/15f5ff44609df48a47dee1d3e750470cdda50a5e/packages/block-editor/src/store/selectors.js#L391 and setup a boolean to match when there's a selected block + it's a template block.

Feel free to ping me if you need more info on ^^

[Copons]

I haven't found any filters on the external editor wrapper that could be used for this, but only ones deeper in the DOM, closer to the blocks list (e.g. editor.BlockListBlock, it targets the actual block, even if the name could be misleading).
In this case, we would need, for example, to add a .is-template-selected class to, say, the body, the editor wrapper, or anything outside the blocks list, in order to target the sidebar.

The :last-child in itself should be safe enough, considering that the sidebar header is an hardcoded list with only two elements (Document and Block), and we always need to hide the second one.

Though, maybe I'm either overthinking this or totally misunderstanding your suggestion. I'll double check if I've missed something obvious, or maybe do some lateral thinking, or eventually just ping you 😄

[gwwar]

It doesn't look like there is a good alternative for the last-child selector, but we can listen to the edit-post store and block-editor store:

So sketching this out roughly in code this might look like:

import { withSelect } from '@wordpress/data';

//wrap an appropriate component
withSelect( ( select ) => {
    const selectedBlock = select( 'core/block-editor' ).getSelectedBlock();
    const a8cTemplateSelected = selectedBlock && selectedBlock.name === 'a8c/template';
    const blockOptionsSelected = select( 'core/edit-post' ).getActiveGeneralSidebarName() === 'edit-post/block';
    return {
        'hideBlockSidebarTab': a8cTemplateSelected && blockOptionsSelected,
    }
} );

Two options then on how to use it, if we are able to add a modifier class in the right spot, we can add a classname + CSS

//in an appropriate component render, let's add another classname:
render() {
    const { hideBlockSidebarTab } = this.props;
    return <container className={ hideBlockSidebarTab ? 'my_container.isTemplateSelected' : 'my_container' } />;
}

// then in CSS add a selector that targets this state to display:none;

Otherwise we can try and use https://reactjs.org/docs/hooks-effect.html to detect this transition state from false/true and trigger the CSS toggle on blockSidebarButton

[Copons]

@Copons do you mind adding a before/after screen or gifs?

Initially, I had intentionally not added any screens as the idea was to have no visual changes between master and this refactor (with the only exception of the sidebar not hiding anymore when clicking the template), but only a tidier code.
Though, subsequent iterations resulted in some "free" fixes, so once I'll rebase I'll also add some before-after screens to make it clearer what's going on.

EDIT: @gwwar I've just added a gazillion of screenshots! 🕺

[gwwar]

Thanks @Copons I'm not sure if I've verified all areas that has changed but I do appreciate the smaller line count. I've smoke tested this a bit with Automattic/themes#1262

Let's get these two in, and then tidy on anything we might have missed. In the future let's try to aim to split out janitorials from feature changes as it's easier to review/safer to land.

I would still recommend getting rid of the setInterval in favor of listening to the Gutenberg redux stores, but I'd be okay with that being an immediate follow up as well.

[Copons]

Thanks @gwwar, I'll ship this and the theme one and start a follow up to try to get rid of the setInterval.
I'll also open an issue (if needed) for all the things I've noticed and reported in the screenshot notes.