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

Register each reference widget as a separate block #25741

Closed
Closed
Show file tree
Hide file tree
Changes from all 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 lib/widgets.php
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ function gutenberg_get_legacy_widget_settings() {
null,
'isReferenceWidget' => false,
'isHidden' => in_array( $class, $widgets_to_exclude_from_legacy_widget_block, true ),
'blockName' => 'core/legacy-widget',
);
}
}
Expand All @@ -195,6 +196,7 @@ function gutenberg_get_legacy_widget_settings() {
'name' => html_entity_decode( $widget_obj['name'] ),
'description' => html_entity_decode( wp_widget_description( $widget_id ) ),
'isReferenceWidget' => true,
'blockName' => 'core/legacy-widget-' . preg_replace( '#[^a-zA-Z0-9-]#', '-', $widget_id ),
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we've ever done this but I wonder if we should use sub-namespaces here? In other words, the name would be core/legacy-widget/marquee.

);
}
}
Expand Down Expand Up @@ -288,4 +290,3 @@ function change_post_template_to_widget_preview( $template ) {
return $template;
}
add_filter( 'template_include', 'change_post_template_to_widget_preview' );

27 changes: 6 additions & 21 deletions packages/edit-widgets/src/blocks/legacy-widget/edit/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ import { get, omit } from 'lodash';
/**
* WordPress dependencies
*/
import { createBlock } from '@wordpress/blocks';
import { useState } from '@wordpress/element';
import { Button, PanelBody, ToolbarGroup } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { withSelect } from '@wordpress/data';
import { useDispatch, withSelect } from '@wordpress/data';
import { BlockControls, InspectorControls } from '@wordpress/block-editor';
import { update } from '@wordpress/icons';

