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.

Sign up for GitHub

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

Selecting Parent Blocks: Try clickthrough #15537

Merged
merged 11 commits into from
Jun 6, 2019
+86 −45
Merged

Selecting Parent Blocks: Try clickthrough #15537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
47d6de7
Selecting parents: Try clickthrough.
May 9, 2019
2667575
Render overlay on top of inner block only when none of the nested blo…
gziolo May 13, 2019
b66cb2d
Fix overlay, fix breadcrumb, polish.
May 21, 2019
74920ee
Remove click-overlay.
jasmussen May 21, 2019
e5a0b12
Fix the selection of inner blocks for reusable blocks template
gziolo May 27, 2019
fafcb1f
Disable async mode for parent blocks when nested block selected
gziolo May 27, 2019
0f5ffac
Refactor BlockListBlock to use AsyncModeProvider
gziolo May 28, 2019
5c28d4b
Make the reusable blocks save button clickable. i
jasmussen May 28, 2019
de050d4
Fix so reusable blocks with nesting are editable.
jasmussen May 28, 2019
b4b3e76
Fix movers.
jasmussen May 28, 2019
b0649e1
Bring the behavior closer to what we have as of today
gziolo Jun 6, 2019
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
17 changes: 10 additions & 7 deletions assets/stylesheets/_z-index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ $z-layers: (
".block-library-classic__toolbar": 10,
".block-editor-block-list__layout .reusable-block-indicator": 1,
".block-editor-block-list__breadcrumb": 2,
".editor-inner-blocks .block-editor-block-list__breadcrumb": 22,
".components-form-toggle__input": 1,
".components-panel__header.edit-post-sidebar__panel-tabs": -1,
".edit-post-sidebar .components-panel": -2,
Expand All @@ -19,7 +18,6 @@ $z-layers: (
".components-modal__header": 10,
".edit-post-meta-boxes-area.is-loading::before": 1,
".edit-post-meta-boxes-area .spinner": 5,
".block-editor-block-contextual-toolbar": 21,
".components-popover__close": 5,
".block-editor-block-list__insertion-point": 6,
".block-editor-inserter-with-shortcuts": 5,
Expand Down Expand Up @@ -51,16 +49,21 @@ $z-layers: (
".components-drop-zone": 100,
".components-drop-zone__content": 110,

// The block mover, particularly in nested contexts,
// should overlap most block content.
".block-editor-block-list__block.is-{selected,hovered} .block-editor-block-mover": 80,

// The block mover for floats should overlap the controls of adjacent blocks.
".block-editor-block-list__block {core/image aligned left or right}": 81,

// Small screen inner blocks overlay must be displayed above drop zone,
// settings menu, and movers.
".block-editor-inner-blocks__small-screen-overlay:after": 120,
".block-editor-inner-blocks.has-overlay::after": 120,

// The toolbar, when contextual, should be above any adjacent nested block click overlays.
".block-editor-block-list__layout .reusable-block-edit-panel": 121,
".block-editor-block-contextual-toolbar": 121,
".editor-inner-blocks .block-editor-block-list__breadcrumb": 122,

// The block mover, particularly in nested contexts,
// should overlap most block content.
".block-editor-block-list__block.is-{selected,hovered} .block-editor-block-mover": 121,

// Show sidebar above wp-admin navigation bar for mobile viewports:
// #wpadminbar { z-index: 99999 }
Expand Down
23 changes: 23 additions & 0 deletions packages/block-editor/src/components/block-list/block-async-mode-provider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/**
* WordPress dependencies
*/
import {
__experimentalAsyncModeProvider as AsyncModeProvider,
useSelect,
} from '@wordpress/data';

const BlockAsyncModeProvider = ( { children, clientId, isBlockInSelection } ) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Conceptually speaking and if there's no impact on performance it would be better to avoid the isBlockInSelection as a prop and compute it here :).

Copy link
Member

Choose a reason for hiding this comment

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

b0649e1 - I had it before but I assumed it could impact performance. I can try with the previous commit.

Copy link
Member

@gziolo gziolo Jun 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I don't thin that benchmark agrees with this idea:

Average time to load: 5724ms
Average time to DOM content load: 5219ms
Average time to type character: 187.28ms 😅 
Slowest time to type character: 319ms 🐢 
Fastest time to type character: 158ms 🐢 

Average time to load: 7533ms 🐢 
Average time to DOM content load: 7011ms 🐢 
Average time to type character: 177.2ms 🐢 
Slowest time to type character: 321ms 🐢 
Fastest time to type character: 145ms 🐢

Copy link
Contributor

Choose a reason for hiding this comment

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

:)

const isParentOfSelectedBlock = useSelect( ( select ) => {
return select( 'core/block-editor' ).hasSelectedInnerBlock( clientId, true );
} );

const isSyncModeForced = isBlockInSelection || isParentOfSelectedBlock;

return (
<AsyncModeProvider value={ ! isSyncModeForced }>
{ children }
</AsyncModeProvider>
);
};

export default BlockAsyncModeProvider;
5 changes: 4 additions & 1 deletion packages/block-editor/src/components/block-list/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ import {
} from '@wordpress/blocks';
import { KeyboardShortcuts, withFilters } from '@wordpress/components';
import { __, sprintf } from '@wordpress/i18n';
import { withDispatch, withSelect } from '@wordpress/data';
import {
withDispatch,
withSelect,
} from '@wordpress/data';
import { withViewportMatch } from '@wordpress/viewport';
import { compose, pure } from '@wordpress/compose';

Expand Down
8 changes: 5 additions & 3 deletions packages/block-editor/src/components/block-list/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { compose } from '@wordpress/compose';
/**
* Internal dependencies
*/
import BlockAsyncModeProvider from './block-async-mode-provider';
import BlockListBlock from './block';
import BlockListAppender from '../block-list-appender';
import { getBlockDOMNode } from '../../utils/dom';
Expand Down Expand Up @@ -207,9 +208,10 @@ class BlockList extends Component {
selectedBlockClientId === clientId;

return (
<AsyncModeProvider
<BlockAsyncModeProvider
key={ 'block-' + clientId }
value={ ! isBlockInSelection }
clientId={ clientId }
isBlockInSelection={ isBlockInSelection }
>
<BlockListBlock
clientId={ clientId }
Expand All @@ -218,7 +220,7 @@ class BlockList extends Component {
rootClientId={ rootClientId }
isDraggable={ isDraggable }
/>
</AsyncModeProvider>
</BlockAsyncModeProvider>
);
} ) }

