Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
Comment thread
fabiankaegy marked this conversation as resolved.
Resolved
Original file line number Diff line number Diff line change
Expand Up @@ -5,46 +5,7 @@ import { useSelect, useDispatch } from '@wordpress/data';
import { isReusableBlock, isTemplatePart } from '@wordpress/blocks';
import useContentOnlySectionEdit from '../../hooks/use-content-only-section-edit';
import { store as blockEditorStore } from '../../store';

function IsolatedEditButton( {
attributes = {},
onNavigateToEntityRecord,
isTemplatePartBlock,
} ) {
const { ref, theme, slug } = attributes;
const entityId = isTemplatePartBlock
? theme && slug && `${ theme }//${ slug }`
: ref;

const handleClick = () => {
if ( ! entityId ) {
return;
}

onNavigateToEntityRecord( {
postId: entityId,
postType: isTemplatePartBlock ? 'wp_template_part' : 'wp_block',
} );
};

return (
<Stack
direction="column"
className="block-editor-block-inspector-edit-contents"
>
<Button
className="block-editor-block-inspector-edit-contents__button"
__next40pxDefaultSize
variant="secondary"
onClick={ handleClick }
accessibleWhenDisabled
disabled={ ! entityId }
>
{ __( 'Edit original' ) }
</Button>
</Stack>
);
}
import { PrivateBlockInspectorEditOriginal } from './edit-original-slot-fill';

