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 Part Block: Add a block label #24557

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,19 @@ _Returns_

- `Array`: Ordered client IDs of editor blocks.

<a name="getBlockParent" href="#getBlockParent">#</a> **getBlockParent**

Given a block client ID, returns the direct parent.

_Parameters_

- _state_ `Object`: Editor state.
- _clientId_ `string`: Block from which to find root client ID.

_Returns_

- `string`: ClientID of the parent block.

<a name="getBlockParents" href="#getBlockParents">#</a> **getBlockParents**

Given a block client ID, returns the list of all its parents from top to bottom.
Expand Down Expand Up @@ -681,6 +694,18 @@ _Returns_

- `?string`: Block Template Lock

<a name="getToolbarId" href="#getToolbarId">#</a> **getToolbarId**

Returns the current id of the Block toolbar

_Parameters_

- _state_ `Object`: Editor state.

_Returns_

- `string`: Id of the currently focused block toolbar.

<a name="hasBlockMovingClientId" href="#hasBlockMovingClientId">#</a> **hasBlockMovingClientId**

Returns whether block moving mode is enabled.
Expand Down Expand Up @@ -1301,6 +1326,14 @@ _Parameters_

- _hasBlockMovingClientId_ `(string|null)`: Enable/Disable block moving mode.

<a name="setBlockToolbarId" href="#setBlockToolbarId">#</a> **setBlockToolbarId**

Returns an action object that sets whether the block has controlled innerblocks.

_Parameters_

- _id_ `string`: The current block toolbar's id.

<a name="setHasControlledInnerBlocks" href="#setHasControlledInnerBlocks">#</a> **setHasControlledInnerBlocks**