Expand Down
14 changes: 14 additions & 0 deletions packages/block-editor/src/components/block-list/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,20 @@
}
}

// Reusable Blocks clickthrough overlays
&.is-reusable > .block-editor-block-list__block-edit .block-editor-inner-blocks.has-overlay {
// Remove only the top click overlay.
&::after {
display: none;
}

// Restore it for subsequent.
.block-editor-inner-blocks.has-overlay::after {
display: block;
}
}


// Alignments
&[data-align="left"],
&[data-align="right"] {
Expand Down
12 changes: 5 additions & 7 deletions packages/block-editor/src/components/inner-blocks/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import classnames from 'classnames';
/**
* WordPress dependencies
*/
import { withViewportMatch } from '@wordpress/viewport';
import { Component } from '@wordpress/element';
import { withSelect, withDispatch } from '@wordpress/data';
import { synchronizeBlocksWithTemplate, withBlockContentContext } from '@wordpress/blocks';
Expand Down Expand Up @@ -106,14 +105,13 @@ class InnerBlocks extends Component {
render() {
const {
clientId,
isSmallScreen,
isSelectedBlockInRoot,
hasOverlay,
renderAppender,
} = this.props;
const { templateInProcess } = this.state;

const classes = classnames( 'editor-inner-blocks block-editor-inner-blocks', {
'has-overlay': isSmallScreen && ! isSelectedBlockInRoot,
'has-overlay': hasOverlay,
} );

return (
Expand All @@ -131,7 +129,6 @@ class InnerBlocks extends Component {

InnerBlocks = compose( [
withBlockEditContext( ( context ) => pick( context, [ 'clientId' ] ) ),
withViewportMatch( { isSmallScreen: '< medium' } ),
withSelect( ( select, ownProps ) => {
const {
isBlockSelected,
Expand All @@ -142,12 +139,13 @@ InnerBlocks = compose( [
getTemplateLock,
} = select( 'core/block-editor' );
const { clientId } = ownProps;
const block = getBlock( clientId );
const rootClientId = getBlockRootClientId( clientId );

return {
isSelectedBlockInRoot: isBlockSelected( clientId ) || hasSelectedInnerBlock( clientId ),
block: getBlock( clientId ),
block,
blockListSettings: getBlockListSettings( clientId ),
hasOverlay: block.name !== 'core/template' && ! isBlockSelected( clientId ) && ! hasSelectedInnerBlock( clientId, true ),
Copy link
Member

@gziolo gziolo May 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@youknowriad - any thoughts on this one? It's a really nasty hack to avoid having the overlay in the context of nesting inside Reusable block...

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 it's fine, ideally, this core/template block should be removed. Rebuilding the reusable blocks UI as a nested editor would allow us to remove it. See #14367

That PR is blocked by the fact that it's hard to use nested Slot/Fills. One tradeoff we can make to unblock it could be to decide that we don't show the inspector settings for the reusable blocks content and redirect the user to the edit page of the reusable blocks for a full editing session.

parentLock: getTemplateLock( rootClientId ),
};
} ),
Expand Down
18 changes: 10 additions & 8 deletions packages/block-editor/src/components/inner-blocks/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
.block-editor-inner-blocks.has-overlay::after {
content: "";
position: absolute;
top: 0;
right: 0;
bottom: 0;
left: 0;
z-index: z-index(".block-editor-inner-blocks__small-screen-overlay:after");
.block-editor-inner-blocks.has-overlay {
&::after {
content: "";
position: absolute;
top: -$block-padding;
right: -$block-padding;
bottom: -$block-padding;
left: -$block-padding;
z-index: z-index(".block-editor-inner-blocks.has-overlay::after");
}
}
4 changes: 4 additions & 0 deletions packages/block-library/src/block/edit-panel/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
margin: 0 (-$block-padding);
padding: $grid-size $block-padding;

// Elevate the reusable blocks toolbar above the clickthrough overlay.
position: relative;
z-index: z-index(".block-editor-block-list__layout .reusable-block-edit-panel");

// Use opacity to work in various editor styles.
border: $border-width dashed $dark-opacity-light-500;
border-bottom: none;
Expand Down
18 changes: 7 additions & 11 deletions packages/block-library/src/columns/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,17 +164,13 @@ div.block-core-columns.is-vertically-aligned-bottom {
*/
[data-type="core/column"] > .editor-block-list__block-edit > .editor-block-list__breadcrumb {
right: 0;
left: auto;
}

// The empty state of a columns block has the default appenders.
// Since those appenders are not blocks, the parent, actual block, appears "hovered" when hovering the appenders.
// Because the column shouldn't be hovered as part of this temporary passthrough, we unset the hover style.
.wp-block-columns [data-type="core/column"].is-hovered {
> .block-editor-block-list__block-edit::before {
content: none;
}

.block-editor-block-list__breadcrumb {
display: none;
}
/**
* Make single Column overlay not extend past boundaries of parent
*/
.block-core-columns > .block-editor-inner-blocks.has-overlay::after {
left: 0;
right: 0;
}
12 changes: 4 additions & 8 deletions packages/e2e-tests/specs/reusable-blocks.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,8 @@ describe( 'Reusable Blocks', () => {
'//*[contains(@class, "components-snackbar")]/*[text()="Block created."]'
);

// Select all of the text in the title field by triple-clicking on it. We
// triple-click because, on Mac, Mod+A doesn't work. This step can be removed
// when https://github.com/WordPress/gutenberg/issues/7972 is fixed
await page.click( '.reusable-block-edit-panel__title', { clickCount: 3 } );
// Select all of the text in the title field.
await pressKeyWithModifier( 'primary', 'a' );

// Give the reusable block a title
await page.keyboard.type( 'Greeting block' );
Expand Down Expand Up @@ -223,10 +221,8 @@ describe( 'Reusable Blocks', () => {
'//*[contains(@class, "components-snackbar")]/*[text()="Block created."]'
);

// Select all of the text in the title field by triple-clicking on it. We
// triple-click because, on Mac, Mod+A doesn't work. This step can be removed
// when https://github.com/WordPress/gutenberg/issues/7972 is fixed
await page.click( '.reusable-block-edit-panel__title', { clickCount: 3 } );
// Select all of the text in the title field.
await pressKeyWithModifier( 'primary', 'a' );

// Give the reusable block a title
await page.keyboard.type( 'Multi-selection reusable block' );
Expand Down