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

Improve child blocks API's and UI. #7003

Merged
merged 7 commits into from
May 31, 2018
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
2 changes: 2 additions & 0 deletions blocks/api/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ export {
getBlockSupport,
hasBlockSupport,
isSharedBlock,
getChildBlockNames,
hasChildBlocks,
} from './registration';
export {
isUnmodifiedDefaultBlock,
Expand Down
22 changes: 22 additions & 0 deletions blocks/api/registration.js
Original file line number Diff line number Diff line change
Expand Up @@ -288,3 +288,25 @@ export function hasBlockSupport( nameOrType, feature, defaultSupports ) {
export function isSharedBlock( blockOrType ) {
return blockOrType.name === 'core/block';
}
/**
* Returns an array with the child blocks of a given block.
*
* @param {string} blockName Block type name.
*
* @return {Array} Array of child block names.
*/
Copy link
Member

Choose a reason for hiding this comment

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

Not sure a JSDoc parser would detect this as being "attached" to the getChildBlockNames function with the newline between.

Copy link
Member Author

Choose a reason for hiding this comment

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

The line breaks were corrected.


export const getChildBlockNames = ( blockName ) => {
return select( 'core/blocks' ).getChildBlockNames( blockName );
};

/**
* Returns a boolean indicating if a block has child blocks or not.
*
* @param {string} blockName Block type name.
*
* @return {boolean} True if a block contains child blocks and false otherwise.
*/
export const hasChildBlocks = ( blockName ) => {
return select( 'core/blocks' ).hasChildBlocks( blockName );
};
35 changes: 35 additions & 0 deletions blocks/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* External dependencies
*/
import createSelector from 'rememo';
import { filter, includes, map } from 'lodash';

/**
* Returns all the available block types.
Expand Down Expand Up @@ -61,3 +62,37 @@ export function getDefaultBlockName( state ) {
export function getFallbackBlockName( state ) {
return state.fallbackBlockName;
}

/**
* Returns an array with the child blocks of a given block.
*
* @param {Object} state Data state.
* @param {string} blockName Block type name.
*
* @return {Array} Array of child block names.
*/
export const getChildBlockNames = createSelector(
Copy link
Member

Choose a reason for hiding this comment

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

This selector seems to be quite important so let’s add unit tests to avoid regressions in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unit tests added 👍

( state, blockName ) => {
return map(
filter( state.blockTypes, ( blockType ) => {
return includes( blockType.parent, blockName );
} ),
( { name } ) => name
);
},
( state ) => [
state.blockTypes,
]
);

/**
* Returns a boolean indicating if a block has child blocks or not.
*
* @param {Object} state Data state.
* @param {string} blockName Block type name.
*
* @return {boolean} True if a block contains child blocks and false otherwise.
*/
export const hasChildBlocks = ( state, blockName ) => {
return getChildBlockNames( state, blockName ).length > 0;
};
137 changes: 137 additions & 0 deletions blocks/store/test/selectors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
/**
* Internal dependencies
*/
import { getChildBlockNames } from '../selectors';

describe( 'selectors', () => {
describe( 'getChildBlockNames', () => {
it( 'should return an empty array if state is empty', () => {
const state = {};

expect( getChildBlockNames( state, 'parent1' ) ).toHaveLength( 0 );
} );

it( 'should return an empty array if no childs exist', () => {
const state = {
blockTypes: [
{
name: 'child1',
parent: [ 'parent1' ],
},
{
name: 'child2',
parent: [ 'parent2' ],
},
{
name: 'parent3',
},
],
};

expect( getChildBlockNames( state, 'parent3' ) ).toHaveLength( 0 );
} );

it( 'should return an empty array if the parent block is not found', () => {
const state = {
blockTypes: [
{
name: 'child1',
parent: [ 'parent1' ],
},
{
name: 'parent1',
},
],
};

expect( getChildBlockNames( state, 'parent3' ) ).toHaveLength( 0 );
} );

it( 'should return an array with the child block names', () => {
const state = {
blockTypes: [
{
name: 'child1',
parent: [ 'parent1' ],
},
{
name: 'child2',
parent: [ 'parent2' ],
},
{
name: 'child3',
parent: [ 'parent1' ],
},
{
name: 'child4',
},
{
name: 'parent1',
},
{
name: 'parent2',
},
],
};

expect( getChildBlockNames( state, 'parent1' ) ).toEqual( [ 'child1', 'child3' ] );
} );

it( 'should return an array with the child block names even if only one child exists', () => {
const state = {
blockTypes: [
{
name: 'child1',
parent: [ 'parent1' ],
},
{
name: 'child2',
parent: [ 'parent2' ],
},
{
name: 'child4',
},
{
name: 'parent1',
},
{
name: 'parent2',
},
],
};

expect( getChildBlockNames( state, 'parent1' ) ).toEqual( [ 'child1' ] );
} );

it( 'should return an array with the child block names even if childs have multiple parents', () => {
const state = {
blockTypes: [
{
name: 'child1',
parent: [ 'parent1' ],
},
{
name: 'child2',
parent: [ 'parent1', 'parent2' ],
},
{
name: 'child3',
parent: [ 'parent1' ],
},
{
name: 'child4',
},
{
name: 'parent1',
},
{
name: 'parent2',
},
],
};

expect( getChildBlockNames( state, 'parent1' ) ).toEqual( [ 'child1', 'child2', 'child3' ] );
expect( getChildBlockNames( state, 'parent2' ) ).toEqual( [ 'child2' ] );
} );
} );
} );
2 changes: 1 addition & 1 deletion edit-post/assets/stylesheets/_variables.scss
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ $admin-sidebar-width-big: 190px;
$admin-sidebar-width-collapsed: 36px;

// Visuals
$shadow-popover: 0 3px 20px rgba( $dark-gray-900, .1 ), 0 1px 3px rgba( $dark-gray-900, .1 );
$shadow-popover: 0 3px 30px rgba( $dark-gray-900, .1 );
$shadow-toolbar: 0 2px 10px rgba( $dark-gray-900, .1 ), 0 0 2px rgba( $dark-gray-900, .1 );
$shadow-below-only: 0 5px 10px rgba( $dark-gray-900, .1 ), 0 2px 2px rgba( $dark-gray-900, .1 );

Expand Down
29 changes: 29 additions & 0 deletions editor/components/inserter/child-blocks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
* Internal dependencies
*/
import './style.scss';
import ItemList from './item-list';
import BlockIcon from '../block-icon';

function ChildBlocks( { rootBlockIcon, rootBlockTitle, items, ...props } ) {
if ( ! items || ! items.length ) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we use ifCondition HOC here?

return null;
}
return (
<div className="editor-inserter__child-blocks">
{ ( rootBlockIcon || rootBlockTitle ) && (
<div className="editor-inserter__parent-block-header">
{ rootBlockIcon && (
<div className="editor-inserter__parent-block-icon">
<BlockIcon icon={ rootBlockIcon } />
</div>
) }
{ rootBlockTitle && <h2>{ rootBlockTitle }</h2> }
</div>
) }
<ItemList items={ items } { ...props } />
</div>
);
}

export default ChildBlocks;
4 changes: 3 additions & 1 deletion editor/components/inserter/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class Inserter extends Component {
title,
children,
onInsertBlock,
rootUID,
} = this.props;

if ( items.length === 0 ) {
Expand Down Expand Up @@ -74,7 +75,7 @@ class Inserter extends Component {
onClose();
};

return <InserterMenu items={ items } onSelect={ onSelect } />;
return <InserterMenu items={ items } onSelect={ onSelect } rootUID={ rootUID } />;
} }
/>
);
Expand All @@ -96,6 +97,7 @@ export default compose( [
insertionPoint,
selectedBlock: getSelectedBlock(),
items: getInserterItems( rootUID ),
rootUID,
};
} ),
withDispatch( ( dispatch, ownProps ) => ( {
Expand Down
10 changes: 9 additions & 1 deletion editor/components/inserter/item-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,15 @@ class ItemList extends Component {
<button
role="menuitem"
key={ item.id }
className={ classnames( 'editor-inserter__item', getBlockMenuDefaultClassName( item.id ) ) }
className={
classnames(
'editor-inserter__item',
getBlockMenuDefaultClassName( item.id ),
{
'editor-inserter__item-with-childs': item.hasChildBlocks,
Copy link
Member

Choose a reason for hiding this comment

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

  • Grammar: "childs" -> "children"
  • This is a modifier, so could be written as one, e.g. has-children (reference)

}
)
}
onClick={ () => onSelect( item ) }
tabIndex={ isCurrent || item.isDisabled ? null : '-1' }
disabled={ item.isDisabled }
Expand Down
Loading