Returns an action object that sets whether the block has controlled innerblocks.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
useEffect,
useCallback,
} from '@wordpress/element';
import { useDispatch } from '@wordpress/data';
import deprecated from '@wordpress/deprecated';
import { focus } from '@wordpress/dom';
import { useShortcut } from '@wordpress/keyboard-shortcuts';
Expand Down Expand Up @@ -108,6 +109,13 @@ function useToolbarFocus( ref, focusOnMount, isAccessibleToolbar ) {

function NavigableToolbar( { children, focusOnMount, ...props } ) {
const wrapper = useRef();

const { setBlockToolbarId } = useDispatch( 'core/block-editor' );

useEffect( () => {
setBlockToolbarId( wrapper.current.id );
}, [ wrapper.current ] );

const isAccessibleToolbar = useIsAccessibleToolbar( wrapper );

useToolbarFocus( wrapper, focusOnMount, isAccessibleToolbar );
Expand Down
12 changes: 12 additions & 0 deletions packages/block-editor/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -1100,3 +1100,15 @@ export function setHasControlledInnerBlocks(
clientId,
};
}

/**
* Returns an action object that sets whether the block has controlled innerblocks.
*
* @param {string} id The current block toolbar's id.
*/
export function setBlockToolbarId( id ) {
return {
type: 'SET_BLOCK_TOOLBAR_ID',
id,
};
}
20 changes: 20 additions & 0 deletions packages/block-editor/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1650,6 +1650,25 @@ export function highlightedBlock( state, action ) {
return state;
}

/**
* Reducer returning current the currently selected block toolbar id.
*
* @param {boolean} state Current state.
* @param {Object} action Dispatched action.
*
* @return {string} Current block toolbar id.
*/
export function toolbarId( state, action ) {
switch ( action.type ) {
case 'SET_BLOCK_TOOLBAR_ID':
const { id } = action;

return id;
}

return state;
}

export default combineReducers( {
blocks,
isTyping,
Expand All @@ -1671,4 +1690,5 @@ export default combineReducers( {
hasBlockMovingClientId,
automaticChangeStatus,
highlightedBlock,
toolbarId,
} );
29 changes: 29 additions & 0 deletions packages/block-editor/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,35 @@ export const getBlockParents = createSelector(
( state ) => [ state.blocks.parents ]
);

/**
* Given a block client ID, returns the direct parent.
*
* @param {Object} state Editor state.
* @param {string} clientId Block from which to find root client ID.
*
* @return {string} ClientID of the parent block.
*/
export const getBlockParent = createSelector(
Copy link
Contributor Author

@jeyip jeyip Aug 19, 2020

Choose a reason for hiding this comment

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

Note:

After combing through core/block-editor package's selectors, I couldn't find one that would only provide information about the direct parent. It seemed as if the other alternatives, like getBlockParents, returned all ancestor ids. This can be changed if someone has better suggestions.

Copy link
Member

Choose a reason for hiding this comment

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

this seems like a legitimate change to me if the behavior doesn't exist yet 👍

I'm curious how the "select parent block" functionality works without this, though??

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'm curious how the "select parent block" functionality works without this, though??

Great question! I'll look into it. That might give us insight into a selector we can use 🙂

Copy link
Contributor Author

@jeyip jeyip Aug 20, 2020

Choose a reason for hiding this comment

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

The only reference I could find to select parent block was in entity-record-item.js L42. It looks like they select the parent block by deriving information about the direct parent from lines 16 - 29.

This is an option if we don't want to include a new selector to the block-editor store. I don't have a strong opinion either way.

( state, clientId ) => {
return state.blocks.parents[ clientId ];
},
( state ) => [ state.blocks.parents ]
Copy link
Member

Choose a reason for hiding this comment

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

Not totally sure, maybe we should change this to ( state, clientId ) => [ state.blocks.parents[ clientID ] ] so that it only refreshes on changes to its own parent, not any parent

);

/**
* Returns the current id of the Block toolbar
*
* @param {Object} state Editor state.
*
* @return {string} Id of the currently focused block toolbar.
*/
export const getToolbarId = createSelector(
( state ) => {
return state.toolbarId;
},
( state ) => [ state.toolbarId ]
);

/**
* Given a block client ID and a block name,
* returns the list of all its parents from top to bottom,
Expand Down
45 changes: 39 additions & 6 deletions packages/block-library/src/template-part/edit/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { BlockControls } from '@wordpress/block-editor';
*/
import useTemplatePartPost from './use-template-part-post';
import TemplatePartNamePanel from './name-panel';
import TemplatePartLabel from './label';
import TemplatePartInnerBlocks from './inner-blocks';
import TemplatePartPlaceholder from './placeholder';

Expand All @@ -29,11 +30,31 @@ export default function TemplatePartEdit( {
// but wait until the third inner blocks change,
// because the first 2 are just the template part
// content loading.
const { innerBlocks } = useSelect(
const {
isNavigationMode,
parentId,
innerBlocks,
selectedBlockClientId,
} = useSelect(
( select ) => {
const { getBlocks } = select( 'core/block-editor' );
const {
getBlocks,
getSelectionStart,
isNavigationMode: _isNavigationMode,
getBlockParent,
} = select( 'core/block-editor' );

// Only sibling blocks can be multi-selected. This
// means that the parent should be the same for all
// multi-selected blocks. We arbitrarily select the first
// multi-selected block.
const _selectedBlockClientId = getSelectionStart()?.clientId;

return {
innerBlocks: getBlocks( clientId ),
isNavigationMode: _isNavigationMode,
parentId: getBlockParent( _selectedBlockClientId ),
selectedBlockClientId: _selectedBlockClientId,
};
},
[ clientId ]
Expand All @@ -57,6 +78,9 @@ export default function TemplatePartEdit( {
}
}, [ innerBlocks ] );

const isParentOfSelectedBlock = parentId === clientId;
const shouldDisplayLabel = ! isNavigationMode() && isParentOfSelectedBlock;

if ( postId ) {
// Part of a template file, post ID already resolved.
return (
Expand All @@ -67,10 +91,19 @@ export default function TemplatePartEdit( {
setAttributes={ setAttributes }
/>
</BlockControls>
<TemplatePartInnerBlocks
postId={ postId }
hasInnerBlocks={ innerBlocks.length > 0 }
/>
<div className="wp-block-template-part__container">
{ shouldDisplayLabel && (
<TemplatePartLabel
postId={ postId }
slug={ slug }
selectedBlockClientId={ selectedBlockClientId }
/>
) }
<TemplatePartInnerBlocks
postId={ postId }
hasInnerBlocks={ innerBlocks.length > 0 }
/>
</div>
</>
);
}
Expand Down
76 changes: 76 additions & 0 deletions packages/block-library/src/template-part/edit/label.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/**
Copy link
Contributor Author

@jeyip jeyip Aug 13, 2020

Choose a reason for hiding this comment

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

Note:

We can definitely talk about refactoring the labels component for reusability if we move forward with template part labels

* External dependencies
*/
import { capitalize } from 'lodash';
import cx from 'classnames';

/**
* WordPress dependencies
*/
import { useRef, useState, useEffect } from '@wordpress/element';
import { useSelect } from '@wordpress/data';
import { useEntityProp } from '@wordpress/core-data';
import { Icon } from '@wordpress/components';

function detectOverlap( e1, e2 ) {
const rect1 = e1 && e1.getBoundingClientRect();
const rect2 = e2 && e2.getBoundingClientRect();

let overlap = null;
if ( rect1 && rect2 ) {
overlap = ! (
rect1.right < rect2.left ||
rect1.left > rect2.right ||
rect1.bottom < rect2.top ||
rect1.top > rect2.bottom
);
return overlap;
}

return false;
}

export default function TemplatePartLabel( { postId, slug } ) {
const [ title ] = useEntityProp(
'postType',
'wp_template_part',
'title',
postId
);
Comment on lines +34 to +39
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have a strong opinion on this, but figured I would point it out. It seems we are also using this useEntityProp for title in the name-panel component. Maybe it would make sense to move it into index and pass it down to these two child components? But maybe it doesn't really matter and is more simple this way.

Copy link
Contributor Author

@jeyip jeyip Aug 14, 2020

Choose a reason for hiding this comment

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

I don't have a strong opinion either. If this becomes widely shared, I'd err on the side of removing implicit dependencies and moving useEntityProp out of the label component. The consuming component can always pass the value through props, and the label component will only be responsible for rendering text.


const { toolbarId } = useSelect( ( select ) => {
const { getToolbarId } = select( 'core/block-editor' );
return { toolbarId: getToolbarId() };
} );

const labelElement = useRef( null );
const [ toolbarElement, setToolbarElement ] = useState( null );

useEffect( () => {
const toolbar = document.getElementById( toolbarId );
setToolbarElement( toolbar );
}, [ toolbarId ] );

const isOverlapped = detectOverlap( labelElement.current, toolbarElement );
const label = capitalize( title || slug );

return (
<div
className={ cx( 'wp-block-template-part__label-container', {
overlapped: isOverlapped,
} ) }
>
<div className="wp-block-template-part__label-layout">
<div
ref={ labelElement }
className="wp-block-template-part__label-content"
>
<Icon icon="block-default" size={ 13 } />
<span className="wp-block-template-part__label-text">
{ label }
</span>
</div>
</div>
</div>
);
}
42 changes: 42 additions & 0 deletions packages/block-library/src/template-part/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,48 @@
}
}

.wp-block-template-part__label-container {
position: absolute;
bottom: 100%;
width: 100%;

.wp-block-template-part__label-layout {
display: flex;
justify-content: flex-start;
}

.wp-block-template-part__label-content {
display: flex;
align-items: center;
border: $border-width solid $gray-200;
border-bottom: none;
border-top-right-radius: $radius-block-ui;
border-top-left-radius: $radius-block-ui;
padding: 0 6px;
}

.wp-block-template-part__label-text {
font-size: $default-font-size;
padding-left: 4px;
}

&.overlapped {
top: 100%;

.wp-block-template-part__label-content {
display: flex;
align-items: center;
border: $border-width solid $gray-200;
border-top: none;
border-top-right-radius: 0;
border-top-left-radius: 0;
border-bottom-right-radius: $radius-block-ui;
border-bottom-left-radius: $radius-block-ui;
padding: 0 6px;
}
}
}

.is-navigate-mode .is-selected .wp-block-template-part__name-panel {
box-shadow: 0 0 0 $border-width var(--wp-admin-theme-color);

Expand Down