-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Changes from 6 commits
c2ff68c
da24c3e
8d564e9
562c558
0334269
a943aa6
7a3e385
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
* External dependencies | ||
*/ | ||
import createSelector from 'rememo'; | ||
import { filter, includes, map } from 'lodash'; | ||
|
||
/** | ||
* Returns all the available block types. | ||
|
@@ -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( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
}; |
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' ] ); | ||
} ); | ||
} ); | ||
} ); |
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 ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we use |
||
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; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
) | ||
} | ||
onClick={ () => onSelect( item ) } | ||
tabIndex={ isCurrent || item.isDisabled ? null : '-1' } | ||
disabled={ item.isDisabled } | ||
|
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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.