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: edit source pattern in focus mode in post and site editors #57036

Merged
merged 32 commits into from
Jan 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
eb0e0af
Move synced patterns global editing to focus mode in editor
glendaviesnz Dec 18, 2023
46f0d79
Add back padding to focus mode and notice to indicate global nature o…
glendaviesnz Dec 14, 2023
026616b
Update experiment wording
glendaviesnz Dec 14, 2023
a5cfffc
Move the edit mode function to the editor package
glendaviesnz Dec 14, 2023
8ec78b4
Change label on edit button
glendaviesnz Dec 14, 2023
4764b56
Remove `pattern-only` mode
glendaviesnz Dec 18, 2023
76904b9
Revert more pattern-only mode code
glendaviesnz Dec 17, 2023
4bb23e7
Another revert
glendaviesnz Dec 17, 2023
501e5d5
Add basic routing to post editor
glendaviesnz Dec 18, 2023
d57e06b
Add document bar and padding
glendaviesnz Dec 18, 2023
de518fa
Fix initialisation of post id and post type
glendaviesnz Dec 18, 2023
5a2a0e7
Only show header bar and padding if edit mode is set to focused
glendaviesnz Dec 18, 2023
e55a163
refine check for edit mode
glendaviesnz Dec 18, 2023
4fb6273
Remove routing
glendaviesnz Dec 19, 2023
5fe18da
Switch to a simple postHistory array for switching back from pattern …
glendaviesnz Dec 21, 2023
88e76a2
move postHistory to a ref
glendaviesnz Dec 19, 2023
afb742b
Move history settings into hook
glendaviesnz Dec 19, 2023
caa3349
refine history stack slightly
glendaviesnz Dec 19, 2023
8fd30e6
Refactor `usePostHistory` to keep all history in a single array. Use …
talldan Dec 19, 2023
e882647
fix edit post specific classname
glendaviesnz Dec 19, 2023
64a462e
Fix iframe classes
glendaviesnz Dec 20, 2023
ab7f344
Editing in focus mode - pass onSelectPost as editor setting
glendaviesnz Dec 21, 2023
917e6a1
Minor tidy ups
glendaviesnz Dec 21, 2023
266c6c6
Move from onSelectPost to getPostLinkProps and add new hook in site e…
glendaviesnz Dec 21, 2023
4a6c7c3
Fixes from code review
glendaviesnz Dec 21, 2023
925efaf
Another fix from review
glendaviesnz Dec 21, 2023
c37010d
Add optional state param to call to history.push
glendaviesnz Dec 21, 2023
de462a0
Remove code duplication in usePostLinkProps hook
glendaviesnz Dec 21, 2023
638e0d2
make sure block editing mode set to default when going to edit original
glendaviesnz Dec 21, 2023
f0c08de
Tidy up edit mode assignment
glendaviesnz Dec 21, 2023
9465f9f
Don't adjust the url if moving to focused mode of entity from post
glendaviesnz Dec 21, 2023
6bc3abb
Change how updating of url is bypassed when switching to focused mode…
glendaviesnz Dec 21, 2023
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
4 changes: 2 additions & 2 deletions lib/experiments-page.php
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,12 @@ function gutenberg_initialize_experiments_settings() {

add_settings_field(
'gutenberg-pattern-partial-syncing',
__( 'Synced patterns partial syncing', 'gutenberg' ),
__( 'Pattern overrides', 'gutenberg' ),
'gutenberg_display_experiment_field',
'gutenberg-experiments',
'gutenberg_experiments_section',
array(
'label' => __( 'Test partial syncing of patterns', 'gutenberg' ),
'label' => __( 'Test overrides in synced patterns', 'gutenberg' ),
'id' => 'gutenberg-pattern-partial-syncing',
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,9 @@ const BlockInspector = ( { showNoBlockSelectedMessage = true } ) => {
blockType: _blockType,
topLevelLockedBlock:
__unstableGetContentLockingParent( _selectedBlockClientId ) ||
( getTemplateLock( _selectedBlockClientId ) === 'contentOnly'
( getTemplateLock( _selectedBlockClientId ) === 'contentOnly' ||
( _selectedBlockName === 'core/block' &&
window.__experimentalPatternPartialSyncing )
Comment on lines +96 to +97
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 we should make sure we consider reviewing this and the similar code (in use-in-between-inserter.js and selectors.js code down the road, as there's a lot of hard coding of values to bend things into a working state. Feels like there should be a better way.

The gist of the problem seems to be that none of the block editing modes seem to be suitable for patterns.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd love to understand the problem more here.

Copy link
Contributor

Choose a reason for hiding this comment

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

At the moment the PR uses useBlockEditingMode to prevent editing inside the pattern block, but it doesn't support exactly what's needed for the pattern block. 'disabled' locks down too much. 'contentOnly' also isn't right. At the moment the PR sets the inner blocks of the pattern to 'disabled' or 'contentOnly' as needed, but that still results in things like the sibling inserter showing.

This part still needs some investigation.

? _selectedBlockClientId
: undefined ),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export function useInBetweenInserter() {
getTemplateLock,
__unstableIsWithinBlockOverlay,
getBlockEditingMode,
getBlockName,
} = useSelect( blockEditorStore );
const { showInsertionPoint, hideInsertionPoint } =
useDispatch( blockEditorStore );
Expand Down Expand Up @@ -75,7 +76,9 @@ export function useInBetweenInserter() {

if (
getTemplateLock( rootClientId ) ||
getBlockEditingMode( rootClientId ) === 'disabled'
getBlockEditingMode( rootClientId ) === 'disabled' ||
( getBlockName( rootClientId ) === 'core/block' &&
window.__experimentalPatternPartialSyncing )
) {
return;
}
Expand Down
7 changes: 5 additions & 2 deletions packages/block-editor/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -2742,8 +2742,11 @@ export const __unstableGetContentLockingParent = createSelector(
while ( state.blocks.parents.has( current ) ) {
current = state.blocks.parents.get( current );
if (
current &&
getTemplateLock( state, current ) === 'contentOnly'
( current &&
getBlockName( state, current ) === 'core/block' &&
window.__experimentalPatternPartialSyncing ) ||
( current &&
getTemplateLock( state, current ) === 'contentOnly' )
) {
result = current;
}
Expand Down
96 changes: 68 additions & 28 deletions packages/block-library/src/block/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,24 @@ import classnames from 'classnames';
*/
import { useRegistry, useSelect, useDispatch } from '@wordpress/data';
import { useRef, useMemo, useEffect } from '@wordpress/element';
import { useEntityProp, useEntityRecord } from '@wordpress/core-data';
import { useEntityRecord, store as coreStore } from '@wordpress/core-data';
import {
Placeholder,
Spinner,
TextControl,
PanelBody,
ToolbarButton,
ToolbarGroup,
} from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import {
useInnerBlocksProps,
__experimentalRecursionProvider as RecursionProvider,
__experimentalUseHasRecursion as useHasRecursion,
InnerBlocks,
InspectorControls,
useBlockProps,
Warning,
privateApis as blockEditorPrivateApis,
store as blockEditorStore,
BlockControls,
} from '@wordpress/block-editor';
import { getBlockSupport, parse } from '@wordpress/blocks';
glendaviesnz marked this conversation as resolved.
Show resolved Hide resolved

Expand Down Expand Up @@ -132,6 +132,15 @@ function getOverridesFromBlocks( blocks, defaultValues ) {
return Object.keys( overrides ).length > 0 ? overrides : undefined;
}

function setBlockEditMode( setEditMode, blocks, mode ) {
blocks.forEach( ( block ) => {
const editMode =
mode || ( isPartiallySynced( block ) ? 'contentOnly' : 'disabled' );
setEditMode( block.clientId, editMode );
setBlockEditMode( setEditMode, block.innerBlocks, mode );
} );
}

export default function ReusableBlockEdit( {
name,
attributes: { ref, overrides },
Expand All @@ -149,14 +158,51 @@ export default function ReusableBlockEdit( {
const isMissing = hasResolved && ! record;
const initialOverrides = useRef( overrides );
const defaultValuesRef = useRef( {} );

const {
replaceInnerBlocks,
__unstableMarkNextChangeAsNotPersistent,
setBlockEditingMode,
} = useDispatch( blockEditorStore );
const { getBlockEditingMode } = useSelect( blockEditorStore );
const { syncDerivedUpdates } = unlock( useDispatch( blockEditorStore ) );

const { innerBlocks, userCanEdit, getBlockEditingMode, getPostLinkProps } =
useSelect(
( select ) => {
const { canUser } = select( coreStore );
const {
getBlocks,
getBlockEditingMode: editingMode,
getSettings,
} = select( blockEditorStore );
const blocks = getBlocks( patternClientId );
const canEdit = canUser( 'update', 'blocks', ref );

// For editing link to the site editor if the theme and user permissions support it.
return {
innerBlocks: blocks,
userCanEdit: canEdit,
getBlockEditingMode: editingMode,
getPostLinkProps:
getSettings().__experimentalGetPostLinkProps,
};
},
[ patternClientId, ref ]
);

const editOriginalProps = getPostLinkProps
? getPostLinkProps( {
postId: ref,
postType: 'wp_block',
canvas: 'edit',
} )
: {};

useEffect(
() => setBlockEditMode( setBlockEditingMode, innerBlocks ),
[ innerBlocks, setBlockEditingMode ]
);
Comment on lines +201 to +204
Copy link
Contributor

Choose a reason for hiding this comment

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

May need to consider using registry.batch here to batch all the action dispatches.

Copy link
Member

@ellatrix ellatrix Feb 5, 2024

Choose a reason for hiding this comment

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

I don't understand this code. Why are we recursively changing blocks on every change?


// Apply the initial overrides from the pattern block to the inner blocks.
useEffect( () => {
const initialBlocks =
Expand Down Expand Up @@ -193,18 +239,6 @@ export default function ReusableBlockEdit( {
syncDerivedUpdates,
] );

const innerBlocks = useSelect(
( select ) => select( blockEditorStore ).getBlocks( patternClientId ),
[ patternClientId ]
);

const [ title, setTitle ] = useEntityProp(
'postType',
'wp_block',
'title',
ref
);

const { alignment, layout } = useInferredLayout(
innerBlocks,
parentLayout
Expand Down Expand Up @@ -247,6 +281,11 @@ export default function ReusableBlockEdit( {
}, blockEditorStore );
}, [ syncDerivedUpdates, patternClientId, registry, setAttributes ] );

const handleEditOriginal = ( event ) => {
setBlockEditMode( setBlockEditingMode, innerBlocks, 'default' );
editOriginalProps.onClick( event );
};

let children = null;

if ( hasAlreadyRendered ) {
Expand Down Expand Up @@ -275,17 +314,18 @@ export default function ReusableBlockEdit( {

return (
<RecursionProvider uniqueId={ ref }>
<InspectorControls>
<PanelBody>
<TextControl
label={ __( 'Name' ) }
value={ title }
onChange={ setTitle }
__nextHasNoMarginBottom
__next40pxDefaultSize
/>
</PanelBody>
</InspectorControls>
{ userCanEdit && editOriginalProps && (
<BlockControls>
<ToolbarGroup>
<ToolbarButton
href={ editOriginalProps.href }
onClick={ handleEditOriginal }
>
{ __( 'Edit original' ) }
</ToolbarButton>
</ToolbarGroup>
</BlockControls>
) }
{ children === null ? (
<div { ...innerBlocksProps } />
) : (
Expand Down
6 changes: 4 additions & 2 deletions packages/edit-post/src/components/browser-url/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ export class BrowserURL extends Component {
}

componentDidUpdate( prevProps ) {
const { postId, postStatus, postType, isSavingPost } = this.props;
const { postId, postStatus, postType, isSavingPost, hasHistory } =
this.props;
const { historyId } = this.state;

// Posts are still dirty while saving so wait for saving to finish
Expand All @@ -56,7 +57,8 @@ export class BrowserURL extends Component {
if (
( postId !== prevProps.postId || postId !== historyId ) &&
Copy link
Contributor

@talldan talldan Jan 2, 2024

Choose a reason for hiding this comment

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

historyId seems to do something similar to usePostHistory, so there might be an opportunity to remove it from this component, replacing it with usePostHistory.

postStatus !== 'auto-draft' &&
postId
postId &&
! hasHistory
Copy link
Contributor

Choose a reason for hiding this comment

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

This line feels mysterious, it'd be good to add a comment about why it was added.

) {
this.setBrowserURL( postId );
}
Expand Down
4 changes: 3 additions & 1 deletion packages/edit-post/src/components/header/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ function Header( { setEntitiesSavedStatesCallback } ) {
isEditingTemplate,
isPublishSidebarOpened,
showIconLabels,
hasHistory,
} = useSelect( ( select ) => {
const { get: getPreference } = select( preferencesStore );
const { getEditorMode } = select( editPostStore );
Expand All @@ -76,6 +77,7 @@ function Header( { setEntitiesSavedStatesCallback } ) {
hasBlockSelection:
!! select( blockEditorStore ).getBlockSelectionStart(),
hasActiveMetaboxes: select( editPostStore ).hasMetaBoxes(),
hasHistory: !! select( editorStore ).getEditorSettings().goBack,
isEditingTemplate:
select( editorStore ).getRenderingMode() === 'template-only',
isPublishSidebarOpened:
Expand Down Expand Up @@ -161,7 +163,7 @@ function Header( { setEntitiesSavedStatesCallback } ) {
isLargeViewport,
} ) }
>
{ isEditingTemplate && <DocumentBar /> }
{ ( isEditingTemplate || hasHistory ) && <DocumentBar /> }
</div>
</motion.div>
<motion.div
Expand Down
4 changes: 3 additions & 1 deletion packages/edit-post/src/components/layout/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ function Layout() {
showBlockBreadcrumbs,
showMetaBoxes,
documentLabel,
hasHistory,
} = useSelect( ( select ) => {
const { getEditorSettings, getPostTypeLabel } = select( editorStore );
const editorSettings = getEditorSettings();
Expand Down Expand Up @@ -199,6 +200,7 @@ function Layout() {
documentLabel: postTypeLabel || _x( 'Document', 'noun' ),
hasBlockSelected:
!! select( blockEditorStore ).getBlockSelectionStart(),
hasHistory: !! getEditorSettings().goBack,
};
}, [] );

Expand Down Expand Up @@ -286,7 +288,7 @@ function Layout() {
return (
<>
<FullscreenMode isActive={ isFullscreenActive } />
<BrowserURL />
<BrowserURL hasHistory={ hasHistory } />
<UnsavedChangesWarning />
<AutosaveMonitor />
<LocalAutosaveMonitor />
Expand Down
44 changes: 34 additions & 10 deletions packages/edit-post/src/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,22 @@ import Layout from './components/layout';
import EditorInitialization from './components/editor-initialization';
import { store as editPostStore } from './store';
import { unlock } from './lock-unlock';
import usePostHistory from './hooks/use-post-history';

const { ExperimentalEditorProvider } = unlock( editorPrivateApis );

function Editor( { postId, postType, settings, initialEdits, ...props } ) {
function Editor( {
postId: initialPostId,
postType: initialPostType,
settings,
initialEdits,
...props
} ) {
const isLargeViewport = useViewportMatch( 'medium' );
glendaviesnz marked this conversation as resolved.
Show resolved Hide resolved
const { currentPost, getPostLinkProps, goBack } = usePostHistory(
initialPostId,
initialPostType
);

const {
hasFixedToolbar,
Expand All @@ -52,22 +63,31 @@ function Editor( { postId, postType, settings, initialEdits, ...props } ) {
const { getEditorSettings } = select( editorStore );
const { getBlockTypes } = select( blocksStore );
const isTemplate = [ 'wp_template', 'wp_template_part' ].includes(
postType
currentPost.postType
);
// Ideally the initializeEditor function should be called using the ID of the REST endpoint.
// to avoid the special case.
let postObject;
if ( isTemplate ) {
const posts = getEntityRecords( 'postType', postType, {
wp_id: postId,
} );
const posts = getEntityRecords(
'postType',
currentPost.postType,
{
wp_id: currentPost.postId,
}
);
postObject = posts?.[ 0 ];
} else {
postObject = getEntityRecord( 'postType', postType, postId );
postObject = getEntityRecord(
'postType',
currentPost.postType,
currentPost.postId
);
}
const supportsTemplateMode =
getEditorSettings().supportsTemplateMode;
const isViewable = getPostType( postType )?.viewable ?? false;
const isViewable =
getPostType( currentPost.postType )?.viewable ?? false;
const canEditTemplate = canUser( 'create', 'templates' );
return {
hasFixedToolbar:
Expand All @@ -89,14 +109,16 @@ function Editor( { postId, postType, settings, initialEdits, ...props } ) {
post: postObject,
};
},
[ postType, postId, isLargeViewport ]
[ currentPost.postType, currentPost.postId, isLargeViewport ]
);

const { updatePreferredStyleVariations } = useDispatch( editPostStore );

const editorSettings = useMemo( () => {
const result = {
...settings,
getPostLinkProps,
goBack,
__experimentalPreferredStyleVariations: {
value: preferredStyleVariations,
onChange: updatePreferredStyleVariations,
Expand Down Expand Up @@ -134,11 +156,13 @@ function Editor( { postId, postType, settings, initialEdits, ...props } ) {
hasInlineToolbar,
focusMode,
isDistractionFree,
keepCaretInsideBlock,
hiddenBlockTypes,
blockTypes,
preferredStyleVariations,
updatePreferredStyleVariations,
keepCaretInsideBlock,
getPostLinkProps,
goBack,
] );

if ( ! post ) {
Expand All @@ -157,7 +181,7 @@ function Editor( { postId, postType, settings, initialEdits, ...props } ) {
>
<ErrorBoundary>
<CommandMenu />
<EditorInitialization postId={ postId } />
<EditorInitialization postId={ currentPost.postId } />
<Layout />
</ErrorBoundary>
<PostLockedModal />
Expand Down
Loading
Loading