Expand All @@ -23,6 +24,7 @@ import WidgetPreview from './widget-preview';
function LegacyWidgetEdit( {
attributes,
availableLegacyWidgets,
clientId,
hasPermissionsToManageWidgets,
prerenderedEditForm,
setAttributes,
Expand All @@ -33,6 +35,7 @@ function LegacyWidgetEdit( {
const [ hasEditForm, setHasEditForm ] = useState( true );
const [ isPreview, setIsPreview ] = useState( false );
const shouldHidePreview = ! isPreview && hasEditForm;
const { replaceBlocks } = useDispatch( 'core/block-editor' );

function changeWidget() {
switchToEdit();
Expand All @@ -59,26 +62,8 @@ function LegacyWidgetEdit( {
availableLegacyWidgets={ availableLegacyWidgets }
hasPermissionsToManageWidgets={ hasPermissionsToManageWidgets }
onChangeWidget={ ( newWidget ) => {
const {
isReferenceWidget,
id_base: idBase,
} = availableLegacyWidgets[ newWidget ];

if ( isReferenceWidget ) {
setAttributes( {
instance: {},
idBase,
referenceWidgetName: newWidget,
widgetClass: undefined,
} );
} else {
setAttributes( {
instance: {},
idBase,
referenceWidgetName: undefined,
widgetClass: newWidget,
} );
}
const { blockName } = availableLegacyWidgets[ newWidget ];
replaceBlocks( clientId, createBlock( blockName, {} ) );
} }
/>
);
Expand Down
35 changes: 31 additions & 4 deletions packages/edit-widgets/src/blocks/legacy-widget/edit/placeholder.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { pickBy, isEmpty, map } from 'lodash';
import { isEmpty, map, some } from 'lodash';

/**
* WordPress dependencies
Expand All @@ -11,17 +11,44 @@ import { __ } from '@wordpress/i18n';
import { SelectControl, Placeholder } from '@wordpress/components';
import { BlockIcon } from '@wordpress/block-editor';
import { brush } from '@wordpress/icons';
import { useSelect } from '@wordpress/data';

export default function LegacyWidgetPlaceholder( {
availableLegacyWidgets,
currentWidget,
hasPermissionsToManageWidgets,
onChangeWidget,
} ) {
const visibleLegacyWidgets = useMemo(
() => pickBy( availableLegacyWidgets, ( { isHidden } ) => ! isHidden ),
[ availableLegacyWidgets ]
const blocksByClientId = useSelect( ( select ) =>
select( 'core/block-editor' ).getBlocksByClientId(
select( 'core/block-editor' ).getClientIdsWithDescendants()
)
);

const visibleLegacyWidgets = useMemo( () => {
const visible = {};
for ( const widgetName in availableLegacyWidgets ) {
const {
isReferenceWidget,
blockName,
isHidden,
} = availableLegacyWidgets[ widgetName ];
if ( isHidden ) {
continue;
}
if ( isReferenceWidget ) {
const blockExists = some( blocksByClientId, {
Copy link
Member

Choose a reason for hiding this comment

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

There's probably a nicer way to do this using canInsertBlock or some such.

name: blockName,
} );
if ( blockExists ) {
continue;
}
}
visible[ widgetName ] = availableLegacyWidgets[ widgetName ];
}
return visible;
}, [ availableLegacyWidgets, blocksByClientId ] );

let placeholderContent;
if ( ! hasPermissionsToManageWidgets ) {
placeholderContent = __(
Expand Down
97 changes: 64 additions & 33 deletions packages/edit-widgets/src/blocks/legacy-widget/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* WordPress dependencies
*/
import { widget as icon } from '@wordpress/icons';
import { __ } from '@wordpress/i18n';
import { __, sprintf } from '@wordpress/i18n';

/**
* Internal dependencies
Expand All @@ -23,6 +23,12 @@ export const settings = {
transforms,
};

const blockBasis = {
metadata,
name,
settings,
};

/**
* Special factory function created specifically for the legacy-widget block. For every other block, JS module exports
* are used for registration. In case of this special block, the return value of the create function is used instead.
Expand All @@ -34,49 +40,74 @@ export const settings = {
* @return {Object} Block object.
*/
export const create = ( editorSettings ) => {
const legacyWidgets = editorSettings?.availableLegacyWidgets ?? {};
const legacyWidgets = Object.entries(
editorSettings.availableLegacyWidgets
).filter( ( [ , widget ] ) => ! widget.isHidden );

return [
{
...blockBasis,
settings: {
...blockBasis.settings,
variations: legacyWidgets
Copy link
Member

Choose a reason for hiding this comment

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

Should we go all in with the block per widget approach so that there's only one type of block to test and debug?

.filter( ( [ , widget ] ) => ! widget.isReferenceWidget )
.map( ( [ className, widget ] ) =>
legacyWidgetToBlockVariation( className, widget )
),
},
},
...legacyWidgets
.filter( ( [ , widget ] ) => widget.isReferenceWidget )
.map( ( [ className, widget ] ) =>
referenceLegacyWidgetsToBlockType( className, widget )
),
];
};

function referenceLegacyWidgetsToBlockType( className, widget ) {
const attrs = blockBasis.metadata.attributes;
return {
metadata,
name,
...blockBasis,
name: widget.blockName,
metadata: {
...blockBasis.metadata,
name: widget.blockName,
supports: {
...blockBasis.metadata.supports,
multiple: false,
},
attributes: {
...attrs,
referenceWidgetName: {
...attrs.referenceWidgetName,
default: className,
},
instance: {
...attrs.instance,
default: {},
},
},
},
Comment on lines +70 to +90
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it's time we bring immer into our toolchains 😅

Or before that happens, could we just deepClone the object and mutate it here?

settings: {
...settings,
variations: legacyWidgetsToBlockVariations( legacyWidgets ),
...blockBasis.settings,
title: widget.name,
// translators: %s: widget label e.g: "Marquee".
description: sprintf( __( 'Displays a %s widget' ), widget.name ),
},
};
};

function legacyWidgetsToBlockVariations( availableLegacyWidgets ) {
const variations = [];
for ( const className in availableLegacyWidgets ) {
const widget = availableLegacyWidgets[ className ];
if ( widget.isHidden ) {
continue;
}
variations.push( legacyWidgetToBlockVariation( className, widget ) );
}
return variations;
}

function legacyWidgetToBlockVariation( className, widget ) {
const blockVariation = {
attributes: {},
return {
attributes: {
idBase: widget.id_base,
widgetClass: className,
instance: {},
},
category: 'widgets',
description: widget.description,
icon: settings.icon,
name: className,
title: widget.name,
};
if ( widget.isReferenceWidget ) {
blockVariation.attributes = {
referenceWidgetName: className,
instance: {},
};
} else {
blockVariation.attributes = {
idBase: widget.id_base,
widgetClass: className,
instance: {},
};
}
return blockVariation;
}
12 changes: 11 additions & 1 deletion packages/edit-widgets/src/blocks/legacy-widget/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,22 @@
* Register legacy widget block.
*/
function register_block_core_legacy_widget() {
register_block_type_from_metadata(
$block_type = register_block_type_from_metadata(
Copy link
Member

Choose a reason for hiding this comment

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

Does using block metadata give us anything at this point? Should we do away with registering the block using metadata altogether and use register_block_type? No need, I think, to pretend that this is a normal block with normal block conventions 🙂

__DIR__ . '/legacy-widget',
array(
'render_callback' => 'render_block_core_legacy_widget',
)
);
$settings = gutenberg_get_legacy_widget_settings();
$legacy_widgets = $settings['availableLegacyWidgets'];
foreach ( $legacy_widgets as $widget_id => $legacy_widget ) {
if ( ! $legacy_widget['isReferenceWidget'] ) {
continue;
}
$legacy_block = clone $block_type;
Copy link
Member

Choose a reason for hiding this comment

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

Personally I'd find array_merge easier to understand than cloning and mutating objects.

$legacy_block->name = $legacy_widget['blockName'];
WP_Block_Type_Registry::get_instance()->register( $legacy_block );
}
}

/**
Expand Down
5 changes: 3 additions & 2 deletions packages/edit-widgets/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
*/
import './store';
import './hooks';
import { create as createLegacyWidget } from './blocks/legacy-widget';
import { create as createLegacyWidgetBlocks } from './blocks/legacy-widget';
import * as widgetArea from './blocks/widget-area';
import EditWidgetsInitializer from './components/edit-widgets-initializer';

Expand All @@ -31,7 +31,8 @@ export function initialize( id, settings ) {
registerCoreBlocks();
if ( process.env.GUTENBERG_PHASE === 2 ) {
__experimentalRegisterExperimentalCoreBlocks( settings );
registerBlock( createLegacyWidget( settings ) );
const blocks = createLegacyWidgetBlocks( settings );
blocks.forEach( registerBlock );
registerBlock( widgetArea );
}
render(
Expand Down