Skip to content

Implement pattern overrides behind IS_GUTENBERG_PLUGIN flag #59702

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.

Sign up for GitHub

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

Merged
youknowriad merged 4 commits into from
Mar 11, 2024
Merged

Implement pattern overrides behind IS_GUTENBERG_PLUGIN flag #59702

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c8e951f
Make PHP registration for pattern overrides plugin only
talldan Mar 7, 2024
78fe833
Make JS registration of pattern overrides plugin only
talldan Mar 7, 2024
e9f10ec
Disable overrides in pattern block edit when no source is registered
talldan Mar 7, 2024
ec52b22
Prevent application of pattern override bindings when no source is re…
talldan Mar 7, 2024
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
Prev Previous commit
Next Next commit
Disable overrides in pattern block edit when no source is registered
  • Loading branch information
@talldan
talldan committed Mar 7, 2024
commit e9f10ec2d671b5de84c26f7e6de4428de4415069
53 changes: 38 additions & 15 deletions packages/block-library/src/block/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
BlockControls,
} from '@wordpress/block-editor';
import { privateApis as patternsPrivateApis } from '@wordpress/patterns';
import { parse, cloneBlock } from '@wordpress/blocks';
import { parse, cloneBlock, store as blocksStore } from '@wordpress/blocks';
import { RichTextData } from '@wordpress/rich-text';

/**
Expand Down Expand Up @@ -265,6 +265,7 @@ export default function ReusableBlockEdit( {
getBlockEditingMode,
onNavigateToEntityRecord,
editingMode,
hasPatternOverridesSource,
} = useSelect(
( select ) => {
const { canUser } = select( coreStore );
Expand All @@ -273,6 +274,7 @@ export default function ReusableBlockEdit( {
getSettings,
getBlockEditingMode: _getBlockEditingMode,
} = select( blockEditorStore );
const { getBlockBindingsSource } = unlock( select( blocksStore ) );
const blocks = getBlocks( patternClientId );
const canEdit = canUser( 'update', 'blocks', ref );

Expand All @@ -284,6 +286,9 @@ export default function ReusableBlockEdit( {
onNavigateToEntityRecord:
getSettings().onNavigateToEntityRecord,
editingMode: _getBlockEditingMode( patternClientId ),
hasPatternOverridesSource: !! getBlockBindingsSource(
Copy link
Contributor

@youknowriad youknowriad Mar 8, 2024

Choose a reason for hiding this comment

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

I wonder why you chose to use the selector over process.env.IS_GUTENBERG_PLUGIN

Copy link
Contributor Author

@talldan talldan Mar 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, I was in two minds about it.

My thinking is that this could be maintained to work like this even after the process.env.IS_GUTENBERG_PLUGIN code is removed, as a potential unregisterBindingSource( 'core/pattern-overrides' ) call could result in the same thing.

Actually, I don't think this approach is a good idea, as a WP 6.5 user could re-register pattern overrides, so I think we'll need to add process.env.IS_GUTENBERG_PLUGIN everywhere.

edit: The register action is private at the moment, so maybe this solution is ok. I don't mind implementing it either way though.

Copy link
Contributor

@getdave getdave Mar 11, 2024

Choose a reason for hiding this comment

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

The register action is private at the moment, so maybe this solution is ok.

Given that it's still possible, shall we just put behind the flag?

Copy link
Contributor

@youknowriad youknowriad Mar 11, 2024

Choose a reason for hiding this comment

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

@getdave It's not possible for third-parties, so it doesn't impact.

Personally, I don't mind either way.

Copy link
Contributor

@getdave getdave Mar 11, 2024

Choose a reason for hiding this comment

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

Oh I thought a third party could opt into Private APIs it's just they have to jump through a lot of hoops to do so.

Personally, I don't mind either way.

Good enough for me. @talldan can make a call.

'core/pattern-overrides'
),
};
},
[ patternClientId, ref ]
Expand All @@ -295,13 +300,20 @@ export default function ReusableBlockEdit( {
setBlockEditingMode,
innerBlocks,
// Disable editing if the pattern itself is disabled.
editingMode === 'disabled' ? 'disabled' : undefined
editingMode === 'disabled' || ! hasPatternOverridesSource
? 'disabled'
: undefined
);
}, [ editingMode, innerBlocks, setBlockEditingMode ] );
}, [
editingMode,
innerBlocks,
setBlockEditingMode,
hasPatternOverridesSource,
] );

const canOverrideBlocks = useMemo(
() => hasOverridableBlocks( innerBlocks ),
[ innerBlocks ]
() => hasPatternOverridesSource && hasOverridableBlocks( innerBlocks ),
[ hasPatternOverridesSource, innerBlocks ]
);

const initialBlocks = useMemo(
Expand Down Expand Up @@ -329,19 +341,21 @@ export default function ReusableBlockEdit( {
registry.batch( () => {
setBlockEditingMode( patternClientId, 'default' );
syncDerivedUpdates( () => {
replaceInnerBlocks(
patternClientId,
applyInitialContentValuesToInnerBlocks(
initialBlocks,
initialContent.current,
defaultContent.current,
legacyIdMap.current
)
);
const blocks = hasPatternOverridesSource
? applyInitialContentValuesToInnerBlocks(
initialBlocks,
initialContent.current,
defaultContent.current,
legacyIdMap.current
)
: initialBlocks;

replaceInnerBlocks( patternClientId, blocks );
} );
setBlockEditingMode( patternClientId, originalEditingMode );
} );
}, [
hasPatternOverridesSource,
__unstableMarkNextChangeAsNotPersistent,
patternClientId,
initialBlocks,
Expand Down Expand Up @@ -377,6 +391,9 @@ export default function ReusableBlockEdit( {
// Sync the `content` attribute from the updated blocks to the pattern block.
// `syncDerivedUpdates` is used here to avoid creating an additional undo level.
useEffect( () => {
if ( ! hasPatternOverridesSource ) {
return;
}
const { getBlocks } = registry.select( blockEditorStore );
let prevBlocks = getBlocks( patternClientId );
return registry.subscribe( () => {
Expand All @@ -394,7 +411,13 @@ export default function ReusableBlockEdit( {
} );
}
}, blockEditorStore );
}, [ syncDerivedUpdates, patternClientId, registry, setAttributes ] );
}, [
hasPatternOverridesSource,
syncDerivedUpdates,
patternClientId,
registry,
setAttributes,
] );

const handleEditOriginal = () => {
onNavigateToEntityRecord( {
Expand Down