Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Template Parts & Reusable Blocks - try overlay element for clickthrough to edit pattern. #31109

Merged
merged 51 commits into from
Jun 25, 2021
Merged
Show file tree
Hide file tree
Changes from 50 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
8da3a6a
clickthrough for template part
Addison-Stavlo Mar 23, 2021
6cbfb21
make store names more consistent and accurate
Addison-Stavlo Mar 23, 2021
89f6cb8
remove unnecessary selector change
Addison-Stavlo Mar 23, 2021
9b685c0
move color from hover to selected, add opacity value
Addison-Stavlo Mar 24, 2021
5d363d7
Merge branch 'trunk' into try/template-part-clickthrough-selection
jameskoster Apr 8, 2021
51b0a9f
Add hover overlay
jameskoster Apr 8, 2021
562a393
rebase master
Addison-Stavlo Apr 22, 2021
e16a091
bleh resizing issues
Addison-Stavlo Apr 22, 2021
f27bcb2
kind of working...
Addison-Stavlo Apr 22, 2021
6eb82de
remove unnecessary popover goo
Addison-Stavlo Apr 22, 2021
e5d4f3f
fix for nested template parts and selection bleeding out of boundary
Addison-Stavlo Apr 23, 2021
621dcd9
remove unnecessary effect/state
Addison-Stavlo Apr 23, 2021
bcbabf9
fix resizing issue
Addison-Stavlo Apr 28, 2021
0d45917
try only selected color when focused
Addison-Stavlo Apr 28, 2021
a3e35e6
cleanup css
Addison-Stavlo Apr 28, 2021
a1ae072
try overlay for initial selection only
Addison-Stavlo Apr 28, 2021
7d74900
enable drag and drop
Addison-Stavlo Apr 30, 2021
0550d76
only show borders on drag and drop
Addison-Stavlo Apr 30, 2021
5272c12
ensure background color not on drag and drop
Addison-Stavlo Apr 30, 2021
8085ce9
apply to reusable blocks
Addison-Stavlo Apr 30, 2021
fd702eb
rebase trunk
Addison-Stavlo May 4, 2021
5351165
fix e2e tests
Addison-Stavlo May 4, 2021
8ff4b94
try add basic test
Addison-Stavlo May 4, 2021
b760a07
try fix test
Addison-Stavlo May 4, 2021
8639351
only use the afterEach where needed
Addison-Stavlo May 4, 2021
08ecb26
use css vars
Addison-Stavlo May 4, 2021
1d0d27b
rebase trunk
Addison-Stavlo May 4, 2021
cf2b380
merge trunk
Addison-Stavlo May 5, 2021
00218e0
fix border color
Addison-Stavlo May 6, 2021
35d61f4
fix block toolbar issue
Addison-Stavlo May 6, 2021
882fbf8
fix width on classic themes
Addison-Stavlo May 7, 2021
15157e9
fix width by moving overlay inside block wrapper
Addison-Stavlo May 7, 2021
f4b6f43
fix bug with selection from list view
Addison-Stavlo May 7, 2021
84c686f
add condition to dismissing
Addison-Stavlo May 7, 2021
b00bb3f
show overlay when highlighting list view
Addison-Stavlo May 7, 2021
d52dc45
use selector for highlight style
Addison-Stavlo May 10, 2021
ff04eb3
refactor and comments
Addison-Stavlo May 10, 2021
57e6a8d
position overlay above resize containers
Addison-Stavlo May 10, 2021
6a6ad3a
fix nested entity test
Addison-Stavlo May 10, 2021
2fe7645
fix template part test
Addison-Stavlo May 10, 2021
5344a22
fix intermittent errors on conversion tests
Addison-Stavlo May 10, 2021
3114fdf
handle conflict/rebase trunk
Addison-Stavlo May 11, 2021
3a3f398
remove box shadow on hover - block hover style will already be present
Addison-Stavlo May 11, 2021
ba5decc
add border back to overlay
Addison-Stavlo May 19, 2021
6055b49
use blockProps in innerBlockProps as well
Addison-Stavlo May 19, 2021
a471f05
cleanup which blockProps are passed to innerBlockProps
Addison-Stavlo May 20, 2021
bdd3ad3
rebase trunk
Addison-Stavlo Jun 16, 2021
8f7d8be
use component as wrapper
Addison-Stavlo Jun 16, 2021
2bf4aca
remove unnecessary layout selectors on TP e2es
Addison-Stavlo Jun 16, 2021
a7ea5e4
fix reusable blocks test xpath
Addison-Stavlo Jun 16, 2021
58cdcf5
rebase trunk
Addison-Stavlo Jun 25, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/base-styles/_functions.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/**
* Converts a hex value into the rgb equivalent.
*
* @param {string} hex - the hexadecimal value to convert
* @return {string} comma separated rgb values
*/
@function hex-to-rgb($hex) {
@return red($hex), green($hex), blue($hex);
}
8 changes: 7 additions & 1 deletion packages/base-styles/_mixins.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
@import "./functions";

