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.

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

Pattern Overrides: Memoize controls component #63189

Closed
wants to merge 1 commit into from
Closed

Pattern Overrides: Memoize controls component #63189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
134 changes: 75 additions & 59 deletions packages/editor/src/hooks/pattern-overrides.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { privateApis as patternsPrivateApis } from '@wordpress/patterns';
import { createHigherOrderComponent } from '@wordpress/compose';
import { useBlockEditingMode } from '@wordpress/block-editor';
import { useSelect } from '@wordpress/data';
import { memo } from '@wordpress/element';
import { store as blocksStore } from '@wordpress/blocks';

/**
Expand All @@ -25,6 +26,68 @@ const {
PATTERN_SYNC_TYPES,
} = unlock( patternsPrivateApis );

// Split into a separate component to avoid a store subscription
// on every block.
const MemoizedControlsWithStoreSubscription = memo(
function ControlsWithStoreSubscription( props ) {
const blockEditingMode = useBlockEditingMode();
const { hasPatternOverridesSource, isEditingSyncedPattern } = useSelect(
( select ) => {
const { getBlockBindingsSource } = unlock(
select( blocksStore )
);
const { getCurrentPostType, getEditedPostAttribute } =
select( editorStore );

return {
// For editing link to the site editor if the theme and user permissions support it.
hasPatternOverridesSource: !! getBlockBindingsSource(
'core/pattern-overrides'
),
isEditingSyncedPattern:
getCurrentPostType() === PATTERN_TYPES.user &&
getEditedPostAttribute( 'meta' )
?.wp_pattern_sync_status !==
PATTERN_SYNC_TYPES.unsynced &&
getEditedPostAttribute( 'wp_pattern_sync_status' ) !==
PATTERN_SYNC_TYPES.unsynced,
};
},
[]
);

const bindings = props.metadata?.bindings;
const hasPatternBindings =
!! bindings &&
Object.values( bindings ).some(
( binding ) => binding.source === 'core/pattern-overrides'
);

const shouldShowPatternOverridesControls =
isEditingSyncedPattern && blockEditingMode === 'default';
const shouldShowResetOverridesControl =
! isEditingSyncedPattern &&
!! props.metadata?.name &&
blockEditingMode !== 'disabled' &&
hasPatternBindings;

if ( ! hasPatternOverridesSource ) {
return null;
}

return (
<>
{ shouldShowPatternOverridesControls && (
<PatternOverridesControls { ...props } />
) }
{ shouldShowResetOverridesControl && (
<ResetOverridesControl { ...props } />
) }
</>
);
}
);

/**
* Override the default edit UI to include a new block inspector control for
* assigning a partial syncing controls to supported blocks in the pattern editor.
Expand All @@ -41,9 +104,19 @@ const withPatternOverrideControls = createHigherOrderComponent(

return (
<>
<BlockEdit { ...props } />
<BlockEdit key="edit" { ...props } />
{ props.isSelected && isSupportedBlock && (
<ControlsWithStoreSubscription { ...props } />
<MemoizedControlsWithStoreSubscription
Copy link
Member

Choose a reason for hiding this comment

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

Can't we use createBlockEditFilter like the other hooks?

Copy link
Member Author

Choose a reason for hiding this comment

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

This package is different, but I can copy the same logic here. This is still draft PR :)

Copy link
Member

Choose a reason for hiding this comment

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

Oh, maybe we could add a private API for now?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is only the editor.BlockEdit filter in the editor package. Does it make sense to abstract it now?

Copy link
Member

Choose a reason for hiding this comment

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

I think it does at some point anyway, we need to start thinking about the block supports API.

Copy link
Member Author

Choose a reason for hiding this comment

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

In my case, to actually start building it 😅 Hopefully, I can share something before 6.7 betas.

Copy link
Contributor

Choose a reason for hiding this comment

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

Block bindings panels should use it too, in case it helps.

Copy link
Member Author

Choose a reason for hiding this comment

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

@cbravobernal, which component exactly are we talking about?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still working on it, and will move it from block-editor to editor package. But is this one:
#62880

clientId={ props.clientId }
name={ props.name }
metadata={ props.attributes.metadata }
setAttributes={ props.setAttributes }
hasUnsupportedAttributes={
props.name === 'core/image' &&
( !! props.attributes.caption?.length ||
!! props.attributes.href?.length )
}
Comment on lines +114 to +118
Copy link
Member Author

Choose a reason for hiding this comment

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

I had to lift this condition to avoid passing the whole attribute object.

/>
) }
{ isSupportedBlock && <PatternOverridesBlockControls /> }
</>
Expand All @@ -52,63 +125,6 @@ const withPatternOverrideControls = createHigherOrderComponent(
'withPatternOverrideControls'
);

// Split into a separate component to avoid a store subscription
// on every block.
function ControlsWithStoreSubscription( props ) {
const blockEditingMode = useBlockEditingMode();
const { hasPatternOverridesSource, isEditingSyncedPattern } = useSelect(
( select ) => {
const { getBlockBindingsSource } = unlock( select( blocksStore ) );
const { getCurrentPostType, getEditedPostAttribute } =
select( editorStore );

return {
// For editing link to the site editor if the theme and user permissions support it.
hasPatternOverridesSource: !! getBlockBindingsSource(
'core/pattern-overrides'
),
isEditingSyncedPattern:
getCurrentPostType() === PATTERN_TYPES.user &&
getEditedPostAttribute( 'meta' )?.wp_pattern_sync_status !==
PATTERN_SYNC_TYPES.unsynced &&
getEditedPostAttribute( 'wp_pattern_sync_status' ) !==
PATTERN_SYNC_TYPES.unsynced,
};
},
[]
);

const bindings = props.attributes.metadata?.bindings;
const hasPatternBindings =
!! bindings &&
Object.values( bindings ).some(
( binding ) => binding.source === 'core/pattern-overrides'
);

const shouldShowPatternOverridesControls =
isEditingSyncedPattern && blockEditingMode === 'default';
const shouldShowResetOverridesControl =
! isEditingSyncedPattern &&
!! props.attributes.metadata?.name &&
blockEditingMode !== 'disabled' &&
hasPatternBindings;

if ( ! hasPatternOverridesSource ) {
return null;
}

return (
<>
{ shouldShowPatternOverridesControls && (
<PatternOverridesControls { ...props } />
) }
{ shouldShowResetOverridesControl && (
<ResetOverridesControl { ...props } />
) }
</>
);
}

addFilter(
'editor.BlockEdit',
'core/editor/with-pattern-override-controls',
Expand Down
24 changes: 9 additions & 15 deletions packages/patterns/src/components/pattern-overrides-controls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,32 +32,32 @@ function addBindings( bindings ) {
}

function PatternOverridesControls( {
attributes,
metadata,
setAttributes,
name: blockName,
hasUnsupportedAttributes,
} ) {
const controlId = useId();
const [ showAllowOverridesModal, setShowAllowOverridesModal ] =
useState( false );
const [ showDisallowOverridesModal, setShowDisallowOverridesModal ] =
useState( false );

const hasName = !! attributes.metadata?.name;
const defaultBindings = attributes.metadata?.bindings?.__default;
const hasName = !! metadata?.name;
const defaultBindings = metadata?.bindings?.__default;
const hasOverrides =
hasName && defaultBindings?.source === PATTERN_OVERRIDES_BINDING_SOURCE;
const isConnectedToOtherSources =
defaultBindings?.source &&
defaultBindings.source !== PATTERN_OVERRIDES_BINDING_SOURCE;

function updateBindings( isChecked, customName ) {
const prevBindings = attributes?.metadata?.bindings;
const prevBindings = metadata?.bindings;
const updatedBindings = isChecked
? addBindings( prevBindings )
: removeBindings( prevBindings );

const updatedMetadata = {
...attributes.metadata,
...metadata,
bindings: updatedBindings,
};

Expand All @@ -75,12 +75,8 @@ function PatternOverridesControls( {
return null;
}

const hasUnsupportedImageAttributes =
blockName === 'core/image' &&
( !! attributes.caption?.length || !! attributes.href?.length );

const helpText =
! hasOverrides && hasUnsupportedImageAttributes
! hasOverrides && hasUnsupportedAttributes
? __(
`Overrides currently don't support image captions or links. Remove the caption or link first before enabling overrides.`
)
Expand Down Expand Up @@ -108,9 +104,7 @@ function PatternOverridesControls( {
setShowAllowOverridesModal( true );
}
} }
disabled={
! hasOverrides && hasUnsupportedImageAttributes
}
disabled={ ! hasOverrides && hasUnsupportedAttributes }
accessibleWhenDisabled
>
{ hasOverrides
Expand All @@ -122,7 +116,7 @@ function PatternOverridesControls( {

{ showAllowOverridesModal && (
<AllowOverridesModal
initialName={ attributes.metadata?.name }
initialName={ metadata?.name }
onClose={ () => setShowAllowOverridesModal( false ) }
onSave={ ( newName ) => {
updateBindings( true, newName );
Expand Down
2 changes: 1 addition & 1 deletion packages/patterns/src/components/reset-overrides-control.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { __ } from '@wordpress/i18n';
const CONTENT = 'content';

export default function ResetOverridesControl( props ) {
const name = props.attributes.metadata?.name;
const name = props.metadata?.name;
const registry = useRegistry();
const isOverriden = useSelect(
( select ) => {
Expand Down
Loading