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

Add appender to Block Navigator #18100

Merged
merged 5 commits into from
Nov 6, 2019
+93 −13
Merged

Add appender to Block Navigator #18100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a890b5c
Add appender to Block Navigator
talldan Oct 25, 2019
519c4af
Use correct name for component
talldan Oct 30, 2019
6f817bb
Avoid selecting block on insert
talldan Oct 30, 2019
85eaa1f
Add spoken message indicating block has been inserted
talldan Oct 31, 2019
bd8fb2e
Use CSS to hide inner blocks instead of conditional rendering. Solves…
talldan Nov 5, 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
19 changes: 19 additions & 0 deletions packages/block-editor/src/components/block-navigation/list.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { create, getTextContent } from '@wordpress/rich-text';
* Internal dependencies
*/
import BlockIcon from '../block-icon';
import ButtonBlockAppender from '../button-block-appender';

/**
* Get the block display name, if it has one, or the block title if it doesn't.
Expand Down Expand Up @@ -43,8 +44,14 @@ export default function BlockNavigationList( {
blocks,
selectedBlockClientId,
selectBlock,
showAppender,

// Internal use only.
Copy link
Member

Choose a reason for hiding this comment

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

This is clever and made me think about how I'd love to see props grouped into // Public and //Private for all our components. I'm always finding it tough to figure out whether a prop is public or e.g. added via withSelect.

(Just a thought—nothing to do with this PR!)

showNestedBlocks,
parentBlockClientId,
} ) {
const shouldShowAppender = showAppender && !! parentBlockClientId;
Copy link
Contributor Author

@talldan talldan Oct 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I had some ideas about improving the logic in this component around showing the appender. At the moment it shows the appender for any block that has childBlocks. An improvement might be:

  • Show the appender if the block type supports it, e.g. supports: { __experimentalBlockNavigatorAppender: true }
  • Show the appender if the block type supports only a single type of inner block (so that the block is added immediately).

Copy link
Member

Choose a reason for hiding this comment

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

Showing it only if it has child blocks (which is the equivalent of showing it only at the end of inner block areas) seems the correct way to me. The fact it adds a block immediately (the parent only supports a single child) or opens a full inserter should be irrelevant for the navigator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, so happy with a popover in a modal?

I can look into getting that working. Cheers for the quick feedback. 👍

Copy link
Member

Choose a reason for hiding this comment

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

Yes, though since we are going to enable this only within navigation menu initially, which only supports a single child, it should affect things much.


return (
/*
* Disable reason: The `list` ARIA role is redundant but
Expand Down Expand Up @@ -75,12 +82,24 @@ export default function BlockNavigationList( {
blocks={ block.innerBlocks }
selectedBlockClientId={ selectedBlockClientId }
selectBlock={ selectBlock }
parentBlockClientId={ block.clientId }
showAppender={ showAppender }
showNestedBlocks
/>
) }
</li>
);
} ) }
{ shouldShowAppender && (
<li>
<div className="editor-block-navigation__item block-editor-block-navigation__item">
<ButtonBlockAppender
rootClientId={ parentBlockClientId }
__experimentalSelectBlockOnInsert={ false }
/>
</div>
</li>
) }
</ul>
/* eslint-enable jsx-a11y/no-redundant-roles */
);
Expand Down
18 changes: 18 additions & 0 deletions packages/block-editor/src/components/block-navigation/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,24 @@ $tree-item-height: 36px;
margin: 0;
}

.block-editor-block-navigation__list .block-editor-button-block-appender {
outline: none;
background: none;
padding: $grid-size;
margin-left: 0.8em;
width: $icon-button-size;
border-radius: 4px;

&:hover:not(:disabled):not([aria-disabled="true"]) {
@include menu-style__hover;
outline: none;
}

&:focus:not(:disabled):not([aria-disabled="true"]) {
@include menu-style__focus;
}
}