/**
* Breakpoint mixins
*/
Expand Down Expand Up @@ -440,11 +442,15 @@
}

@mixin admin-scheme($color-primary) {
// Define RGB equivalents for use in rgba function.
// Hexadecimal css vars do not work in the rgba function.
--wp-admin-theme-color: #{$color-primary};

--wp-admin-theme-color--rgb: #{hex-to-rgb($color-primary)};
// Darker shades.
--wp-admin-theme-color-darker-10: #{darken($color-primary, 5%)};
--wp-admin-theme-color-darker-10--rgb: #{hex-to-rgb(darken($color-primary, 5%))};
--wp-admin-theme-color-darker-20: #{darken($color-primary, 10%)};
--wp-admin-theme-color-darker-20--rgb: #{hex-to-rgb(darken($color-primary, 10%))};

// Focus style width.
// Avoid rounding issues by showing a whole 2px for 1x screens, and 1.5px on high resolution screens.
Expand Down
3 changes: 3 additions & 0 deletions packages/base-styles/_z-index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ $z-layers: (
// The toolbar, when contextual, should be above any adjacent nested block click overlays.
".block-editor-block-contextual-toolbar": 61,

// Ensures content overlay appears higher than resize containers used for image/video/etc.
".block-editor-block-content-overlay__overlay": 10,

// The block mover, particularly in nested contexts,
// should overlap most block content.
".block-editor-block-list__block.is-{selected,hovered} .block-editor-block-mover": 61,
Expand Down
101 changes: 101 additions & 0 deletions packages/block-editor/src/components/block-content-overlay/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/**
* WordPress dependencies
*/
import { useSelect } from '@wordpress/data';
import { useState, useEffect } from '@wordpress/element';

/**
* Internal dependencies
*/
import { store as blockEditorStore } from '../../store';

/**
* External dependencies
*/
import classnames from 'classnames';

export default function BlockContentOverlay( {
clientId,
tagName: TagName = 'div',
wrapperProps,
className,
} ) {
const baseClassName = 'block-editor-block-content-overlay';
const [ isOverlayActive, setIsOverlayActive ] = useState( true );
const [ isHovered, setIsHovered ] = useState( false );

const {
isParentSelected,
hasChildSelected,
isDraggingBlocks,
isParentHighlighted,
} = useSelect(
( select ) => {
const {
isBlockSelected,
hasSelectedInnerBlock,
isDraggingBlocks: _isDraggingBlocks,
isBlockHighlighted,
} = select( blockEditorStore );
return {
isParentSelected: isBlockSelected( clientId ),
hasChildSelected: hasSelectedInnerBlock( clientId, true ),
isDraggingBlocks: _isDraggingBlocks(),
isParentHighlighted: isBlockHighlighted( clientId ),
};
},
[ clientId ]
);

const classes = classnames(
baseClassName,
wrapperProps?.className,
className,
{
'overlay-active': isOverlayActive,
'parent-highlighted': isParentHighlighted,
'is-dragging-blocks': isDraggingBlocks,
}
);

useEffect( () => {
// Reenable when blocks are not in use.
if ( ! isParentSelected && ! hasChildSelected && ! isOverlayActive ) {
setIsOverlayActive( true );
}
// Disable if parent selected by another means (such as list view).
// We check hover to ensure the overlay click interaction is not taking place.
// Trying to click the overlay will select the parent block via its 'focusin'
// listener on the wrapper, so if the block is selected while hovered we will
// let the mouseup disable the overlay instead.
if ( isParentSelected && ! isHovered && isOverlayActive ) {
setIsOverlayActive( false );
}
// Ensure overlay is disabled if a child block is selected.
if ( hasChildSelected && isOverlayActive ) {
setIsOverlayActive( false );
}
}, [ isParentSelected, hasChildSelected, isOverlayActive, isHovered ] );

// Disabled because the overlay div doesn't actually have a role or functionality
// as far as the a11y is concerned. We're just catching the first click so that
// the block can be selected without interacting with its contents.
/* eslint-disable jsx-a11y/no-static-element-interactions */
return (
<TagName
{ ...wrapperProps }
className={ classes }
onMouseEnter={ () => setIsHovered( true ) }
onMouseLeave={ () => setIsHovered( false ) }
>
{ isOverlayActive && (
<div
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there on way to do this with less elements involved? Maybe pointer-events: none and enable when the block is selected?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure we did this already in #34012

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Sorry, missed that.

className={ `${ baseClassName }__overlay` }
onMouseUp={ () => setIsOverlayActive( false ) }
/>
) }
{ wrapperProps?.children }
</TagName>
);
}
/* eslint-enable jsx-a11y/no-static-element-interactions */
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Specificity required to ensure overlay width is not restricted to that
// of standard block content. The overlay's width should be as wide as
// its children require.
.editor-styles-wrapper .wp-block .block-editor-block-content-overlay__overlay {
max-width: none;
}

.block-editor-block-content-overlay {
.block-editor-block-content-overlay__overlay {
position: absolute;
top: 0;
left: 0;
width: 100%;
height: 100%;
background: transparent;
border: none;
border-radius: $radius-block-ui;
z-index: z-index(".block-editor-block-content-overlay__overlay");
}

&:hover:not(.is-dragging-blocks),
&.parent-highlighted {
> .block-editor-block-content-overlay__overlay {
background: rgba(var(--wp-admin-theme-color--rgb), 0.1);
box-shadow: 0 0 0 $border-width var(--wp-admin-theme-color) inset;
}
}

&.overlay-active:not(.is-dragging-blocks) {
*:not(.block-editor-block-content-overlay__overlay) {
pointer-events: none;
}
}

&.is-dragging-blocks {
box-shadow: 0 0 0 $border-width var(--wp-admin-theme-color);
.block-editor-block-content-overlay__overlay {
pointer-events: none;
}
}
}
1 change: 1 addition & 0 deletions packages/block-editor/src/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export {
export { default as __experimentalBlockFullHeightAligmentControl } from './block-full-height-alignment-control';
export { default as __experimentalBlockAlignmentMatrixControl } from './block-alignment-matrix-control';
export { default as BlockBreadcrumb } from './block-breadcrumb';
export { default as __experimentalBlockContentOverlay } from './block-content-overlay';
export { BlockContextProvider } from './block-context';
export {
default as BlockControls,
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
@import "./components/block-breadcrumb/style.scss";
@import "./components/block-card/style.scss";
@import "./components/block-compare/style.scss";
@import "./components/block-content-overlay/style.scss";
@import "./components/block-draggable/style.scss";
@import "./components/block-mobile-toolbar/style.scss";
@import "./components/block-mover/style.scss";
Expand Down
13 changes: 8 additions & 5 deletions packages/block-library/src/block/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { __ } from '@wordpress/i18n';
import {
__experimentalUseInnerBlocksProps as useInnerBlocksProps,
__experimentalUseNoRecursiveRenders as useNoRecursiveRenders,
__experimentalBlockContentOverlay as BlockContentOverlay,
InnerBlocks,
BlockControls,
InspectorControls,
Expand Down Expand Up @@ -70,6 +71,8 @@ export default function ReusableBlockEdit( { attributes: { ref }, clientId } ) {
ref
);

const blockProps = useBlockProps();

const innerBlocksProps = useInnerBlocksProps(
{},
{
Expand All @@ -82,8 +85,6 @@ export default function ReusableBlockEdit( { attributes: { ref }, clientId } ) {
}
);

const blockProps = useBlockProps();

if ( hasAlreadyRendered ) {
return (
<div { ...blockProps }>
Expand Down Expand Up @@ -136,9 +137,11 @@ export default function ReusableBlockEdit( { attributes: { ref }, clientId } ) {
/>
</PanelBody>
</InspectorControls>
<div className="block-library-block__reusable-block-container">
{ <div { ...innerBlocksProps } /> }
</div>
<BlockContentOverlay
clientId={ clientId }
wrapperProps={ innerBlocksProps }
className="block-library-block__reusable-block-container"
/>
</div>
</RecursionProvider>
);
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/template-part/edit/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ export default function TemplatePartEdit( {
) }
{ isEntityAvailable && (
<TemplatePartInnerBlocks
clientId={ clientId }
tagName={ TagName }
blockProps={ blockProps }
postId={ templatePartId }
Expand Down
14 changes: 12 additions & 2 deletions packages/block-library/src/template-part/edit/inner-blocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { useEntityBlockEditor } from '@wordpress/core-data';
import {
InnerBlocks,
__experimentalUseInnerBlocksProps as useInnerBlocksProps,
__experimentalBlockContentOverlay as BlockContentOverlay,
useSetting,
store as blockEditorStore,
} from '@wordpress/block-editor';
Expand All @@ -15,8 +16,9 @@ export default function TemplatePartInnerBlocks( {
postId: id,
hasInnerBlocks,
layout,
tagName: TagName,
tagName,
blockProps,
clientId,
} ) {
const themeSupportsLayout = useSelect( ( select ) => {
const { getSettings } = select( blockEditorStore );
Expand Down Expand Up @@ -45,6 +47,7 @@ export default function TemplatePartInnerBlocks( {
'wp_template_part',
{ id }
);

const innerBlocksProps = useInnerBlocksProps( blockProps, {
value: blocks,
onInput,
Expand All @@ -54,5 +57,12 @@ export default function TemplatePartInnerBlocks( {
: InnerBlocks.ButtonBlockAppender,
__experimentalLayout: _layout,
} );
return <TagName { ...innerBlocksProps } />;

return (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the changes here could potentially break alignments inside template parts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ive mostly been testing on a header including a nested template part, and the alignments seem to be the same as before and correspond to the front-end output. I definitely wouldn't want to break any alignments though, is there anything specific use case that might make this problematic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this new approach should be a bit more safe here.

<BlockContentOverlay
clientId={ clientId }
tagName={ tagName }
wrapperProps={ innerBlocksProps }
/>
);
}
2 changes: 1 addition & 1 deletion packages/e2e-test-utils/src/inserter.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ export async function insertReusableBlock( searchTerm ) {
await waitForInserterCloseAndContentFocus();
// We should wait until the block is loaded
await page.waitForXPath(
'//*[@class="block-library-block__reusable-block-container"]'
'//*[contains(@class,"block-library-block__reusable-block-container")]'
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
canvas,
openDocumentSettingsSidebar,
pressKeyWithModifier,
selectBlockByClientId,
} from '@wordpress/e2e-test-utils';

/**
Expand Down Expand Up @@ -218,7 +219,7 @@ describe( 'Multi-entity editor states', () => {
removeErrorMocks();
} );

afterEach( async () => {
const saveAndWaitResponse = async () => {
await Promise.all( [
saveAllEntities(),

Expand All @@ -241,7 +242,7 @@ describe( 'Multi-entity editor states', () => {
} ),
] );
removeErrorMocks();
} );
};

it( 'should only dirty the parent entity when editing the parent', async () => {
// Clear selection so that the block is not added to the template part.
Expand All @@ -253,9 +254,12 @@ describe( 'Multi-entity editor states', () => {
expect( await isEntityDirty( templateName ) ).toBe( true );
expect( await isEntityDirty( templatePartName ) ).toBe( false );
expect( await isEntityDirty( nestedTPName ) ).toBe( false );
await saveAndWaitResponse();
} );

it( 'should only dirty the child when editing the child', async () => {
// Select parent TP to unlock selecting content.
await canvas().click( '.wp-block-template-part' );
await canvas().click(
'.wp-block-template-part .wp-block[data-type="core/paragraph"]'
);
Expand All @@ -264,9 +268,16 @@ describe( 'Multi-entity editor states', () => {
expect( await isEntityDirty( templateName ) ).toBe( false );
expect( await isEntityDirty( templatePartName ) ).toBe( true );
expect( await isEntityDirty( nestedTPName ) ).toBe( false );
await saveAndWaitResponse();
} );

it( 'should only dirty the nested entity when editing the nested entity', async () => {
// Select parent TP to unlock selecting child.
await canvas().click( '.wp-block-template-part' );
// Select child TP to unlock selecting content.
await canvas().click(
'.wp-block-template-part .wp-block-template-part'
);
await canvas().click(
'.wp-block-template-part .wp-block-template-part .wp-block[data-type="core/paragraph"]'
);
Expand All @@ -275,6 +286,21 @@ describe( 'Multi-entity editor states', () => {
expect( await isEntityDirty( templateName ) ).toBe( false );
expect( await isEntityDirty( templatePartName ) ).toBe( false );
expect( await isEntityDirty( nestedTPName ) ).toBe( true );
await saveAndWaitResponse();
} );

it( 'should not allow selecting template part content without parent selected', async () => {
// Unselect blocks.
await selectBlockByClientId();
// Try to select a child block of a template part.
await canvas().click(
'.wp-block-template-part .wp-block-template-part .wp-block[data-type="core/paragraph"]'
);

const selectedBlock = await page.evaluate( () => {
return wp.data.select( 'core/block-editor' ).getSelectedBlock();
} );
expect( selectedBlock?.name ).toBe( 'core/template-part' );
} );
} );
} );
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,6 @@ describe( 'Multi-entity save flow', () => {
);
await createNewButton.click();
await page.waitForSelector( activatedTemplatePartSelector );
await page.keyboard.press( 'Tab' );
await page.keyboard.type( 'test-template-part' );
await page.click( '.block-editor-button-block-appender' );
await page.click( '.editor-block-list-item-paragraph' );
await page.keyboard.type( 'some words...' );
Expand Down Expand Up @@ -163,6 +161,9 @@ describe( 'Multi-entity save flow', () => {

// Update template part.
await page.click( templatePartSelector );
await page.click(
`${ templatePartSelector } .wp-block[data-type="core/paragraph"]`
);
await page.keyboard.type( '...some more words...' );
await page.keyboard.press( 'Enter' );

Expand Down
Loading