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

Patterns: disable editing of pattern entity in block editor #56534

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 5 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
3 changes: 2 additions & 1 deletion packages/block-editor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -867,13 +867,14 @@ function MyBlock( { attributes, setAttributes } ) {
}
```

`mode` can be one of three options:
`mode` can be one of four options:

- `'disabled'`: Prevents editing the block entirely, i.e. it cannot be
selected.
- `'contentOnly'`: Hides all non-content UI, e.g. auxiliary controls in the
toolbar, the block movers, block settings.
- `'default'`: Allows editing the block as normal.
- `'syncedPattern'`: Restricts editing of synced pattern blocks to only child blocks that have attribute connections set.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to avoid adding this new option?

I had hoped it would be possible to have the pattern block as disabled and then the selected connected inner blocks as contentOnly, with it all orchestrated by the pattern 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'd be good to document where disabled isn't working well, and then it makes it easier to consider what options there are.

As useBlockEditingMode is a public API, there does need to be some careful consideration of the options added to it. Something like syncedPattern might be useful internally as a convenient way to solve this problem, but I can't see how plugins would use it.

Copy link
Contributor Author

@glendaviesnz glendaviesnz Nov 27, 2023

Choose a reason for hiding this comment

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

Yeh, I did explore that, but it seemed like a slightly hacky modification of disabled - but as you note that may be better for internal use rather than extending the external API. It is probably easier to put up a separate PR that uses useBlockEditingMode( 'disabled' ) than try and explain why it seemed slightly hacky. I will do that tomorrow. It is worth doing that I think to make it easier to decide which is the best approach.

I had hoped it would be possible to have the pattern block as disabled and then the selected connected inner blocks as contentOnly, with it all orchestrated by the pattern block.

That is basically what this approach will extend to once we have the attribute connections merged in - currently it is just setting all the children as disabled in order to limit the scope to disabling the inline editing of the synced patterns which can be merged as an independent feature to the partial syncing.

Copy link
Contributor Author

@glendaviesnz glendaviesnz Nov 27, 2023

Choose a reason for hiding this comment

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

I will also add some commits to each with the connected attributes included as it will be useful to see how the different approaches look with the connected innerBlocks actually wired up - should't take much extra work to do that.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is probably easier to put up a separate PR that uses useBlockEditingMode( 'disabled' ) than try and explain why it seemed slightly hacky. I will do that tomorrow. It is worth doing that I think to make it easier to decide which is the best approach.

Thanks, that would really help weigh up the trade-offs.


The mode is inherited by all of the block's inner blocks, unless they have
their own mode.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { store as blockEditorStore } from '../../store';
import { BlockListBlockContext } from '../block-list/block-list-block-context';

/**
* @typedef {'disabled'|'contentOnly'|'default'} BlockEditingMode
* @typedef {'disabled'|'contentOnly'|'default'|'syncedPattern'} BlockEditingMode
*/

/**
Expand All @@ -26,13 +26,14 @@ import { BlockListBlockContext } from '../block-list/block-list-block-context';
* }
* ```
*
* `mode` can be one of three options:
* `mode` can be one of four options:
*
* - `'disabled'`: Prevents editing the block entirely, i.e. it cannot be
* selected.
* - `'contentOnly'`: Hides all non-content UI, e.g. auxiliary controls in the
* toolbar, the block movers, block settings.
* - `'default'`: Allows editing the block as normal.
* - `'syncedPattern'`: Restricts editing of synced pattern blocks to only child blocks that have attribute connections set.
*
* The mode is inherited by all of the block's inner blocks, unless they have
* their own mode.
Expand Down
7 changes: 5 additions & 2 deletions packages/block-editor/src/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ function BlockListBlock( {
);
const { removeBlock } = useDispatch( blockEditorStore );
const onRemove = useCallback( () => removeBlock( clientId ), [ clientId ] );
const isSyncedPatternChild =
name !== 'core/block' && blockEditingMode === 'syncedPattern';

const parentLayout = useLayout() || {};

Expand Down Expand Up @@ -142,7 +144,7 @@ function BlockListBlock( {

const blockType = getBlockType( name );

if ( blockEditingMode === 'disabled' ) {
if ( blockEditingMode === 'disabled' || isSyncedPatternChild ) {
wrapperProps = {
...wrapperProps,
tabIndex: -1,
Expand Down Expand Up @@ -216,7 +218,8 @@ function BlockListBlock( {
clientId,
className: classnames(
{
'is-editing-disabled': blockEditingMode === 'disabled',
'is-editing-disabled':
blockEditingMode === 'disabled' || isSyncedPatternChild,
'is-content-locked-temporarily-editing-as-blocks':
isTemporarilyEditingAsBlocks,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ export function useInBetweenInserter() {

if (
getTemplateLock( rootClientId ) ||
getBlockEditingMode( rootClientId ) === 'disabled'
getBlockEditingMode( rootClientId ) === 'disabled' ||
getBlockEditingMode( rootClientId ) === 'syncedPattern'
) {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/block-lock/modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import useBlockDisplayInformation from '../use-block-display-information';
import { store as blockEditorStore } from '../../store';

// Entity based blocks which allow edit locking
const ALLOWS_EDIT_LOCKING = [ 'core/block', 'core/navigation' ];
const ALLOWS_EDIT_LOCKING = [ 'core/navigation' ];

function getTemplateLockValue( lock ) {
// Prevents all operations.
Expand Down
6 changes: 4 additions & 2 deletions packages/block-editor/src/components/block-toolbar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ const BlockToolbar = ( { hideDragHandle } ) => {
const isSynced =
isReusableBlock( blockType ) || isTemplatePart( blockType );

const isSyncedPattern = isReusableBlock( blockType );
const classes = classnames( 'block-editor-block-toolbar', {
'is-synced': isSynced,
} );
Expand All @@ -101,7 +102,8 @@ const BlockToolbar = ( { hideDragHandle } ) => {
isLargeViewport &&
blockEditingMode === 'default' && <BlockParentSelector /> }
{ ( shouldShowVisualToolbar || isMultiToolbar ) &&
blockEditingMode === 'default' && (
( blockEditingMode === 'default' ||
blockEditingMode === 'syncedPattern' ) && (
<div ref={ nodeRef } { ...showHoveredOrFocusedGestures }>
<ToolbarGroup className="block-editor-block-toolbar__block-controls">
<BlockSwitcher clientIds={ blockClientIds } />
Expand Down Expand Up @@ -148,7 +150,7 @@ const BlockToolbar = ( { hideDragHandle } ) => {
</>
) }
<BlockEditVisuallyButton clientIds={ blockClientIds } />
{ blockEditingMode === 'default' && (
{ ( blockEditingMode === 'default' || isSyncedPattern ) && (
<BlockSettingsMenu clientIds={ blockClientIds } />
) }
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ export default function BlockContextualToolbar( {
const hasAnyBlockControls = useHasAnyBlockControls();
if (
! isToolbarEnabled ||
( blockEditingMode !== 'default' && ! hasAnyBlockControls )
( blockEditingMode !== 'syncedPattern' &&
blockEditingMode !== 'default' &&
! hasAnyBlockControls )
) {
return null;
}
Expand Down
8 changes: 6 additions & 2 deletions packages/block-editor/src/components/list-view/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ function ListViewBlock( {
// List View respects this by also hiding the block settings menu.
hasBlockSupport( blockName, '__experimentalToolbar', true ) &&
// Don't show the settings menu if block is disabled or content only.
blockEditingMode === 'default';
( blockEditingMode === 'default' ||
( blockName === 'core/block' &&
blockEditingMode === 'syncedPattern' ) );
const instanceId = useInstanceId( ListViewBlock );
const descriptionId = `list-view-block-select-button__${ instanceId }`;
const blockPositionDescription = getBlockPositionDescription(
Expand Down Expand Up @@ -243,7 +245,9 @@ function ListViewBlock( {
path={ path }
id={ `list-view-${ listViewInstanceId }-block-${ clientId }` }
data-block={ clientId }
data-expanded={ canEdit ? isExpanded : undefined }
data-expanded={
canEdit && blockName !== 'core/block' ? isExpanded : undefined
}
ref={ rowRef }
>
<TreeGridCell
Expand Down
41 changes: 41 additions & 0 deletions packages/block-library/src/block/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,16 @@ import {
useEntityBlockEditor,
useEntityProp,
useEntityRecord,
store as coreStore,
} from '@wordpress/core-data';
import { useSelect } from '@wordpress/data';
import {
Placeholder,
Spinner,
TextControl,
PanelBody,
ToolbarButton,
ToolbarGroup,
} from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import {
Expand All @@ -27,8 +31,13 @@ import {
useBlockProps,
Warning,
privateApis as blockEditorPrivateApis,
useBlockEditingMode,
BlockControls,
store as editorStore,
} from '@wordpress/block-editor';

import { useRef, useMemo } from '@wordpress/element';
import { addQueryArgs } from '@wordpress/url';

/**
* Internal dependencies
Expand Down Expand Up @@ -72,6 +81,31 @@ export default function ReusableBlockEdit( {
attributes: { ref },
__unstableParentLayout: parentLayout,
} ) {
const editUrl = useSelect(
( select ) => {
const { canUser } = select( coreStore );
const { getSettings } = select( editorStore );

const isBlockTheme = getSettings().__unstableIsBlockBasedTheme;
const defaultUrl = addQueryArgs( 'post.php', {
action: 'edit',
post: ref,
} );
const siteEditorUrl = addQueryArgs( 'site-editor.php', {
postType: 'wp_block',
postId: ref,
categoryType: 'pattern',
canvas: 'edit',
} );

// For editing link to the site editor if the theme and user permissions support it.
return canUser( 'read', 'templates' ) && isBlockTheme
? siteEditorUrl
: defaultUrl;
},
[ ref ]
);
useBlockEditingMode( 'syncedPattern' );
const hasAlreadyRendered = useHasRecursion( ref );
const { record, hasResolved } = useEntityRecord(
'postType',
Expand Down Expand Up @@ -141,6 +175,13 @@ export default function ReusableBlockEdit( {

return (
<RecursionProvider uniqueId={ ref }>
<BlockControls>
<ToolbarGroup>
<ToolbarButton href={ editUrl }>
{ __( 'Edit' ) }
</ToolbarButton>
</ToolbarGroup>
</BlockControls>
<InspectorControls>
<PanelBody>
<TextControl
Expand Down
Loading