.block-editor-block-navigation__list .block-editor-block-navigation__list {
margin-top: 2px;
border-left: $tree-border-width solid $light-gray-900;
Expand Down
3 changes: 2 additions & 1 deletion packages/block-editor/src/components/button-block-appender/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@ import { _x, sprintf } from '@wordpress/i18n';
import BlockDropZone from '../block-drop-zone';
import Inserter from '../inserter';

function ButtonBlockAppender( { rootClientId, className } ) {
function ButtonBlockAppender( { rootClientId, className, __experimentalSelectBlockOnInsert: selectBlockOnInsert } ) {
Copy link
Member

Choose a reason for hiding this comment

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

The argument that dispatch( 'core/block-editor' ).insertBlock accepts is named updateSelection. Should we re-use that name for consistency?

Copy link
Contributor Author

@talldan talldan Oct 31, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

<ButtonBlockAppender updateSelection={ false } /> didn't seem as clear to me as <ButtonBlockAppender selectBlockOnInsert={ false } />.

return (
<>
<BlockDropZone rootClientId={ rootClientId } />
<Inserter
rootClientId={ rootClientId }
__experimentalSelectBlockOnInsert={ selectBlockOnInsert }
renderToggle={ ( { onToggle, disabled, isOpen, blockTitle, hasSingleBlockType } ) => {
let label;
if ( hasSingleBlockType ) {
Expand Down
25 changes: 23 additions & 2 deletions packages/block-editor/src/components/inserter/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { get } from 'lodash';
/**
* WordPress dependencies
*/
import { speak } from '@wordpress/a11y';
import { __, _x, sprintf } from '@wordpress/i18n';
import { Dropdown, IconButton } from '@wordpress/components';
import { Component } from '@wordpress/element';
Expand Down Expand Up @@ -90,7 +91,13 @@ class Inserter extends Component {
* @return {WPElement} Dropdown content element.
*/
renderContent( { onClose } ) {
const { rootClientId, clientId, isAppender, showInserterHelpPanel } = this.props;
const {
rootClientId,
clientId,
isAppender,
showInserterHelpPanel,
__experimentalSelectBlockOnInsert: selectBlockOnInsert,
} = this.props;

return (
<InserterMenu
Expand All @@ -99,15 +106,18 @@ class Inserter extends Component {
clientId={ clientId }
isAppender={ isAppender }
showInserterHelpPanel={ showInserterHelpPanel }
__experimentalSelectBlockOnInsert={ selectBlockOnInsert }
/>
);
}

render() {
const { position, hasSingleBlockType, insertOnlyAllowedBlock } = this.props;

if ( hasSingleBlockType ) {
return this.renderToggle( { onToggle: insertOnlyAllowedBlock } );
}

return (
<Dropdown
className="editor-inserter block-editor-inserter"
Expand All @@ -133,10 +143,12 @@ export default compose( [
const allowedBlocks = __experimentalGetAllowedBlocks( rootClientId );

const hasSingleBlockType = allowedBlocks && ( get( allowedBlocks, [ 'length' ], 0 ) === 1 );

let allowedBlockType = false;
if ( hasSingleBlockType ) {
allowedBlockType = allowedBlocks[ 0 ];
}

return {
hasItems: hasInserterItems( rootClientId ),
hasSingleBlockType,
Expand All @@ -151,6 +163,7 @@ export default compose( [
const {
hasSingleBlockType,
allowedBlockType,
__experimentalSelectBlockOnInsert: selectBlockOnInsert,
} = ownProps;

if ( ! hasSingleBlockType ) {
Expand Down Expand Up @@ -184,11 +197,19 @@ export default compose( [
} = dispatch( 'core/block-editor' );

const blockToInsert = createBlock( allowedBlockType.name );

insertBlock(
blockToInsert,
getInsertionIndex(),
rootClientId
rootClientId,
selectBlockOnInsert
);

if ( ! selectBlockOnInsert ) {
// translators: %s: the name of the block that has been added
const message = sprintf( __( '%s block added' ), allowedBlockType.title );
speak( message );
}
},
};
} ),
Expand Down
21 changes: 17 additions & 4 deletions packages/block-editor/src/components/inserter/menu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import classnames from 'classnames';
/**
* WordPress dependencies
*/
import { speak } from '@wordpress/a11y';
import { __, _n, _x, sprintf } from '@wordpress/i18n';
import { Component, createRef } from '@wordpress/element';
import {
Expand Down Expand Up @@ -531,21 +532,33 @@ export default compose(
const {
getSelectedBlock,
} = select( 'core/block-editor' );
const { isAppender } = ownProps;
const { name, initialAttributes } = item;
const {
isAppender,
onSelect,
__experimentalSelectBlockOnInsert: selectBlockOnInsert,
} = ownProps;
const { name, title, initialAttributes } = item;
const selectedBlock = getSelectedBlock();
const insertedBlock = createBlock( name, initialAttributes );

if ( ! isAppender && selectedBlock && isUnmodifiedDefaultBlock( selectedBlock ) ) {
replaceBlocks( selectedBlock.clientId, insertedBlock );
} else {
insertBlock(
insertedBlock,
getInsertionIndex(),
ownProps.destinationRootClientId
ownProps.destinationRootClientId,
selectBlockOnInsert
);

if ( ! selectBlockOnInsert ) {
// translators: %s: the name of the block that has been added
const message = sprintf( __( '%s block added' ), title );
speak( message );
}
}

ownProps.onSelect();
onSelect();
return insertedBlock;
},
};
Expand Down
10 changes: 4 additions & 6 deletions packages/block-library/src/navigation-menu-item/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,12 +193,10 @@ function NavigationMenuItemEdit( {
placeholder={ __( 'Add item…' ) }
withoutInteractiveFormatting
/>
{ ( isSelected || isParentOfSelectedBlock ) &&
<InnerBlocks
allowedBlocks={ [ 'core/navigation-menu-item' ] }
renderAppender={ hasDescendants ? InnerBlocks.ButtonBlockAppender : false }
/>
}
<InnerBlocks
allowedBlocks={ [ 'core/navigation-menu-item' ] }
renderAppender={ hasDescendants ? InnerBlocks.ButtonBlockAppender : false }
/>
</div>
</Fragment>
);
Expand Down
9 changes: 9 additions & 0 deletions packages/block-library/src/navigation-menu-item/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,15 @@
// and the edit container should compensate.
// This is to make sure the edit and view are the same.
padding: 0 $grid-size;

// Only display inner blocks when the block is being edited.
.block-editor-inner-blocks {
display: none;
}

&.is-editing .block-editor-inner-blocks {
display: block;
}
}

.wp-block-navigation-menu-item__edit-container {
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/navigation-menu/block-navigation-list.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export default function BlockNavigationList( { clientId } ) {
selectedBlockClientId={ selectedBlockClientId }
selectBlock={ selectBlock }
showNestedBlocks
showAppender
/>
);
}