function InlineEditButton( {
clientId,
Expand Down Expand Up @@ -94,14 +55,11 @@ export default function EditContents( { clientId } ) {
stopEditingContentOnlySection,
} = useContentOnlySectionEdit( clientId );

const { block, onNavigateToEntityRecord, canEdit } = useSelect(
const { block, canEdit } = useSelect(
( select ) => {
const { getBlock, getSettings, canEditBlock } =
select( blockEditorStore );
const { getBlock, canEditBlock } = select( blockEditorStore );
return {
block: getBlock( clientId ),
onNavigateToEntityRecord:
getSettings().onNavigateToEntityRecord,
canEdit: canEditBlock( clientId ),
};
},
Expand All @@ -112,27 +70,24 @@ export default function EditContents( { clientId } ) {
return null;
}

const isSyncedPattern = isReusableBlock( block );
const isTemplatePartBlock = isTemplatePart( block );
const shouldUseIsolatedEditor =
( isSyncedPattern || isTemplatePartBlock ) && onNavigateToEntityRecord;

if ( shouldUseIsolatedEditor ) {
return (
<IsolatedEditButton
attributes={ block?.attributes }
onNavigateToEntityRecord={ onNavigateToEntityRecord }
isTemplatePartBlock={ isTemplatePartBlock }
/>
);
}

return (
const inlineEditButton = (
<InlineEditButton
clientId={ clientId }
editedContentOnlySection={ editedContentOnlySection }
editContentOnlySection={ editContentOnlySection }
stopEditingContentOnlySection={ stopEditingContentOnlySection }
/>
);

// Synced patterns and template parts are edited through the entity they
// render, which the editor package handles. Without its fill, edit inline.
if ( isReusableBlock( block ) || isTemplatePart( block ) ) {
return (
<PrivateBlockInspectorEditOriginal.Slot fillProps={ { clientId } }>
{ ( fills ) => ( fills.length ? fills : inlineEditButton ) }
Comment thread
fabiankaegy marked this conversation as resolved.
Resolved
</PrivateBlockInspectorEditOriginal.Slot>
);
}

return inlineEditButton;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { createSlotFill } from '@wordpress/components';

/**
* Where the block inspector shows how to edit the entity a synced pattern or
* template part block renders. Those entities are WordPress-specific, so the
* editor package fills it; the fill receives the block's `clientId`.
*/
export const PrivateBlockInspectorEditOriginal = createSlotFill(
Symbol( 'PrivateBlockInspectorEditOriginal' )
);
2 changes: 2 additions & 0 deletions packages/block-editor/src/private-apis.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
import { getPopulatedCategories } from './components/inserter/block-patterns-tab/utils';
import { PrivateListView } from './components/list-view';
import InspectorControlsLastItem from './components/inspector-controls/last-item';
import { PrivateBlockInspectorEditOriginal } from './components/block-inspector/edit-original-slot-fill';
import { useHasBlockToolbar } from './components/block-toolbar/use-has-block-toolbar';
import { showPlaceholderKey as blockToolbarShowPlaceholderKey } from './components/block-toolbar';
import { cleanEmptyObject, usePrivateStyleOverride } from './hooks/utils';
Expand Down Expand Up @@ -88,6 +89,7 @@ const { getDuotoneFilter } = unlock( globalStylesEnginePrivateApis );
export const privateApis = {};
lock( privateApis, {
...globalStyles,
PrivateBlockInspectorEditOriginal,
ExperimentalBlockCanvas,
BlockCanvasCover,
ExperimentalBlockEditorProvider,
Expand Down
41 changes: 36 additions & 5 deletions packages/block-library/src/block/edit.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
InnerBlocks,
} from '@wordpress/block-editor';
import { privateApis as patternsPrivateApis } from '@wordpress/patterns';
import { getBlockBindingsSource } from '@wordpress/blocks';
import { getBlockBindingsSource, parse } from '@wordpress/blocks';
import { unlock } from '../lock-unlock';

const { useLayoutClasses } = unlock( blockEditorPrivateApis );
Expand Down Expand Up @@ -139,6 +139,7 @@ function ReusableBlockControl( {
}

const EMPTY_OBJECT = {};
const EMPTY_ARRAY = [];

function ReusableBlockEdit( {
name,
Expand All @@ -151,10 +152,40 @@ function ReusableBlockEdit( {
'wp_block',
ref
);
const [ blocks ] = useEntityBlockEditor( 'postType', 'wp_block', {
const [ editableBlocks ] = useEntityBlockEditor( 'postType', 'wp_block', {
id: ref,
} );
const isMissing = hasResolved && ! record;

// A user who can read the pattern but not edit it can't load it in the
// `edit` context. Its `view` context has the content too, so render that.
const isUnavailableToEdit = hasResolved && ! record;
Comment on lines +161 to +163

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should the code check canUser here instead (or useResourcePermissions)? It feels like the more correct thing to do. Maybe less optimal though.

It would imply a situation where hasResolved is true, record is falsey, but the user does have permissions to edit, and I don't know if that can actually happen.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's avoid useResourcePermissions; it overfetches, and in most cases, you just need a single permission check at a time.

Maybe split rendering into two branches. I think Template Parts is already doing something similar.

Cons: An extra OPTIONS request needs to settle before we can start rendering.
Pros: No forbidden request for edit context before we dispatch a request for view.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I kept the edit-context result as the signal. Checking canUser first would add a permission request before every pattern block renders, including for users who can edit it, who are the common case. A missing record with edit permission would mean the pattern doesn't exist, and then the view fetch fails too, so it still ends up "missing".

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks! I tried the split locally (checking canUser first, then rendering an edit or a view branch) and measured it over 15 editor loads each.

For users who can edit the pattern, it made the pattern about 0.9s slower to render (2.2s → 3.1s median from the first pattern request). canUser makes its own OPTIONS request unless a record or list fetch has already recorded the permissions, and that request plus the edit GET then queue one after the other behind the rest of the editor's load. For read-only users the timing was the same either way (about 2.1s); the split only swaps the refused edit request for an OPTIONS one.

Since users who can edit are the common case, I'd keep the current approach. The refused request only happens for read-only users, and only once per pattern. Happy to revisit if you'd rather avoid the 403 anyway.

const { readOnlyRecord, hasResolvedReadOnly } = useSelect(
( select ) => {
if ( ! isUnavailableToEdit ) {
return {};
}
const args = [ 'postType', 'wp_block', ref, { context: 'view' } ];
const { getEntityRecord, hasFinishedResolution } =
select( coreStore );
return {
readOnlyRecord: getEntityRecord( ...args ),
hasResolvedReadOnly: hasFinishedResolution(
'getEntityRecord',
args
),
};
},
[ isUnavailableToEdit, ref ]
);
Comment thread
fabiankaegy marked this conversation as resolved.
Resolved
const readOnlyContent = readOnlyRecord?.content?.raw;
const readOnlyBlocks = useMemo(
() => ( readOnlyContent ? parse( readOnlyContent ) : EMPTY_ARRAY ),
[ readOnlyContent ]
);
const blocks = readOnlyRecord ? readOnlyBlocks : editableBlocks;
Comment thread
fabiankaegy marked this conversation as resolved.
Resolved
const isResolving =
! hasResolved || ( isUnavailableToEdit && ! hasResolvedReadOnly );
Comment thread
fabiankaegy marked this conversation as resolved.
Resolved
const isMissing = isUnavailableToEdit && ! isResolving && ! readOnlyRecord;
Comment thread
fabiankaegy marked this conversation as resolved.
Resolved

const { __unstableMarkLastChangeAsPersistent } =
useDispatch( blockEditorStore );
Expand Down Expand Up @@ -238,7 +269,7 @@ function ReusableBlockEdit( {
);
}

if ( ! hasResolved ) {
if ( isResolving ) {
children = (
<Placeholder>
<Spinner />
Expand All @@ -248,7 +279,7 @@ function ReusableBlockEdit( {

return (
<>
{ hasResolved && ! isMissing && (
{ ! isResolving && ! isMissing && (
<ReusableBlockControl
recordId={ ref }
canOverrideBlocks={ canOverrideBlocks }
Expand Down
28 changes: 21 additions & 7 deletions packages/block-library/src/block/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,30 @@ export const settings = {
return;
}

const entity = select( coreStore ).getEditedEntityRecord(
'postType',
'wp_block',
ref
);
if ( ! entity?.title ) {
const {
getEditedEntityRecord,
getEntityRecord,
hasFinishedResolution,
} = select( coreStore );
let title = getEditedEntityRecord( 'postType', 'wp_block', ref )?.title;
// A user who can't edit the pattern only has its `view` record.
if (
! title &&
hasFinishedResolution( 'getEntityRecord', [
'postType',
'wp_block',
ref,
] )
) {
title = getEntityRecord( 'postType', 'wp_block', ref, {
context: 'view',
} )?.title?.raw;
}
if ( ! title ) {
return;
}

return decodeEntities( entity.title );
return decodeEntities( title );
},
};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import { Button } from '@wordpress/components';
import { Stack } from '@wordpress/ui';
import { __ } from '@wordpress/i18n';
import { useSelect } from '@wordpress/data';
import { isTemplatePart } from '@wordpress/blocks';
import {
privateApis as blockEditorPrivateApis,
store as blockEditorStore,
// @ts-expect-error No exported types
} from '@wordpress/block-editor';
import { store as coreStore } from '@wordpress/core-data';
import { store as editorStore } from '../../store';
import { unlock } from '../../lock-unlock';

const { PrivateBlockInspectorEditOriginal } = unlock( blockEditorPrivateApis );

interface EditorSettings {
onNavigateToEntityRecord?: ( entity: {
postId: number | string;
postType: string;
} ) => void;
}

interface EditOriginalButtonProps {
/** Client ID of the synced pattern or template part block. */
clientId: string;
}

/**
* The block inspector's "Edit original" button for synced patterns and
* template parts, which opens the entity the block renders.
*/
export default function EditOriginalInspectorControls() {
return (
<PrivateBlockInspectorEditOriginal.Fill>
{ ( { clientId }: EditOriginalButtonProps ) => (
<EditOriginalButton clientId={ clientId } />
) }
</PrivateBlockInspectorEditOriginal.Fill>
);
}

function EditOriginalButton( { clientId }: EditOriginalButtonProps ) {
const { postType, postId, canUserEdit, onNavigateToEntityRecord } =
useSelect(
( select ) => {
const block = select( blockEditorStore ).getBlock( clientId );
// Editor settings are typed as a bare `Object`.
const editorSettings = select(
editorStore
).getEditorSettings() as EditorSettings;
const { ref, theme, slug } = block?.attributes ?? {};
const isTemplatePartBlock = !! block && isTemplatePart( block );
const _postType = isTemplatePartBlock
? 'wp_template_part'
: 'wp_block';
const _postId = isTemplatePartBlock
? theme && slug && `${ theme }//${ slug }`
: ref;

return {
postType: _postType,
postId: _postId,
// Matches the block toolbar's "Edit original" button.
canUserEdit:
! _postId ||
!! select( coreStore ).canUser( 'update', {
kind: 'postType',
name: _postType,
id: _postId,
} ),
onNavigateToEntityRecord:
editorSettings.onNavigateToEntityRecord,
};
},
[ clientId ]
);

if ( ! canUserEdit || ! onNavigateToEntityRecord ) {
return null;
}

return (
<Stack
direction="column"
className="block-editor-block-inspector-edit-contents"
>
<Button
className="block-editor-block-inspector-edit-contents__button"
__next40pxDefaultSize
variant="secondary"
onClick={ () =>
onNavigateToEntityRecord( { postId, postType } )
}
accessibleWhenDisabled
disabled={ ! postId }
>
{ __( 'Edit original' ) }
</Button>
</Stack>
);
}
2 changes: 2 additions & 0 deletions packages/editor/src/components/provider/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import EditorKeyboardShortcutsRegister from '../global-keyboard-shortcuts/regist
import PatternRenameModal from '../pattern-rename-modal';
import PatternDuplicateModal from '../pattern-duplicate-modal';
import TemplatePartMenuItems from '../template-part-menu-items';
import EditOriginalInspectorControls from '../edit-original-inspector-controls';
import MediaEditorModalMount from '../media/media-editor-modal';
import { getCanvasWidthByDeviceType } from '../../utils/device-type';

Expand Down Expand Up @@ -451,6 +452,7 @@ export const ExperimentalEditorProvider = withRegistryProvider(
<>
<PatternsMenuItems />
<TemplatePartMenuItems />
<EditOriginalInspectorControls />
Comment thread
fabiankaegy marked this conversation as resolved.
Resolved
{ mode === 'template-locked' && (
<DisableNonPageContentBlocks />
) }
Expand Down
Loading
Loading