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

Navigation: Allow Search block to be added alongside links #22656

Merged
merged 5 commits into from
Jul 3, 2020
Merged

Navigation: Allow Search block to be added alongside links #22656

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c8d3bcc
Navigation: Allow Search block to be added alongside links
noisysocks May 27, 2020
50d49b7
Rename linkBlock to just block since more types are supported
adamziel Jul 2, 2020
c215042
Update comment
adamziel Jul 2, 2020
5d402fb
Navigation: Compensate for update_post_meta() calling stripslashes()
noisysocks Jul 3, 2020
8a5bfb5
Navigation: Link to Core ticket which tracks downstream changes
noisysocks Jul 3, 2020
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
63 changes: 62 additions & 1 deletion lib/class-wp-rest-menu-items-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ protected function prepare_item_for_database( $request ) {
'menu-item-title' => $menu_item_obj->title,
'menu-item-url' => $menu_item_obj->url,
'menu-item-description' => $menu_item_obj->description,
'menu-item-content' => $menu_item_obj->menu_item_content,
'menu-item-attr-title' => $menu_item_obj->attr_title,
'menu-item-target' => $menu_item_obj->target,
// Stored in the database as a string.
Expand All @@ -337,6 +338,7 @@ protected function prepare_item_for_database( $request ) {
'menu-item-title' => '',
'menu-item-url' => '',
'menu-item-description' => '',
'menu-item-content' => '',
'menu-item-attr-title' => '',
'menu-item-target' => '',
'menu-item-classes' => '',
Expand Down Expand Up @@ -390,6 +392,15 @@ protected function prepare_item_for_database( $request ) {
}
}

// Nav menu content.
if ( ! empty( $schema['properties']['content'] ) && isset( $request['content'] ) ) {
if ( is_string( $request['content'] ) ) {
$prepared_nav_item['menu-item-content'] = $request['content'];
} elseif ( isset( $request['content']['raw'] ) ) {
$prepared_nav_item['menu-item-content'] = $request['content']['raw'];
}
}

// Check if object id exists before saving.
if ( ! $prepared_nav_item['menu-item-object'] ) {
// If taxonony, check if term exists.
Expand Down Expand Up @@ -429,6 +440,13 @@ protected function prepare_item_for_database( $request ) {
}
}

// If menu item is type html, then content is required.
if ( 'html' === $prepared_nav_item['menu-item-type'] ) {
if ( empty( $prepared_nav_item['menu-item-content'] ) ) {
return new WP_Error( 'rest_content_required', __( 'Content required if menu item of type html.', 'gutenberg' ), array( 'status' => 400 ) );
}
}

// If menu id is set, valid the value of menu item position and parent id.
if ( ! empty( $prepared_nav_item['menu-id'] ) ) {
// Check if nav menu is valid.
Expand Down Expand Up @@ -596,6 +614,20 @@ public function prepare_item_for_response( $post, $request ) {
$data['object_id'] = absint( $menu_item->object_id );
}

if ( rest_is_field_included( 'content', $fields ) ) {
$data['content'] = array();
}
if ( rest_is_field_included( 'content.raw', $fields ) ) {
$data['content']['raw'] = $menu_item->content;
}
if ( rest_is_field_included( 'content.rendered', $fields ) ) {
/** This filter is documented in wp-includes/post-template.php */
$data['content']['rendered'] = apply_filters( 'the_content', $menu_item->content );
}
if ( rest_is_field_included( 'content.block_version', $fields ) ) {
$data['content']['block_version'] = block_version( $menu_item->content );
}

if ( in_array( 'parent', $fields, true ) ) {
// Same as post_parent, expose as integer.
$data['parent'] = absint( $menu_item->menu_item_parent );
Expand Down Expand Up @@ -787,7 +819,7 @@ public function get_item_schema() {
$schema['properties']['type'] = array(
'description' => __( 'The family of objects originally represented, such as "post_type" or "taxonomy".', 'gutenberg' ),
'type' => 'string',
'enum' => array( 'taxonomy', 'post_type', 'post_type_archive', 'custom' ),
'enum' => array( 'taxonomy', 'post_type', 'post_type_archive', 'custom', 'html' ),
'context' => array( 'view', 'edit', 'embed' ),
'default' => 'custom',
);
Expand Down Expand Up @@ -861,6 +893,35 @@ public function get_item_schema() {
'default' => 0,
);

$schema['properties']['content'] = array(
'description' => __( 'HTML content to display for this menu item. May contain blocks.', 'gutenberg' ),
'context' => array( 'view', 'edit', 'embed' ),
'type' => 'object',
'arg_options' => array(
'sanitize_callback' => null, // Note: sanitization implemented in self::prepare_item_for_database().
'validate_callback' => null, // Note: validation implemented in self::prepare_item_for_database().
),
'properties' => array(
'raw' => array(
'description' => __( 'HTML content, as it exists in the database.', 'gutenberg' ),
'type' => 'string',
'context' => array( 'edit' ),
),
'rendered' => array(
'description' => __( 'HTML content, transformed for display.', 'gutenberg' ),
'type' => 'string',
'context' => array( 'view', 'edit' ),
'readonly' => true,
),
'block_version' => array(
'description' => __( 'Version of the block format used in the HTML content.', 'gutenberg' ),
'type' => 'integer',
'context' => array( 'edit' ),
'readonly' => true,
),
),
);

$schema['properties']['target'] = array(
'description' => __( 'The target attribute of the link element for this menu item.', 'gutenberg' ),
'type' => 'string',
Expand Down
108 changes: 108 additions & 0 deletions lib/compat.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,3 +472,111 @@ function gutenberg_render_block_with_assigned_block_context( $pre_render, $parse
* @see WP_Block::render
*/
remove_action( 'enqueue_block_assets', 'wp_enqueue_registered_block_scripts_and_styles' );

/**
* Shim that hooks into `wp_update_nav_menu_item` and makes it so that nav menu
* items support a 'content' field. This field contains HTML and is used by nav
* menu items with `type` set to `'html'`.
*
* Specifically, this shim makes it so that:
*
* 1) The `wp_update_nav_menu_item()` function supports setting
* `'menu-item-content'` on a menu item. When merged to Core, this functionality
* should exist in `wp_update_nav_menu_item()`.
*
* 2) The `customize_save` ajax action supports setting `'content'` on a nav
* menu item. When merged to Core, this functionality should exist in
* `WP_Customize_Manager::save()`.
*
* This shim can be removed when the Gutenberg plugin requires a WordPress
* version that has the ticket below.
*
* @see <TICKET>
*
* @param int $menu_id ID of the updated menu.
* @param int $menu_item_db_id ID of the new menu item.
* @param array $args An array of arguments used to update/add the menu item.
*/
function gutenberg_update_nav_menu_item_content( $menu_id, $menu_item_db_id, $args ) {
global $wp_customize;

// Support setting content in customize_save admin-ajax.php requests by
// grabbing the unsanitized $_POST values.
if ( isset( $wp_customize ) ) {
$values = $wp_customize->unsanitized_post_values();
if ( isset( $values[ "nav_menu_item[$menu_item_db_id]" ]['content'] ) ) {
if ( is_string( $values[ "nav_menu_item[$menu_item_db_id]" ]['content'] ) ) {
$args['menu-item-content'] = $values[ "nav_menu_item[$menu_item_db_id]" ]['content'];
} elseif ( isset( $values[ "nav_menu_item[$menu_item_db_id]" ]['content']['raw'] ) ) {
$args['menu-item-content'] = $values[ "nav_menu_item[$menu_item_db_id]" ]['content']['raw'];
}
}
}

$defaults = array(
'menu-item-content' => '',
);

$args = wp_parse_args( $args, $defaults );

update_post_meta( $menu_item_db_id, '_menu_item_content', $args['menu-item-content'] );
noisysocks marked this conversation as resolved.
Show resolved Hide resolved
}
add_action( 'wp_update_nav_menu_item', 'gutenberg_update_nav_menu_item_content', 10, 3 );

/**
* Shim that hooks into `wp_setup_nav_menu_items` and makes it so that nav menu
* items have a 'content' field. This field contains HTML and is used by nav
* menu items with `type` set to `'html'`.
*
* Specifically, this shim makes it so that the `wp_setup_nav_menu_item()`
* function sets `content` on the returned menu item. When merged to Core, this
* functionality should exist in `wp_setup_nav_menu_item()`.
*
* This shim can be removed when the Gutenberg plugin requires a WordPress
* version that has the ticket below.
*
* @see <TICKET>
*
* @param object $menu_item The menu item object.
*/
function gutenberg_setup_html_nav_menu_item( $menu_item ) {
if ( 'html' === $menu_item->type ) {
$menu_item->type_label = __( 'HTML', 'gutenberg' );
$menu_item->content = ! isset( $menu_item->content ) ? get_post_meta( $menu_item->db_id, '_menu_item_content', true ) : $menu_item->content;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocker but just flagging this means plenty of DB roundtrips if the object cache happens to be disabled.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's pretty par the course for wp_setup_nav_menu_item() which is where this code will eventually live.

https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/nav-menu.php#L806

}

return $menu_item;
}
add_filter( 'wp_setup_nav_menu_item', 'gutenberg_setup_html_nav_menu_item' );

/**
* Shim that hooks into `walker_nav_menu_start_el` and makes it so that the
* default walker which renders a menu will correctly render the HTML associated
* with any navigation menu item that has `type` set to `'html`'.
*
* Specifically, this shim makes it so that `Walker_Nav_Menu::start_el()`
* renders the `content` of a nav menu item when its `type` is `'html'`. When
* merged to Core, this functionality should exist in
* `Walker_Nav_Menu::start_el()`.
*
* This shim can be removed when the Gutenberg plugin requires a WordPress
* version that has the ticket below.
*
* @see <TICKET>
*
* @param string $item_output The menu item's starting HTML output.
* @param WP_Post $item Menu item data object.
* @param int $depth Depth of menu item. Used for padding.
* @param stdClass $args An object of wp_nav_menu() arguments.
*/
function gutenberg_output_html_nav_menu_item( $item_output, $item, $depth, $args ) {
if ( 'html' === $item->type ) {
$item_output = $args->before;
/** This filter is documented in wp-includes/post-template.php */
$item_output .= apply_filters( 'the_content', $item->content );
$item_output .= $args->after;
}

return $item_output;
}
add_filter( 'walker_nav_menu_start_el', 'gutenberg_output_html_nav_menu_item', 10, 4 );
5 changes: 4 additions & 1 deletion packages/block-library/src/navigation/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,10 @@ function Navigation( {
>
<InnerBlocks
ref={ ref }
allowedBlocks={ [ 'core/navigation-link' ] }
allowedBlocks={ [
'core/navigation-link',
'core/search',
] }
renderAppender={
( isImmediateParentOfSelectedBlock &&
! selectedBlockHasDescendants ) ||
Expand Down
8 changes: 7 additions & 1 deletion packages/e2e-tests/specs/experiments/navigation.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,11 +421,17 @@ describe( 'Navigation', () => {

await showBlockToolbar();

// Add another Link block.
// Add another block.
// Using 'click' here checks for regressions of https://github.com/WordPress/gutenberg/issues/18329,
// an issue where the block appender requires two clicks.
await page.click( '.wp-block-navigation .block-list-appender' );

// Select a Link block.
const [ linkButton ] = await page.$x(
"//*[contains(@class, 'block-editor-inserter__quick-inserter')]//*[text()='Link']"
);
await linkButton.click();

// After adding a new block, search input should be shown immediately.
// Verify that Escape would close the popover.
// Regression: https://github.com/WordPress/gutenberg/pull/19885
Expand Down
26 changes: 21 additions & 5 deletions packages/edit-navigation/src/store/actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { v4 as uuid } from 'uuid';
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { serialize } from '@wordpress/blocks';

/**
* Internal dependencies
Expand Down Expand Up @@ -167,7 +168,7 @@ function* batchSave( menuId, menuItemsByClientId, navigationBlock ) {
function computeCustomizedAttribute( blocks, menuId, menuItemsByClientId ) {
const blocksList = blocksTreeToFlatList( blocks );
const dataList = blocksList.map( ( { block, parentId, position } ) =>
linkBlockToRequestItem( block, parentId, position )
blockToRequestItem( block, parentId, position )
);

// Create an object like { "nav_menu_item[12]": {...}} }
Expand Down Expand Up @@ -195,14 +196,29 @@ function computeCustomizedAttribute( blocks, menuId, menuItemsByClientId ) {
);
}

function linkBlockToRequestItem( block, parentId, position ) {
function blockToRequestItem( block, parentId, position ) {
const menuItem = omit( getMenuItemForBlock( block ), 'menus', 'meta' );

let attributes;

if ( block.name === 'core/navigation-link' ) {
attributes = {
type: 'custom',
title: block.attributes?.label,
original_title: '',
url: block.attributes.url,
};
} else {
attributes = {
type: 'html',
content: serialize( block ),
};
}
Comment on lines +204 to +216
Copy link
Contributor

Choose a reason for hiding this comment

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

It's totally too early for that, but a future refactor into a factory-like structure for transform/reverse transform could be a good idea to avoid a huge if/else mess.

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'm so excited because I love mess.


return {
...menuItem,
...attributes,
position,
title: block.attributes?.label,
url: block.attributes.url,
original_title: '',
classes: ( menuItem.classes || [] ).join( ' ' ),
xfn: ( menuItem.xfn || [] ).join( ' ' ),
nav_menu_term_id: menuId,
Expand Down
25 changes: 17 additions & 8 deletions packages/edit-navigation/src/store/resolvers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { groupBy, sortBy } from 'lodash';
/**
* WordPress dependencies
*/
import { createBlock } from '@wordpress/blocks';
import { parse, createBlock } from '@wordpress/blocks';

/**
* Internal dependencies
Expand Down Expand Up @@ -99,12 +99,9 @@ function createNavigationBlock( menuItems ) {
itemsByParentID[ item.id ]
);
}
const linkBlock = convertMenuItemToLinkBlock(
item,
menuItemInnerBlocks
);
menuItemIdToClientId[ item.id ] = linkBlock.clientId;
innerBlocks.push( linkBlock );
const block = convertMenuItemToBlock( item, menuItemInnerBlocks );
menuItemIdToClientId[ item.id ] = block.clientId;
innerBlocks.push( block );
}
return innerBlocks;
};
Expand All @@ -115,7 +112,19 @@ function createNavigationBlock( menuItems ) {
return [ navigationBlock, menuItemIdToClientId ];
}

function convertMenuItemToLinkBlock( menuItem, innerBlocks = [] ) {
function convertMenuItemToBlock( menuItem, innerBlocks = [] ) {
if ( menuItem.type === 'html' ) {
const parsedBlocks = parse( menuItem.content.raw );

if ( parsedBlocks.length !== 1 ) {
Copy link
Contributor

@adamziel adamziel Jul 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This will kick in once a block plugin gets disabled. I don't think there's a good way of addressing it, but just wanted to double check - is this behavior consistent with the post editor?

Copy link
Member Author

Choose a reason for hiding this comment

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

If a block plugin is disabled then parse() will return an array containing one core/missing block. The body of this if() won't be executed and so a core/missing is what will be used for the menu item. This matches the behaviour of the post editor.

The if() here is to guard against the case where, for whatever reason, there is 0 or >1 blocks in the menu item's HTML. I'm not really sure what to do in this case—it's undefined behaviour and probably indicates a bug somewhere. Perhaps failing loudly would be better?

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 put it on my todo list to come back to this as will need to think on it further.

return createBlock( 'core/freeform', {
originalContent: menuItem.content.raw,
} );
}

return parsedBlocks[ 0 ];
}

const attributes = {
label: menuItem.title.rendered,
url: menuItem.url,
Expand Down
Loading