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

Fix: Legacy widget: callback widgets with a edit form. #14395

Closed
wants to merge 1 commit into from
Closed

Fix: Legacy widget: callback widgets with a edit form. #14395

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
160 changes: 128 additions & 32 deletions lib/class-wp-rest-widget-updater-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,18 @@ public function __construct() {
public function register_routes() {
register_rest_route(
$this->namespace,
// Regex representing a PHP class extracted from http://php.net/manual/en/language.oop5.basic.php.
'/' . $this->rest_base . '/(?P<identifier>[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)/',
'/' . $this->rest_base . '/(?P<identifier>[\w-_]+)/',
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain why this is changing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @aduth, previously the identifier was only the name of a PHP class. Now the identifier can be the name of a PHP class or the instance id of a widget (for callback widgets), so the regex needed this update to match instance ids.

array(
'args' => array(
'identifier' => array(
'identifier' => array(
'description' => __( 'Class name of the widget.', 'gutenberg' ),
'type' => 'string',
'required' => true,
),
'is_callback_widget' => array(
'description' => __( 'Flag indicating if the widget is registered using register_sidebar_widget or is a subclass of WP_Widget.', 'gutenberg' ),
'type' => 'boolean',
'default' => false,
),
),
array(
Expand Down Expand Up @@ -76,53 +81,110 @@ public function compute_new_widget_permissions_check() {
}

/**
* Returns the new widget instance and the form that represents it.
* Checks if the widget being referenced is valid.
*
* @since 5.2.0
* @access public
* @param string $identifier Widget id for callback widgets or widget class name for class widgets.
* @param boolean $is_callback_widget If true the widget is a back widget if false the widget is a class widget.
*
* @param WP_REST_Request $request Full details about the request.
* @return WP_REST_Response|WP_Error Response object on success, or WP_Error object on failure.
* @return boolean True if the widget being referenced exists and false otherwise.
*/
public function compute_new_widget( $request ) {
$url_params = $request->get_url_params();

$widget = $request->get_param( 'identifier' );

private function is_valid_widget( $identifier, $is_callback_widget ) {
if ( $is_callback_widget ) {
global $wp_registered_widgets;
return isset( $wp_registered_widgets[ $identifier ] );
}
global $wp_widget_factory;
return isset( $wp_widget_factory->widgets[ $identifier ] ) &&
( $wp_widget_factory->widgets[ $identifier ] instanceof WP_Widget );
}

/**
* Computes an array with instance changes cleaned of widget specific prefixes and sufixes.
*
* @since 5.7.0
* @param string $id_base Widget ID Base.
* @param string $id Widget instance identifier.
* @param array $instance_changes Array with the form values being being changed.
*
* @return array An array based on $instance_changes whose keys have the widget specific sufixes and prefixes removed.
*/
private function parse_instance_changes( $id_base, $id, $instance_changes ) {
$instance_changes_parsed = array();
$start_position = strlen( 'widget-' . $id_base . '[' . $id . '][' );
foreach ( $instance_changes as $key => $value ) {
$key_parsed = substr( $key, $start_position, -1 );
$instance_changes_parsed[ $key_parsed ] = $value;
}
return $instance_changes_parsed;
}

/**
* Returns the bew callback widget form.
*
* @since 5.7.0
* @param string $identifier Widget id for callback widgets or widget class name for class widgets.
* @param array $instance_changes Array with the form values being being changed.
*
* @return WP_REST_Response Response object.
*/
private function compute_new_widget_handle_callback_widgets( $identifier, $instance_changes ) {
global $wp_registered_widget_controls;
$form = '';
if (
null === $widget ||
! isset( $wp_widget_factory->widgets[ $widget ] ) ||
! ( $wp_widget_factory->widgets[ $widget ] instanceof WP_Widget )
isset( $wp_registered_widget_controls[ $identifier ]['callback'] ) &&
is_callable( $wp_registered_widget_controls[ $identifier ]['callback'] )
) {
return new WP_Error(
'widget_invalid',
__( 'Invalid widget.', 'gutenberg' ),
array(
'status' => 404,
)
);
$control = $wp_registered_widget_controls[ $identifier ];
$_POST = array_merge( $_POST, $instance_changes );
ob_start();
call_user_func_array( $control['callback'], $control['params'] );
$form = ob_get_clean();
}

$widget_obj = $wp_widget_factory->widgets[ $widget ];
return rest_ensure_response(
array(
'instance' => array(),
'form' => $form,
'id_base' => $identifier,
'id' => $identifier,
)
);
}

$instance = $request->get_param( 'instance' );
/**
* Returns the new class widget instance and the form that represents it.
*
* @since 5.7.0
* @access public
*
* @param string $identifier Widget id for callback widgets or widget class name for class widgets.
* @param array $instance Previous widget instance.
* @param array $instance_changes Array with the form values being being changed.
* @param string $id_to_use Identifier of the specific widget instance.
* @return WP_REST_Response Response object on success, or WP_Error object on failure.
*/
private function compute_new_widget_handle_class_widgets( $identifier, $instance, $instance_changes, $id_to_use ) {
if ( null === $instance ) {
$instance = array();
}
$id_to_use = $request->get_param( 'id_to_use' );
if ( null === $id_to_use ) {
$id_to_use = -1;
}

global $wp_widget_factory;
$widget_obj = $wp_widget_factory->widgets[ $identifier ];

$widget_obj->_set( $id_to_use );
$id_base = $widget_obj->id_base;
$id = $widget_obj->id;
ob_start();

$instance_changes = $request->get_param( 'instance_changes' );
if ( null !== $instance_changes ) {
$old_instance = $instance;
$instance = $widget_obj->update( $instance_changes, $old_instance );
$instance_changes = $this->parse_instance_changes( $id_base, $id_to_use, $instance_changes );
$old_instance = $instance;
$instance = $widget_obj->update( $instance_changes, $old_instance );

/**
* Filters a widget's settings before saving.
*
Expand Down Expand Up @@ -166,10 +228,7 @@ public function compute_new_widget( $request ) {
*/
do_action_ref_array( 'in_widget_form', array( &$widget_obj, &$return, $instance ) );
}

$id_base = $widget_obj->id_base;
$id = $widget_obj->id;
$form = ob_get_clean();
$form = ob_get_clean();

return rest_ensure_response(
array(
Expand All @@ -180,6 +239,43 @@ public function compute_new_widget( $request ) {
)
);
}

/**
* Returns the new widget instance and the form that represents it.
*
* @since 5.7.0
* @access public
*
* @param WP_REST_Request $request Full details about the request.
* @return WP_REST_Response|WP_Error Response object on success, or WP_Error object on failure.
*/
public function compute_new_widget( $request ) {
$identifier = $request->get_param( 'identifier' );
$is_callback_widget = $request->get_param( 'is_callback_widget' );
Copy link
Member

Choose a reason for hiding this comment

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

I believe this should be included in args for the register_routes function?

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 was not sure if args should only be used for arguments part of the route itself e.g: /(?P[\w-_]+)/. But in my tests, it seems args also work on arguments part of the post data, so I updated the code to follow your suggestion.


if ( ! $this->is_valid_widget( $identifier, $is_callback_widget ) ) {
return new WP_Error(
'widget_invalid',
__( 'Invalid widget.', 'gutenberg' ),
array(
'status' => 404,
)
);
}

if ( $is_callback_widget ) {
return $this->compute_new_widget_handle_callback_widgets(
$identifier,
$request->get_param( 'instance_changes' )
);
}
return $this->compute_new_widget_handle_class_widgets(
$identifier,
$request->get_param( 'instance' ),
$request->get_param( 'instance_changes' ),
$request->get_param( 'id_to_use' )
);
}
}
/**
* End: Include for phase 2
Expand Down
14 changes: 5 additions & 9 deletions packages/block-library/src/legacy-widget/edit/dom-manager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,28 +110,24 @@ class LegacyWidgetEditDomManager extends Component {

retrieveUpdatedInstance() {
if ( this.formRef.current ) {
const { idBase, widgetNumber } = this.props;
const form = this.formRef.current;
const formData = new window.FormData( form );
const updatedInstance = {};
const keyPrefixLength = `widget-${ idBase }[${ widgetNumber }][`.length;
const keySuffixLength = `]`.length;
for ( const rawKey of formData.keys() ) {
for ( const key of formData.keys() ) {
// This fields are added to the form because the widget JavaScript code may use this values.
// They are not relevant for the update mechanism.
if ( includes(
[ 'widget-id', 'id_base', 'widget_number', 'multi_number', 'add_new' ],
rawKey,
key,
) ) {
continue;
}
const keyParsed = rawKey.substring( keyPrefixLength, rawKey.length - keySuffixLength );

const value = formData.getAll( rawKey );
const value = formData.getAll( key );
if ( value.length > 1 ) {
updatedInstance[ keyParsed ] = value;
updatedInstance[ key ] = value;
} else {
updatedInstance[ keyParsed ] = value[ 0 ];
updatedInstance[ key ] = value[ 0 ];
}
}
return updatedInstance;
Expand Down
13 changes: 9 additions & 4 deletions packages/block-library/src/legacy-widget/edit/handler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,19 @@ class LegacyWidgetEditHandler extends Component {

componentDidMount() {
this.isStillMounted = true;
this.requestWidgetUpdater();
this.requestWidgetUpdater( undefined, ( response ) => {
this.props.onInstanceChange( null, !! response.form );
} );
}

componentDidUpdate( prevProps ) {
if (
prevProps.instance !== this.props.instance &&
this.instanceUpdating !== this.props.instance
) {
this.requestWidgetUpdater();
this.requestWidgetUpdater( undefined, ( response ) => {
this.props.onInstanceChange( null, !! response.form );
} );
}
if ( this.instanceUpdating === this.props.instance ) {
this.instanceUpdating = null;
Expand Down Expand Up @@ -81,12 +85,12 @@ class LegacyWidgetEditHandler extends Component {
onInstanceChange( instanceChanges ) {
this.requestWidgetUpdater( instanceChanges, ( response ) => {
this.instanceUpdating = response.instance;
this.props.onInstanceChange( response.instance );
this.props.onInstanceChange( response.instance, !! response.form );
} );
}

requestWidgetUpdater( instanceChanges, callback ) {
const { identifier, instanceId, instance } = this.props;
const { identifier, instanceId, instance, isCallbackWidget } = this.props;
if ( ! identifier ) {
return;
}
Expand All @@ -98,6 +102,7 @@ class LegacyWidgetEditHandler extends Component {
instance,
// use negative ids to make sure the id does not exist on the database.
id_to_use: instanceId * -1,
is_callback_widget: isCallbackWidget,
instance_changes: instanceChanges,
},
method: 'POST',
Expand Down
32 changes: 22 additions & 10 deletions packages/block-library/src/legacy-widget/edit/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class LegacyWidgetEdit extends Component {
constructor() {
super( ...arguments );
this.state = {
hasEditForm: true,
isPreview: false,
};
this.switchToEdit = this.switchToEdit.bind( this );
Expand All @@ -47,7 +48,7 @@ class LegacyWidgetEdit extends Component {
hasPermissionsToManageWidgets,
setAttributes,
} = this.props;
const { isPreview } = this.state;
const { isPreview, hasEditForm } = this.state;
const { identifier, isCallbackWidget } = attributes;
const widgetObject = identifier && availableLegacyWidgets[ identifier ];
if ( ! widgetObject ) {
Expand Down Expand Up @@ -115,8 +116,8 @@ class LegacyWidgetEdit extends Component {
icon="update"
>
</IconButton>
{ ! isCallbackWidget && (
<>
{ hasEditForm && (
<Fragment>
<Button
className={ `components-tab-button ${ ! isPreview ? 'is-active' : '' }` }
onClick={ this.switchToEdit }
Expand All @@ -129,26 +130,34 @@ class LegacyWidgetEdit extends Component {
>
<span>{ __( 'Preview' ) }</span>
</Button>
</>
</Fragment>
) }
</Toolbar>
</BlockControls>
{ inspectorControls }
{ ! isCallbackWidget && (
{ hasEditForm && (
<LegacyWidgetEditHandler
isVisible={ ! isPreview }
identifier={ attributes.identifier }
instance={ attributes.instance }
isCallbackWidget={ isCallbackWidget }
onInstanceChange={
( newInstance ) => {
this.props.setAttributes( {
instance: newInstance,
} );
( newInstance, newHasEditForm ) => {
if ( newInstance ) {
this.props.setAttributes( {
instance: newInstance,
} );
}
if ( newHasEditForm !== this.hasEditForm ) {
this.setState( {
hasEditForm: newHasEditForm,
} );
}
}
}
/>
) }
{ ( isPreview || isCallbackWidget ) && this.renderWidgetPreview() }
{ ( isPreview || ! hasEditForm ) && this.renderWidgetPreview() }
</>
);
}
Expand All @@ -159,6 +168,9 @@ class LegacyWidgetEdit extends Component {
instance: {},
identifier: undefined,
} );
this.setState( {
hasEditForm: true,
} );
}

switchToEdit() {
Expand Down