From 9905c0d851f518f16043ac382f8ea9abd0735e30 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 20 Apr 2021 15:53:34 +0100 Subject: [PATCH 01/48] Create correct nav link block variations from menu data type --- .../src/navigation-link/fallback-variations.js | 10 ++++++++-- packages/block-library/src/navigation-link/hooks.js | 6 +++++- packages/edit-navigation/src/store/resolvers.js | 2 ++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/block-library/src/navigation-link/fallback-variations.js b/packages/block-library/src/navigation-link/fallback-variations.js index 5f8d44702a1157..ab56d6c7e7deb6 100644 --- a/packages/block-library/src/navigation-link/fallback-variations.js +++ b/packages/block-library/src/navigation-link/fallback-variations.js @@ -58,8 +58,14 @@ const fallbackVariations = [ */ fallbackVariations.forEach( ( variation ) => { if ( variation.isActive ) return; - variation.isActive = ( blockAttributes, variationAttributes ) => - blockAttributes.type === variationAttributes.type; + + variation.isActive = ( blockAttributes, variationAttributes ) => { + const normalizedVariationKind = variationAttributes.kind.replace( + '-', + '_' + ); + return blockAttributes.type === normalizedVariationKind; + }; } ); export default fallbackVariations; diff --git a/packages/block-library/src/navigation-link/hooks.js b/packages/block-library/src/navigation-link/hooks.js index 7fb89cafe414dd..47c6b530534a31 100644 --- a/packages/block-library/src/navigation-link/hooks.js +++ b/packages/block-library/src/navigation-link/hooks.js @@ -46,7 +46,11 @@ export function enhanceNavigationLinkVariations( settings, name ) { // Otherwise decorate server passed variations with an icon and isActive function if ( settings.variations ) { const isActive = ( blockAttributes, variationAttributes ) => { - return blockAttributes.type === variationAttributes.type; + const normalizedVariationKind = variationAttributes.kind.replace( + '-', + '_' + ); + return blockAttributes.type === normalizedVariationKind; }; const variations = settings.variations.map( ( variation ) => { return { diff --git a/packages/edit-navigation/src/store/resolvers.js b/packages/edit-navigation/src/store/resolvers.js index 798f7db0581958..4b67cd64a27e00 100644 --- a/packages/edit-navigation/src/store/resolvers.js +++ b/packages/edit-navigation/src/store/resolvers.js @@ -106,6 +106,7 @@ function createNavigationBlock( menuItems ) { } const sortedItems = sortBy( items, 'menu_order' ); + for ( const item of sortedItems ) { let menuItemInnerBlocks = []; if ( itemsByParentID[ item.id ]?.length ) { @@ -155,6 +156,7 @@ function convertMenuItemToBlock( menuItem, innerBlocks = [] ) { ...( menuItem.target === NEW_TAB_TARGET_ATTRIBUTE && { opensInNewTab: true, } ), + type: menuItem.type, }; return createBlock( 'core/navigation-link', attributes, innerBlocks ); From 00d7040b04e0140e7190afca1aedd0f8e23a48af Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 20 Apr 2021 16:03:28 +0100 Subject: [PATCH 02/48] Set menu object type based on block type --- packages/edit-navigation/src/store/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index 60a0418d163fd1..e7b4a72b55d986 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -138,7 +138,7 @@ export function computeCustomizedAttribute( if ( block.name === 'core/navigation-link' ) { attributes = { - type: 'custom', + type: block.attributes?.type || 'custom', title: block.attributes?.label, original_title: '', url: block.attributes.url, From 6005551212a9c85301baa82ed64a85a307785057 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 20 Apr 2021 16:24:36 +0100 Subject: [PATCH 03/48] Correctly hydrate nav block itself from stored menu object fields. --- packages/block-library/src/navigation/placeholder.js | 3 +++ packages/edit-navigation/src/store/resolvers.js | 1 + packages/edit-navigation/src/store/utils.js | 3 +++ 3 files changed, 7 insertions(+) diff --git a/packages/block-library/src/navigation/placeholder.js b/packages/block-library/src/navigation/placeholder.js index c0ac1b6ff4c301..e1591173d88fdf 100644 --- a/packages/block-library/src/navigation/placeholder.js +++ b/packages/block-library/src/navigation/placeholder.js @@ -56,6 +56,9 @@ function mapMenuItemsToBlocks( menuItems ) { ? __( '(no title)' ) : menuItem.title.rendered, opensInNewTab: menuItem.target === '_blank', + type: menuItem?.type, + id: menuItem?.object_id, + title: menuItem?.attr_title, }; if ( menuItem.url ) { diff --git a/packages/edit-navigation/src/store/resolvers.js b/packages/edit-navigation/src/store/resolvers.js index 4b67cd64a27e00..fc69fddff2d4d2 100644 --- a/packages/edit-navigation/src/store/resolvers.js +++ b/packages/edit-navigation/src/store/resolvers.js @@ -156,6 +156,7 @@ function convertMenuItemToBlock( menuItem, innerBlocks = [] ) { ...( menuItem.target === NEW_TAB_TARGET_ATTRIBUTE && { opensInNewTab: true, } ), + id: menuItem?.object_id, type: menuItem.type, }; diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index e7b4a72b55d986..0d16afe6327371 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -149,6 +149,9 @@ export function computeCustomizedAttribute( target: block.attributes.opensInNewTab ? NEW_TAB_TARGET_ATTRIBUTE : '', + object_id: block.attributes?.id, + + }; } else { attributes = { From 4a00a65e4bbd9f35ed36de6a6445e4746b3dde41 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 21 Apr 2021 12:13:20 +0100 Subject: [PATCH 04/48] =?UTF-8?q?Fix=20to=20store/retrieve=20nav=20link=20?= =?UTF-8?q?type=20from/to=20the=20menu=20item=E2=80=99s=20=E2=80=9Cobject?= =?UTF-8?q?=E2=80=9D=20field.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This field is the correct field in which to store the type of the object being stored (eg: “post”, “pages”…etc). See https://core.trac.wordpress.org/browser/tags/5.7.1/src/wp-includes/nav-menu.php#L796 --- .../src/navigation-link/fallback-variations.js | 6 +----- packages/block-library/src/navigation-link/hooks.js | 6 +----- packages/edit-navigation/src/store/resolvers.js | 4 ++-- packages/edit-navigation/src/store/utils.js | 2 +- 4 files changed, 5 insertions(+), 13 deletions(-) diff --git a/packages/block-library/src/navigation-link/fallback-variations.js b/packages/block-library/src/navigation-link/fallback-variations.js index ab56d6c7e7deb6..0639e325e5bb9d 100644 --- a/packages/block-library/src/navigation-link/fallback-variations.js +++ b/packages/block-library/src/navigation-link/fallback-variations.js @@ -60,11 +60,7 @@ fallbackVariations.forEach( ( variation ) => { if ( variation.isActive ) return; variation.isActive = ( blockAttributes, variationAttributes ) => { - const normalizedVariationKind = variationAttributes.kind.replace( - '-', - '_' - ); - return blockAttributes.type === normalizedVariationKind; + return blockAttributes.type === variationAttributes.type; }; } ); diff --git a/packages/block-library/src/navigation-link/hooks.js b/packages/block-library/src/navigation-link/hooks.js index 47c6b530534a31..7fb89cafe414dd 100644 --- a/packages/block-library/src/navigation-link/hooks.js +++ b/packages/block-library/src/navigation-link/hooks.js @@ -46,11 +46,7 @@ export function enhanceNavigationLinkVariations( settings, name ) { // Otherwise decorate server passed variations with an icon and isActive function if ( settings.variations ) { const isActive = ( blockAttributes, variationAttributes ) => { - const normalizedVariationKind = variationAttributes.kind.replace( - '-', - '_' - ); - return blockAttributes.type === normalizedVariationKind; + return blockAttributes.type === variationAttributes.type; }; const variations = settings.variations.map( ( variation ) => { return { diff --git a/packages/edit-navigation/src/store/resolvers.js b/packages/edit-navigation/src/store/resolvers.js index fc69fddff2d4d2..95df21a6cf8372 100644 --- a/packages/edit-navigation/src/store/resolvers.js +++ b/packages/edit-navigation/src/store/resolvers.js @@ -156,8 +156,8 @@ function convertMenuItemToBlock( menuItem, innerBlocks = [] ) { ...( menuItem.target === NEW_TAB_TARGET_ATTRIBUTE && { opensInNewTab: true, } ), - id: menuItem?.object_id, - type: menuItem.type, + id: menuItem?.object_id, + type: menuItem.object, // https://core.trac.wordpress.org/browser/tags/5.7.1/src/wp-includes/nav-menu.php#L796 }; return createBlock( 'core/navigation-link', attributes, innerBlocks ); diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index 0d16afe6327371..d286e71e5612e8 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -138,7 +138,7 @@ export function computeCustomizedAttribute( if ( block.name === 'core/navigation-link' ) { attributes = { - type: block.attributes?.type || 'custom', + type: block.attributes?.object || 'custom', title: block.attributes?.label, original_title: '', url: block.attributes.url, From b5a5f5a9afdef8f2ece4094f5e5b967ce63ad2e9 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 21 Apr 2021 12:15:30 +0100 Subject: [PATCH 05/48] Fix up Nav Block to use object field for type --- packages/block-library/src/navigation/placeholder.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/navigation/placeholder.js b/packages/block-library/src/navigation/placeholder.js index e1591173d88fdf..02a6d3137d8a0f 100644 --- a/packages/block-library/src/navigation/placeholder.js +++ b/packages/block-library/src/navigation/placeholder.js @@ -56,7 +56,7 @@ function mapMenuItemsToBlocks( menuItems ) { ? __( '(no title)' ) : menuItem.title.rendered, opensInNewTab: menuItem.target === '_blank', - type: menuItem?.type, + type: menuItem?.object, id: menuItem?.object_id, title: menuItem?.attr_title, }; From 0304ca4d397d887681f9224c74b8c87db1f5f8ed Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 21 Apr 2021 13:39:29 +0100 Subject: [PATCH 06/48] =?UTF-8?q?Avoid=20storing=20menu=20objects=20with?= =?UTF-8?q?=20=E2=80=9Ccustom=E2=80=9D=20as=20type.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/edit-navigation/src/store/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index d286e71e5612e8..eaeb752d3ffeb8 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -138,7 +138,7 @@ export function computeCustomizedAttribute( if ( block.name === 'core/navigation-link' ) { attributes = { - type: block.attributes?.object || 'custom', + type: block.attributes?.object, title: block.attributes?.label, original_title: '', url: block.attributes.url, From 3fd02d4958d18ef4882746f618a3b6f8796fc8f9 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 21 Apr 2021 14:18:21 +0100 Subject: [PATCH 07/48] Only set id and type on the generated block if the object and object_id fields are provided in the menu object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This avoids “undefined” values. --- packages/edit-navigation/src/store/resolvers.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/edit-navigation/src/store/resolvers.js b/packages/edit-navigation/src/store/resolvers.js index 95df21a6cf8372..42b67e954038a3 100644 --- a/packages/edit-navigation/src/store/resolvers.js +++ b/packages/edit-navigation/src/store/resolvers.js @@ -156,8 +156,8 @@ function convertMenuItemToBlock( menuItem, innerBlocks = [] ) { ...( menuItem.target === NEW_TAB_TARGET_ATTRIBUTE && { opensInNewTab: true, } ), - id: menuItem?.object_id, - type: menuItem.object, // https://core.trac.wordpress.org/browser/tags/5.7.1/src/wp-includes/nav-menu.php#L796 + ...( menuItem?.object_id && { id: menuItem.object_id } ), + ...( menuItem?.object && { type: menuItem.object } ), // https://core.trac.wordpress.org/browser/tags/5.7.1/src/wp-includes/nav-menu.php#L796 }; return createBlock( 'core/navigation-link', attributes, innerBlocks ); From 481c8d50f10a210d12c48803914774d41f0bd8e2 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 21 Apr 2021 14:18:57 +0100 Subject: [PATCH 08/48] Add test to cover correct creation of core/navigation-link block variations from menu objects --- .../src/store/test/resolvers.js | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/packages/edit-navigation/src/store/test/resolvers.js b/packages/edit-navigation/src/store/test/resolvers.js index 26c85cfd48dbd4..6e962f967f5500 100644 --- a/packages/edit-navigation/src/store/test/resolvers.js +++ b/packages/edit-navigation/src/store/test/resolvers.js @@ -189,4 +189,105 @@ describe( 'getNavigationPostForMenu', () => { expect( generator.next().done ).toBe( true ); } ); + + it( 'creates correct core/navigation-link block variations from menu objects', () => { + const menuId = 123; + + const generator = getNavigationPostForMenu( menuId ); + + // Advance generator + generator.next(); + generator.next(); + generator.next(); + + const menuItems = [ + { + id: 100, + title: { + raw: 'wp.com', + rendered: 'wp.com', + }, + url: 'http://wp.com', + menu_order: 1, + menus: [ 1 ], + parent: 0, + classes: [ 'menu', 'classes' ], + xfn: [ 'nofollow' ], + description: 'description', + attr_title: 'link title', + object: 'post', + object_id: 123, // the post object ID not the menu object ID + }, + { + id: 101, + title: { + raw: 'wp.org', + rendered: 'wp.org', + }, + url: 'http://wp.org', + menu_order: 2, + menus: [ 1 ], + parent: 0, + classes: [], + xfn: [], + description: '', + attr_title: '', + object: 'page', + object_id: 456, // the page object ID not the menu object ID + }, + { + id: 102, + title: { + raw: 'wordpress.org', + rendered: 'wordpress.org', + }, + url: 'https://wordpress.org', + menu_order: 3, + menus: [ 1 ], + parent: 0, + classes: [], + xfn: [], + description: '', + attr_title: '', + object: 'custom', + }, + ]; + + // Feed a known value to the generator yield result + generator.next( menuItems ); + + const persistPostAction = generator.next().value; + + // Get the core/navigation-link blocks from the generated core/navigation block innerBlocks. + const blockAttrs = persistPostAction.args[ 2 ].blocks[ 0 ].innerBlocks.map( + ( block ) => block.attributes + ); + + // Post link + expect( blockAttrs[ 0 ] ).toEqual( + expect.objectContaining( { + id: 123, + type: 'post', + } ) + ); + + // Page link + expect( blockAttrs[ 1 ] ).toEqual( + expect.objectContaining( { + id: 456, + type: 'page', + } ) + ); + + // Custom link + expect( blockAttrs[ 2 ] ).toEqual( + expect.objectContaining( { + type: 'custom', + } ) + ); + + // We should not manually create an ID unless the menu object + // has a `object_id` field set. + expect( blockAttrs[ 2 ].id ).toBeUndefined(); + } ); } ); From 02ab08fb9cae695c1447b0852bff871fdb2dae6a Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 21 Apr 2021 14:35:15 +0100 Subject: [PATCH 09/48] Conditional persist block id and type attrs to menu object fields --- packages/edit-navigation/src/store/utils.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index eaeb752d3ffeb8..72825557736f54 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -138,7 +138,6 @@ export function computeCustomizedAttribute( if ( block.name === 'core/navigation-link' ) { attributes = { - type: block.attributes?.object, title: block.attributes?.label, original_title: '', url: block.attributes.url, @@ -149,9 +148,12 @@ export function computeCustomizedAttribute( target: block.attributes.opensInNewTab ? NEW_TAB_TARGET_ATTRIBUTE : '', - object_id: block.attributes?.id, - - + ...( block.attributes?.id && { + object_id: block.attributes.id, + } ), + ...( block.attributes?.type && { + object: block.attributes.type, + } ), }; } else { attributes = { From 29ef10ace7b1adda89629240e0b76a3d48ea32b3 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 21 Apr 2021 14:35:37 +0100 Subject: [PATCH 10/48] Add tests to cover conversion from blocks to menu objects --- .../edit-navigation/src/store/test/utils.js | 48 ++++++++++++++++++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/packages/edit-navigation/src/store/test/utils.js b/packages/edit-navigation/src/store/test/utils.js index 44d03900b85771..89d12275a9d740 100644 --- a/packages/edit-navigation/src/store/test/utils.js +++ b/packages/edit-navigation/src/store/test/utils.js @@ -237,6 +237,21 @@ describe( 'computeCustomizedAttribute', () => { isValid: true, name: 'core/navigation-link', }, + { + attributes: { + id: 678, + label: 'Page Example', + opensInNewTab: false, + url: 'https://localhost:8889/page-example/', + className: '', + rel: '', + type: 'page', + }, + clientId: 'navigation-link-block-client-id-3', + innerBlocks: [], + isValid: true, + name: 'core/navigation-link', + }, ]; const menuId = 123; @@ -251,6 +266,7 @@ describe( 'computeCustomizedAttribute', () => { url: 'http://wp.com', menu_order: 1, menus: [ 1 ], + object: 'custom', }, 'navigation-link-block-client-id-2': { id: 101, @@ -261,6 +277,18 @@ describe( 'computeCustomizedAttribute', () => { url: 'http://wp.org', menu_order: 2, menus: [ 1 ], + object: 'custom', + }, + 'navigation-link-block-client-id-3': { + id: 102, + title: { + raw: 'Page Example', + rendered: 'Page Example', + }, + url: 'https://wordpress.org', + menu_order: 3, + menus: [ 1 ], + object: 'page', }, }; @@ -284,7 +312,7 @@ describe( 'computeCustomizedAttribute', () => { position: 1, status: 'publish', title: 'wp.org', - type: 'custom', + object: 'custom', url: 'http://wp.org', xfn: [ 'external' ], target: '', @@ -300,11 +328,27 @@ describe( 'computeCustomizedAttribute', () => { position: 2, status: 'publish', title: 'wp.com', - type: 'custom', + object: 'custom', url: 'http://wp.com', xfn: [ '' ], target: '_blank', }, + 'nav_menu_item[102]': { + _invalid: false, + classes: [ '' ], + id: 102, + menu_item_parent: 0, + menu_order: 3, + nav_menu_term_id: 123, + original_title: '', + position: 3, + status: 'publish', + title: 'Page Example', + object: 'page', + object_id: 678, + url: 'https://localhost:8889/page-example/', + xfn: [ '' ], + }, } ); } ); } ); From aa2b9f2b1d3d7a5e9e1fd6ceb004bc932b8a1fef Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 21 Apr 2021 14:36:42 +0100 Subject: [PATCH 11/48] Revert changes to fallback variations isActive detection logic --- .../block-library/src/navigation-link/fallback-variations.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/block-library/src/navigation-link/fallback-variations.js b/packages/block-library/src/navigation-link/fallback-variations.js index 0639e325e5bb9d..134f48b2e858d6 100644 --- a/packages/block-library/src/navigation-link/fallback-variations.js +++ b/packages/block-library/src/navigation-link/fallback-variations.js @@ -59,9 +59,8 @@ const fallbackVariations = [ fallbackVariations.forEach( ( variation ) => { if ( variation.isActive ) return; - variation.isActive = ( blockAttributes, variationAttributes ) => { - return blockAttributes.type === variationAttributes.type; - }; + variation.isActive = ( blockAttributes, variationAttributes ) => + blockAttributes.type === variationAttributes.type; } ); export default fallbackVariations; From c155f0abe7ccb8eb75d83f12f664643ad1f5effc Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 21 Apr 2021 15:50:13 +0100 Subject: [PATCH 12/48] =?UTF-8?q?Remove=20accidental=20new=20line=20in=20f?= =?UTF-8?q?ile=20causes=20=E2=80=9Cchanges=E2=80=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../block-library/src/navigation-link/fallback-variations.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/block-library/src/navigation-link/fallback-variations.js b/packages/block-library/src/navigation-link/fallback-variations.js index 134f48b2e858d6..5f8d44702a1157 100644 --- a/packages/block-library/src/navigation-link/fallback-variations.js +++ b/packages/block-library/src/navigation-link/fallback-variations.js @@ -58,7 +58,6 @@ const fallbackVariations = [ */ fallbackVariations.forEach( ( variation ) => { if ( variation.isActive ) return; - variation.isActive = ( blockAttributes, variationAttributes ) => blockAttributes.type === variationAttributes.type; } ); From 74a8f128c4fd22d89117f64404312915849a69c7 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 23 Apr 2021 12:03:35 +0100 Subject: [PATCH 13/48] Create mapping utils to map menu items to blocks and vice versa --- .../edit-navigation/src/store/resolvers.js | 44 +++++++++++++- packages/edit-navigation/src/store/utils.js | 59 ++++++++++++++++--- 2 files changed, 93 insertions(+), 10 deletions(-) diff --git a/packages/edit-navigation/src/store/resolvers.js b/packages/edit-navigation/src/store/resolvers.js index 42b67e954038a3..e27d92e74ec801 100644 --- a/packages/edit-navigation/src/store/resolvers.js +++ b/packages/edit-navigation/src/store/resolvers.js @@ -153,12 +153,50 @@ function convertMenuItemToBlock( menuItem, innerBlocks = [] ) { className: menuItem.classes.join( ' ' ), description: menuItem.description, rel: menuItem.xfn.join( ' ' ), - ...( menuItem.target === NEW_TAB_TARGET_ATTRIBUTE && { + ...( menuItem?.object_id && + mapMenuItemFieldToBlockAttribute( + 'object_id', + menuItem.object_id + ) ), + ...( menuItem?.object && + mapMenuItemFieldToBlockAttribute( 'object', menuItem.object ) ), + ...( menuItem?.type && + mapMenuItemFieldToBlockAttribute( 'type', menuItem.type ) ), + ...( menuItem.target === NEW_TAB_TARGET_ATTRIBUTE && { opensInNewTab: true, } ), - ...( menuItem?.object_id && { id: menuItem.object_id } ), - ...( menuItem?.object && { type: menuItem.object } ), // https://core.trac.wordpress.org/browser/tags/5.7.1/src/wp-includes/nav-menu.php#L796 }; return createBlock( 'core/navigation-link', attributes, innerBlocks ); } + +function mapMenuItemFieldToBlockAttribute( menuItemField, menuItemVal ) { + const MAPPING = { + object_id: 'id', + object: 'type', + // `nav_menu_item` may be one of: + // 1. `post_type`, + // 2. `post_type_archive`, + // 3. `taxonomy`, + // 4. `custom`. + type: { + attr: 'kind', + mapper: () => { + switch ( menuItemVal ) { + case 'post_type': + case 'post_type_archive': + return menuItemVal.replace( '_', '-' ); + default: + return menuItemVal; + } + }, + }, + }; + + const key = MAPPING[ menuItemField ].attr ?? MAPPING[ menuItemField ]; + const val = MAPPING[ menuItemField ].mapper?.() ?? menuItemVal; + + return { + [ key ]: val, + }; +} diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index 72825557736f54..3c3c8d4035b539 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -145,15 +145,24 @@ export function computeCustomizedAttribute( xfn: block.attributes.rel?.split( ' ' ), classes: block.attributes.className?.split( ' ' ), attr_title: block.attributes.title, - target: block.attributes.opensInNewTab + ...( block.attributes?.type && + mapBlockAttributeToMenuItemField( + 'type', + block.attributes?.type + ) ), + ...( block.attributes?.id && + mapBlockAttributeToMenuItemField( + 'id', + block.attributes?.id + ) ), + ...( block.attributes?.kind && + mapBlockAttributeToMenuItemField( + 'kind', + block.attributes?.kind + ) ), + target: block.attributes.opensInNewTab ? NEW_TAB_TARGET_ATTRIBUTE : '', - ...( block.attributes?.id && { - object_id: block.attributes.id, - } ), - ...( block.attributes?.type && { - object: block.attributes.type, - } ), }; } else { attributes = { @@ -176,4 +185,40 @@ export function computeCustomizedAttribute( function getMenuItemForBlock( block ) { return omit( menuItemsByClientId[ block.clientId ] || {}, '_links' ); } + + function mapBlockAttributeToMenuItemField( + blockAttributeName, + blockAttributeValue + ) { + const MAPPING = { + id: 'object_id', + type: 'object', + // `nav_menu_item` may be one of: + // 1. `post_type`, + // 2. `post_type_archive`, + // 3. `taxonomy`, + // 4. `custom`. + kind: { + attr: 'type', + mapper: () => { + switch ( blockAttributeValue ) { + case 'post-type': + case 'post-type-archive': + return blockAttributeValue.replace( '-', '_' ); + default: + return blockAttributeValue; + } + }, + }, + }; + + const key = + MAPPING[ blockAttributeName ].attr ?? MAPPING[ blockAttributeName ]; + const val = + MAPPING[ blockAttributeName ].mapper?.() ?? blockAttributeValue; + + return { + [ key ]: val, + }; + } } From 4749b62f204fb8e50b2fb4ff69d70c8e51117c1f Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 23 Apr 2021 12:08:58 +0100 Subject: [PATCH 14/48] Genericise conversion util and share --- .../edit-navigation/src/store/resolvers.js | 9 ++------ packages/edit-navigation/src/store/utils.js | 22 ++++++++++++------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/packages/edit-navigation/src/store/resolvers.js b/packages/edit-navigation/src/store/resolvers.js index e27d92e74ec801..5e811c5fdcdf93 100644 --- a/packages/edit-navigation/src/store/resolvers.js +++ b/packages/edit-navigation/src/store/resolvers.js @@ -18,7 +18,7 @@ import { } from '../constants'; import { resolveMenuItems, dispatch } from './controls'; -import { buildNavigationPostId } from './utils'; +import { buildNavigationPostId, convertAttribute } from './utils'; /** * Creates a "stub" navigation post reflecting the contents of menu with id=menuId. The @@ -193,10 +193,5 @@ function mapMenuItemFieldToBlockAttribute( menuItemField, menuItemVal ) { }, }; - const key = MAPPING[ menuItemField ].attr ?? MAPPING[ menuItemField ]; - const val = MAPPING[ menuItemField ].mapper?.() ?? menuItemVal; - - return { - [ key ]: val, - }; + return convertAttribute( MAPPING, menuItemField, menuItemVal ); } diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index 3c3c8d4035b539..0eda2b0e01c282 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -212,13 +212,19 @@ export function computeCustomizedAttribute( }, }; - const key = - MAPPING[ blockAttributeName ].attr ?? MAPPING[ blockAttributeName ]; - const val = - MAPPING[ blockAttributeName ].mapper?.() ?? blockAttributeValue; - - return { - [ key ]: val, - }; + return convertAttribute( + MAPPING, + blockAttributeName, + blockAttributeValue + ); } } + +export const convertAttribute = function convertAttribute( mapping, key, val ) { + const left = mapping[ key ].attr ?? mapping[ key ]; + const right = mapping[ key ].mapper?.() ?? val; + + return { + [ left ]: right, + }; +}; From 741d811439a1566f7405b0a35d25c29b70fe2484 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 23 Apr 2021 12:45:26 +0100 Subject: [PATCH 15/48] Doc block for conversion util --- packages/edit-navigation/src/store/utils.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index 0eda2b0e01c282..6c0c0c6ea86ccd 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -219,7 +219,16 @@ export function computeCustomizedAttribute( ); } } - +/** + * Converts a given key in an object to it's mapped equivalent. + * + * Typically used to convert block attributes to their equivalent menu item fields. + * + * @param {Object} mapping a key/value mapping of attributes to their equivalents. + * @param {string} key the name of the key within the mapping to be converted. + * @param {string} val the value to be assigned to the converted field. + * @return {Object} the converted field object. + */ export const convertAttribute = function convertAttribute( mapping, key, val ) { const left = mapping[ key ].attr ?? mapping[ key ]; const right = mapping[ key ].mapper?.() ?? val; From 4d5ce11581ab3191ed013f62b472f9cc2ec8bcfe Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 23 Apr 2021 12:49:45 +0100 Subject: [PATCH 16/48] Add simple conversion of kind on the nav block itself --- packages/block-library/src/navigation/placeholder.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/navigation/placeholder.js b/packages/block-library/src/navigation/placeholder.js index 02a6d3137d8a0f..b85a00eee7dc29 100644 --- a/packages/block-library/src/navigation/placeholder.js +++ b/packages/block-library/src/navigation/placeholder.js @@ -58,7 +58,8 @@ function mapMenuItemsToBlocks( menuItems ) { opensInNewTab: menuItem.target === '_blank', type: menuItem?.object, id: menuItem?.object_id, - title: menuItem?.attr_title, + // the equivalent on `nav_menu_item` is "type" but it is stored with underscores whereas the block attr uses hyphens. + kind: menuItem.type.replace( '_', '_' ), }; if ( menuItem.url ) { From 6d24f15c41d9b02438226032ea7f90a0382d87db Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 23 Apr 2021 12:50:49 +0100 Subject: [PATCH 17/48] Correct type to kind conversion on block --- packages/block-library/src/navigation/placeholder.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/navigation/placeholder.js b/packages/block-library/src/navigation/placeholder.js index b85a00eee7dc29..2944393563072d 100644 --- a/packages/block-library/src/navigation/placeholder.js +++ b/packages/block-library/src/navigation/placeholder.js @@ -59,7 +59,7 @@ function mapMenuItemsToBlocks( menuItems ) { type: menuItem?.object, id: menuItem?.object_id, // the equivalent on `nav_menu_item` is "type" but it is stored with underscores whereas the block attr uses hyphens. - kind: menuItem.type.replace( '_', '_' ), + kind: menuItem.type.replace( '_', '-' ), }; if ( menuItem.url ) { From 3528abcef5237a16aac610b0b9620979f3506a0b Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 23 Apr 2021 13:19:52 +0100 Subject: [PATCH 18/48] Fix up --- packages/edit-navigation/src/store/resolvers.js | 2 +- packages/edit-navigation/src/store/utils.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/edit-navigation/src/store/resolvers.js b/packages/edit-navigation/src/store/resolvers.js index 5e811c5fdcdf93..50bb3a43ba2896 100644 --- a/packages/edit-navigation/src/store/resolvers.js +++ b/packages/edit-navigation/src/store/resolvers.js @@ -162,7 +162,7 @@ function convertMenuItemToBlock( menuItem, innerBlocks = [] ) { mapMenuItemFieldToBlockAttribute( 'object', menuItem.object ) ), ...( menuItem?.type && mapMenuItemFieldToBlockAttribute( 'type', menuItem.type ) ), - ...( menuItem.target === NEW_TAB_TARGET_ATTRIBUTE && { + ...( menuItem?.target === NEW_TAB_TARGET_ATTRIBUTE && { opensInNewTab: true, } ), }; diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index 6c0c0c6ea86ccd..29678c6eff1564 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -160,7 +160,7 @@ export function computeCustomizedAttribute( 'kind', block.attributes?.kind ) ), - target: block.attributes.opensInNewTab + target: block.attributes.opensInNewTab ? NEW_TAB_TARGET_ATTRIBUTE : '', }; From 5ce0c375936fb6b51f4e3735340498106c112804 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Mon, 26 Apr 2021 10:44:28 +0100 Subject: [PATCH 19/48] Simplify mapper function --- packages/edit-navigation/src/store/resolvers.js | 12 ++---------- packages/edit-navigation/src/store/utils.js | 13 +++---------- 2 files changed, 5 insertions(+), 20 deletions(-) diff --git a/packages/edit-navigation/src/store/resolvers.js b/packages/edit-navigation/src/store/resolvers.js index 50bb3a43ba2896..33c1dd1bb0e891 100644 --- a/packages/edit-navigation/src/store/resolvers.js +++ b/packages/edit-navigation/src/store/resolvers.js @@ -180,16 +180,8 @@ function mapMenuItemFieldToBlockAttribute( menuItemField, menuItemVal ) { // 3. `taxonomy`, // 4. `custom`. type: { - attr: 'kind', - mapper: () => { - switch ( menuItemVal ) { - case 'post_type': - case 'post_type_archive': - return menuItemVal.replace( '_', '-' ); - default: - return menuItemVal; - } - }, + attr: 'kind', // the equivalent on `nav_menu_item` is "type" but it is stored with underscores whereas the block attr uses hyphens. + mapper: () => menuItemVal.replace( '_', '-' ), }, }; diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index 29678c6eff1564..f052537efc9768 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -199,16 +199,8 @@ export function computeCustomizedAttribute( // 3. `taxonomy`, // 4. `custom`. kind: { - attr: 'type', - mapper: () => { - switch ( blockAttributeValue ) { - case 'post-type': - case 'post-type-archive': - return blockAttributeValue.replace( '-', '_' ); - default: - return blockAttributeValue; - } - }, + attr: 'type', // the equivalent on `nav_menu_item` is "type" but it is stored with underscores whereas the block attr uses hyphens. + mapper: () => blockAttributeValue.replace( '-', '_' ), }, }; @@ -219,6 +211,7 @@ export function computeCustomizedAttribute( ); } } + /** * Converts a given key in an object to it's mapped equivalent. * From 7dc340e51b6993878e390dc7e67de4d18e522b87 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Mon, 26 Apr 2021 10:51:27 +0100 Subject: [PATCH 20/48] Fix broken test --- packages/edit-navigation/src/store/test/utils.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/edit-navigation/src/store/test/utils.js b/packages/edit-navigation/src/store/test/utils.js index 89d12275a9d740..e277571e93aa20 100644 --- a/packages/edit-navigation/src/store/test/utils.js +++ b/packages/edit-navigation/src/store/test/utils.js @@ -348,6 +348,7 @@ describe( 'computeCustomizedAttribute', () => { object_id: 678, url: 'https://localhost:8889/page-example/', xfn: [ '' ], + target: '', }, } ); } ); From c5102e37548ea1ba85e9a8f8d0f8ec5ff4431d73 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Mon, 26 Apr 2021 11:41:23 +0100 Subject: [PATCH 21/48] Extract mapping functions to shared utils and test --- .../edit-navigation/src/store/resolvers.js | 23 +--- .../edit-navigation/src/store/test/utils.js | 120 +++++++++++++++++- packages/edit-navigation/src/store/utils.js | 81 ++++++++---- 3 files changed, 178 insertions(+), 46 deletions(-) diff --git a/packages/edit-navigation/src/store/resolvers.js b/packages/edit-navigation/src/store/resolvers.js index 33c1dd1bb0e891..59ffdd4ed05ff6 100644 --- a/packages/edit-navigation/src/store/resolvers.js +++ b/packages/edit-navigation/src/store/resolvers.js @@ -18,7 +18,10 @@ import { } from '../constants'; import { resolveMenuItems, dispatch } from './controls'; -import { buildNavigationPostId, convertAttribute } from './utils'; +import { + buildNavigationPostId, + mapMenuItemFieldToBlockAttribute, +} from './utils'; /** * Creates a "stub" navigation post reflecting the contents of menu with id=menuId. The @@ -169,21 +172,3 @@ function convertMenuItemToBlock( menuItem, innerBlocks = [] ) { return createBlock( 'core/navigation-link', attributes, innerBlocks ); } - -function mapMenuItemFieldToBlockAttribute( menuItemField, menuItemVal ) { - const MAPPING = { - object_id: 'id', - object: 'type', - // `nav_menu_item` may be one of: - // 1. `post_type`, - // 2. `post_type_archive`, - // 3. `taxonomy`, - // 4. `custom`. - type: { - attr: 'kind', // the equivalent on `nav_menu_item` is "type" but it is stored with underscores whereas the block attr uses hyphens. - mapper: () => menuItemVal.replace( '_', '-' ), - }, - }; - - return convertAttribute( MAPPING, menuItemField, menuItemVal ); -} diff --git a/packages/edit-navigation/src/store/test/utils.js b/packages/edit-navigation/src/store/test/utils.js index e277571e93aa20..3dd0a90f20842a 100644 --- a/packages/edit-navigation/src/store/test/utils.js +++ b/packages/edit-navigation/src/store/test/utils.js @@ -6,6 +6,8 @@ import { menuItemsQuery, serializeProcessing, computeCustomizedAttribute, + mapBlockAttributeToMenuItemField, + mapMenuItemFieldToBlockAttribute, } from '../utils'; import { isProcessingPost, @@ -246,6 +248,7 @@ describe( 'computeCustomizedAttribute', () => { className: '', rel: '', type: 'page', + kind: 'post-type', }, clientId: 'navigation-link-block-client-id-3', innerBlocks: [], @@ -289,6 +292,8 @@ describe( 'computeCustomizedAttribute', () => { menu_order: 3, menus: [ 1 ], object: 'page', + object_id: 678, + type: 'post_type', }, }; @@ -344,8 +349,9 @@ describe( 'computeCustomizedAttribute', () => { position: 3, status: 'publish', title: 'Page Example', - object: 'page', - object_id: 678, + object: 'page', // equivalent: block.attributes.type + object_id: 678, // equivalent: block.attributes.id + type: 'post_type', // // equivalent: block.attributes.kind url: 'https://localhost:8889/page-example/', xfn: [ '' ], target: '', @@ -353,3 +359,113 @@ describe( 'computeCustomizedAttribute', () => { } ); } ); } ); + +describe( 'Mapping block attributes and menu item fields', () => { + const blocks = [ + { + attributes: { + id: 100, + type: 'page', + kind: 'post-type', + }, + clientId: 'navigation-link-block-client-id-1', + innerBlocks: [], + isValid: true, + name: 'core/navigation-link', + }, + { + attributes: { + id: 101, + type: 'post', + kind: 'post-type', + }, + clientId: 'navigation-link-block-client-id-2', + innerBlocks: [], + isValid: true, + name: 'core/navigation-link', + }, + { + attributes: { + id: 102, + type: 'category', + kind: 'taxonomy', + }, + clientId: 'navigation-link-block-client-id-3', + innerBlocks: [], + isValid: true, + name: 'core/navigation-link', + }, + { + attributes: { + id: 103, + type: 'tag', + kind: 'taxonomy', + }, + clientId: 'navigation-link-block-client-id-4', + innerBlocks: [], + isValid: true, + name: 'core/navigation-link', + }, + ]; + + const menuItems = [ + { + object_id: 100, + object: 'page', + type: 'post_type', + }, + { + object_id: 101, + object: 'post', + type: 'post_type', + }, + { + object_id: 102, + object: 'category', + type: 'taxonomy', + }, + { + object_id: 103, + object: 'tag', + type: 'taxonomy', + }, + ]; + + const blockAttrs = blocks.map( ( block ) => block.attributes ); + + describe( 'mapBlockAttributeToMenuItemField', () => { + it( 'maps block attributes to equivalent menu item fields', () => { + const expected = blockAttrs.map( ( attrs ) => { + return Object.entries( attrs ).reduce( + ( acc, [ key, value ] ) => { + acc = { + ...acc, + ...mapBlockAttributeToMenuItemField( key, value ), + }; + return acc; + }, + {} + ); + } ); + expect( expected ).toEqual( menuItems ); + } ); + } ); + + describe( 'mapMenuItemFieldToBlockAttribute', () => { + it( 'maps menu item fields equivalent block attributes', () => { + const expected = menuItems.map( ( attrs ) => { + return Object.entries( attrs ).reduce( + ( acc, [ key, value ] ) => { + acc = { + ...acc, + ...mapMenuItemFieldToBlockAttribute( key, value ), + }; + return acc; + }, + {} + ); + } ); + expect( expected ).toEqual( blockAttrs ); + } ); + } ); +} ); diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index f052537efc9768..6764ff1cb661e6 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -185,32 +185,63 @@ export function computeCustomizedAttribute( function getMenuItemForBlock( block ) { return omit( menuItemsByClientId[ block.clientId ] || {}, '_links' ); } +} - function mapBlockAttributeToMenuItemField( - blockAttributeName, - blockAttributeValue - ) { - const MAPPING = { - id: 'object_id', - type: 'object', - // `nav_menu_item` may be one of: - // 1. `post_type`, - // 2. `post_type_archive`, - // 3. `taxonomy`, - // 4. `custom`. - kind: { - attr: 'type', // the equivalent on `nav_menu_item` is "type" but it is stored with underscores whereas the block attr uses hyphens. - mapper: () => blockAttributeValue.replace( '-', '_' ), - }, - }; +/** + * Maps a given block attribute to its equivalent menu item field. + * + * @param {string} blockAttributeName the name of the block attribute to be mapped. + * @param {string} blockAttributeValue the value of the block attribute to be mapped. + * @return {Object} block attribute mapped to correct menu item. + */ +export const mapBlockAttributeToMenuItemField = function mapBlockAttributeToMenuItemField( + blockAttributeName, + blockAttributeValue +) { + const MAPPING = { + id: 'object_id', + type: 'object', + // `nav_menu_item` may be one of: + // 1. `post_type`, + // 2. `post_type_archive`, + // 3. `taxonomy`, + // 4. `custom`. + kind: { + attr: 'type', // the equivalent on `nav_menu_item` is "type" but it is stored with underscores whereas the block attr uses hyphens. + mapper: () => blockAttributeValue.replace( '-', '_' ), + }, + }; - return convertAttribute( - MAPPING, - blockAttributeName, - blockAttributeValue - ); - } -} + return convertAttribute( MAPPING, blockAttributeName, blockAttributeValue ); +}; + +/** + * Maps a given menu item field to its equivalent block attribute. + * + * @param {string} menuItemField the name of the menu item field to be mapped. + * @param {string} menuItemVal the value of the menu item field to be mapped. + * @return {Object} menu item field mapped to correct block attribute. + */ +export const mapMenuItemFieldToBlockAttribute = function mapMenuItemFieldToBlockAttribute( + menuItemField, + menuItemVal +) { + const MAPPING = { + object_id: 'id', + object: 'type', + // `nav_menu_item` may be one of: + // 1. `post_type`, + // 2. `post_type_archive`, + // 3. `taxonomy`, + // 4. `custom`. + type: { + attr: 'kind', // the equivalent on `nav_menu_item` is "type" but it is stored with underscores whereas the block attr uses hyphens. + mapper: () => menuItemVal.replace( '_', '-' ), + }, + }; + + return convertAttribute( MAPPING, menuItemField, menuItemVal ); +}; /** * Converts a given key in an object to it's mapped equivalent. @@ -222,7 +253,7 @@ export function computeCustomizedAttribute( * @param {string} val the value to be assigned to the converted field. * @return {Object} the converted field object. */ -export const convertAttribute = function convertAttribute( mapping, key, val ) { +const convertAttribute = function convertAttribute( mapping, key, val ) { const left = mapping[ key ].attr ?? mapping[ key ]; const right = mapping[ key ].mapper?.() ?? val; From 710e61589e649465efe23cffdb11af0c56ab4e74 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Mon, 26 Apr 2021 16:47:15 +0100 Subject: [PATCH 22/48] Fix snapshot test to account for new attributes --- .../navigation-editor.test.js.snap | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/e2e-tests/specs/experiments/__snapshots__/navigation-editor.test.js.snap b/packages/e2e-tests/specs/experiments/__snapshots__/navigation-editor.test.js.snap index 730e2a283f2118..b7cd4e2a823336 100644 --- a/packages/e2e-tests/specs/experiments/__snapshots__/navigation-editor.test.js.snap +++ b/packages/e2e-tests/specs/experiments/__snapshots__/navigation-editor.test.js.snap @@ -10,36 +10,36 @@ exports[`Navigation editor allows creation of a menu when there are no current m exports[`Navigation editor displays the first menu from the REST response when at least one menu exists 1`] = ` " - + - - + + - - - - - + + + + + - + - + - - - - + + + + - - + + " `; From 46d4ae627a2a3790369d6fbff8b7d3e8a21aa00b Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 27 Apr 2021 09:26:15 +0100 Subject: [PATCH 23/48] Only map id if it is not a custom link --- .../src/navigation/placeholder.js | 4 +- .../edit-navigation/src/store/test/utils.js | 38 +++++++++++++++++++ packages/edit-navigation/src/store/utils.js | 1 + 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/navigation/placeholder.js b/packages/block-library/src/navigation/placeholder.js index 2944393563072d..eab8e661371e73 100644 --- a/packages/block-library/src/navigation/placeholder.js +++ b/packages/block-library/src/navigation/placeholder.js @@ -57,7 +57,9 @@ function mapMenuItemsToBlocks( menuItems ) { : menuItem.title.rendered, opensInNewTab: menuItem.target === '_blank', type: menuItem?.object, - id: menuItem?.object_id, + ...( menuItem?.object !== 'custom' && { + id: menuItem?.object_id, + } ), // the equivalent on `nav_menu_item` is "type" but it is stored with underscores whereas the block attr uses hyphens. kind: menuItem.type.replace( '_', '-' ), }; diff --git a/packages/edit-navigation/src/store/test/utils.js b/packages/edit-navigation/src/store/test/utils.js index 3dd0a90f20842a..5f36f3987fbc8d 100644 --- a/packages/edit-navigation/src/store/test/utils.js +++ b/packages/edit-navigation/src/store/test/utils.js @@ -406,6 +406,15 @@ describe( 'Mapping block attributes and menu item fields', () => { isValid: true, name: 'core/navigation-link', }, + { + attributes: { + type: 'custom', + }, + clientId: 'navigation-link-block-client-id-5', + innerBlocks: [], + isValid: true, + name: 'core/navigation-link', + }, ]; const menuItems = [ @@ -429,6 +438,10 @@ describe( 'Mapping block attributes and menu item fields', () => { object: 'tag', type: 'taxonomy', }, + { + object: 'custom', + type: 'custom', + }, ]; const blockAttrs = blocks.map( ( block ) => block.attributes ); @@ -467,5 +480,30 @@ describe( 'Mapping block attributes and menu item fields', () => { } ); expect( expected ).toEqual( blockAttrs ); } ); + + it( 'does not map id to object_id for "custom" block variation', () => { + const customBlockVariationAttrs = { + type: 'custom', + id: 123456, + }; + + const actual = Object.entries( customBlockVariationAttrs ).reduce( + ( acc, [ key, value ] ) => { + acc = { + ...acc, + ...mapMenuItemFieldToBlockAttribute( key, value ), + }; + return acc; + }, + {} + ); + + expect( actual ).toEqual( { + object: 'custom', + type: 'custom', + } ); + + expect( actual.id ).toBeUndefined(); + } ); } ); } ); diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index 6764ff1cb661e6..a30cb6adf98980 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -151,6 +151,7 @@ export function computeCustomizedAttribute( block.attributes?.type ) ), ...( block.attributes?.id && + block.attributes?.type !== 'custom' && mapBlockAttributeToMenuItemField( 'id', block.attributes?.id From 056216009912836b84df140373a65e50599cd96e Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 27 Apr 2021 10:05:54 +0100 Subject: [PATCH 24/48] Update nav block snapshot to include new attributes --- .../__snapshots__/navigation.test.js.snap | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap b/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap index 74860922357e8d..52204753521c4a 100644 --- a/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap +++ b/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap @@ -2,36 +2,36 @@ exports[`Navigation Creating from existing Menus allows a navigation block to be created from existing menus 1`] = ` " - + - - + + - - - - - + + + + + - + - + - - - - + + + + - - + + " `; From a28f7a5bacaeb23eb864b113f8bba79f7d7b5b6c Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 27 Apr 2021 11:01:39 +0100 Subject: [PATCH 25/48] Fix test that could never have passed --- .../edit-navigation/src/store/test/utils.js | 30 ++----------------- 1 file changed, 3 insertions(+), 27 deletions(-) diff --git a/packages/edit-navigation/src/store/test/utils.js b/packages/edit-navigation/src/store/test/utils.js index 5f36f3987fbc8d..44be2384ff4d4c 100644 --- a/packages/edit-navigation/src/store/test/utils.js +++ b/packages/edit-navigation/src/store/test/utils.js @@ -409,6 +409,7 @@ describe( 'Mapping block attributes and menu item fields', () => { { attributes: { type: 'custom', + kind: 'custom', }, clientId: 'navigation-link-block-client-id-5', innerBlocks: [], @@ -446,7 +447,7 @@ describe( 'Mapping block attributes and menu item fields', () => { const blockAttrs = blocks.map( ( block ) => block.attributes ); - describe( 'mapBlockAttributeToMenuItemField', () => { + describe( 'mapping block attributes to menu item fields', () => { it( 'maps block attributes to equivalent menu item fields', () => { const expected = blockAttrs.map( ( attrs ) => { return Object.entries( attrs ).reduce( @@ -464,7 +465,7 @@ describe( 'Mapping block attributes and menu item fields', () => { } ); } ); - describe( 'mapMenuItemFieldToBlockAttribute', () => { + describe( 'mapping menu item fields to block attributes', () => { it( 'maps menu item fields equivalent block attributes', () => { const expected = menuItems.map( ( attrs ) => { return Object.entries( attrs ).reduce( @@ -480,30 +481,5 @@ describe( 'Mapping block attributes and menu item fields', () => { } ); expect( expected ).toEqual( blockAttrs ); } ); - - it( 'does not map id to object_id for "custom" block variation', () => { - const customBlockVariationAttrs = { - type: 'custom', - id: 123456, - }; - - const actual = Object.entries( customBlockVariationAttrs ).reduce( - ( acc, [ key, value ] ) => { - acc = { - ...acc, - ...mapMenuItemFieldToBlockAttribute( key, value ), - }; - return acc; - }, - {} - ); - - expect( actual ).toEqual( { - object: 'custom', - type: 'custom', - } ); - - expect( actual.id ).toBeUndefined(); - } ); } ); } ); From 0bff9c4f54051da144cac48bcf5af3f21bdc25bb Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 27 Apr 2021 11:02:08 +0100 Subject: [PATCH 26/48] Allow for pass through mapping if a mapping is not defined for this attribute --- packages/edit-navigation/src/store/utils.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index a30cb6adf98980..87023506239660 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -255,6 +255,13 @@ export const mapMenuItemFieldToBlockAttribute = function mapMenuItemFieldToBlock * @return {Object} the converted field object. */ const convertAttribute = function convertAttribute( mapping, key, val ) { + // If there is no mapping for this key then default to assuming a 1:1 relationship. + if ( ! mapping[ key ] ) { + return { + [ key ]: val, + }; + } + const left = mapping[ key ].attr ?? mapping[ key ]; const right = mapping[ key ].mapper?.() ?? val; From 7d24fc30a0bbacefe71332d48ff960c7de8684ff Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 29 Apr 2021 09:37:45 +0100 Subject: [PATCH 27/48] Update packages/edit-navigation/src/store/utils.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Albert Juhé Lluveras --- packages/edit-navigation/src/store/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index 87023506239660..ba69a6d851a80a 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -245,7 +245,7 @@ export const mapMenuItemFieldToBlockAttribute = function mapMenuItemFieldToBlock }; /** - * Converts a given key in an object to it's mapped equivalent. + * Converts a given key in an object to its mapped equivalent. * * Typically used to convert block attributes to their equivalent menu item fields. * From 3648914eeaac63c79b4f92b2c96d29ad4e60e915 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 29 Apr 2021 13:27:45 +0100 Subject: [PATCH 28/48] Refactor to create per block/menuItem conversion rather than on per field/attr basis --- .../edit-navigation/src/store/resolvers.js | 32 +-- .../edit-navigation/src/store/test/utils.js | 190 +++++++++++++++--- packages/edit-navigation/src/store/utils.js | 156 +++++--------- 3 files changed, 214 insertions(+), 164 deletions(-) diff --git a/packages/edit-navigation/src/store/resolvers.js b/packages/edit-navigation/src/store/resolvers.js index 59ffdd4ed05ff6..462d4ee26572d8 100644 --- a/packages/edit-navigation/src/store/resolvers.js +++ b/packages/edit-navigation/src/store/resolvers.js @@ -11,17 +11,10 @@ import { parse, createBlock } from '@wordpress/blocks'; /** * Internal dependencies */ -import { - NAVIGATION_POST_KIND, - NAVIGATION_POST_POST_TYPE, - NEW_TAB_TARGET_ATTRIBUTE, -} from '../constants'; +import { NAVIGATION_POST_KIND, NAVIGATION_POST_POST_TYPE } from '../constants'; import { resolveMenuItems, dispatch } from './controls'; -import { - buildNavigationPostId, - mapMenuItemFieldToBlockAttribute, -} from './utils'; +import { buildNavigationPostId, menuItemToBlockAttributes } from './utils'; /** * Creates a "stub" navigation post reflecting the contents of menu with id=menuId. The @@ -149,26 +142,7 @@ function convertMenuItemToBlock( menuItem, innerBlocks = [] ) { return createBlock( block.name, block.attributes, innerBlocks ); } - const attributes = { - label: menuItem.title.rendered, - url: menuItem.url, - title: menuItem.attr_title, - className: menuItem.classes.join( ' ' ), - description: menuItem.description, - rel: menuItem.xfn.join( ' ' ), - ...( menuItem?.object_id && - mapMenuItemFieldToBlockAttribute( - 'object_id', - menuItem.object_id - ) ), - ...( menuItem?.object && - mapMenuItemFieldToBlockAttribute( 'object', menuItem.object ) ), - ...( menuItem?.type && - mapMenuItemFieldToBlockAttribute( 'type', menuItem.type ) ), - ...( menuItem?.target === NEW_TAB_TARGET_ATTRIBUTE && { - opensInNewTab: true, - } ), - }; + const attributes = menuItemToBlockAttributes( menuItem ); return createBlock( 'core/navigation-link', attributes, innerBlocks ); } diff --git a/packages/edit-navigation/src/store/test/utils.js b/packages/edit-navigation/src/store/test/utils.js index 44be2384ff4d4c..6ec7b3fdff7fbb 100644 --- a/packages/edit-navigation/src/store/test/utils.js +++ b/packages/edit-navigation/src/store/test/utils.js @@ -6,8 +6,8 @@ import { menuItemsQuery, serializeProcessing, computeCustomizedAttribute, - mapBlockAttributeToMenuItemField, - mapMenuItemFieldToBlockAttribute, + blockAttributesToMenuItem, + menuItemToBlockAttributes, } from '../utils'; import { isProcessingPost, @@ -220,6 +220,7 @@ describe( 'computeCustomizedAttribute', () => { url: 'http://wp.org', className: 'block classnames', rel: 'external', + type: 'custom', }, clientId: 'navigation-link-block-client-id-1', innerBlocks: [], @@ -233,6 +234,7 @@ describe( 'computeCustomizedAttribute', () => { url: 'http://wp.com', className: '', rel: '', + type: 'custom', }, clientId: 'navigation-link-block-client-id-2', innerBlocks: [], @@ -270,6 +272,7 @@ describe( 'computeCustomizedAttribute', () => { menu_order: 1, menus: [ 1 ], object: 'custom', + original_title: '', }, 'navigation-link-block-client-id-2': { id: 101, @@ -281,6 +284,7 @@ describe( 'computeCustomizedAttribute', () => { menu_order: 2, menus: [ 1 ], object: 'custom', + original_title: '', }, 'navigation-link-block-client-id-3': { id: 102, @@ -294,6 +298,7 @@ describe( 'computeCustomizedAttribute', () => { object: 'page', object_id: 678, type: 'post_type', + original_title: '', }, }; @@ -314,13 +319,17 @@ describe( 'computeCustomizedAttribute', () => { menu_order: 1, nav_menu_term_id: 123, original_title: '', + object: 'custom', position: 1, status: 'publish', - title: 'wp.org', - object: 'custom', + title: { + raw: 'wp.org', + rendered: 'wp.org', + }, url: 'http://wp.org', xfn: [ 'external' ], target: '', + type: 'custom', }, 'nav_menu_item[101]': { _invalid: false, @@ -332,11 +341,15 @@ describe( 'computeCustomizedAttribute', () => { original_title: '', position: 2, status: 'publish', - title: 'wp.com', + title: { + raw: 'wp.com', + rendered: 'wp.com', + }, object: 'custom', url: 'http://wp.com', xfn: [ '' ], target: '_blank', + type: 'custom', }, 'nav_menu_item[102]': { _invalid: false, @@ -348,7 +361,10 @@ describe( 'computeCustomizedAttribute', () => { original_title: '', position: 3, status: 'publish', - title: 'Page Example', + title: { + raw: 'Page Example', + rendered: 'Page Example', + }, object: 'page', // equivalent: block.attributes.type object_id: 678, // equivalent: block.attributes.id type: 'post_type', // // equivalent: block.attributes.kind @@ -364,9 +380,16 @@ describe( 'Mapping block attributes and menu item fields', () => { const blocks = [ { attributes: { + label: 'Example Page', + url: '/example-page/', + description: 'Lorem ipsum dolor sit amet.', + rel: 'friend met', + className: 'my-custom-class-one my-custom-class-two', + title: 'Example page link title attribute', id: 100, type: 'page', kind: 'post-type', + opensInNewTab: true, }, clientId: 'navigation-link-block-client-id-1', innerBlocks: [], @@ -375,9 +398,16 @@ describe( 'Mapping block attributes and menu item fields', () => { }, { attributes: { + label: 'Example Post', + url: '/example-post/', + description: 'Consectetur adipiscing elit.', + rel: 'friend', + className: 'my-custom-class-one', + title: 'Example post link title attribute', id: 101, type: 'post', kind: 'post-type', + opensInNewTab: false, }, clientId: 'navigation-link-block-client-id-2', innerBlocks: [], @@ -386,9 +416,17 @@ describe( 'Mapping block attributes and menu item fields', () => { }, { attributes: { + label: 'Example Category', + url: '/example-category/', + description: + 'Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.', + rel: '', + className: '', + title: '', id: 102, type: 'category', kind: 'taxonomy', + opensInNewTab: true, }, clientId: 'navigation-link-block-client-id-3', innerBlocks: [], @@ -397,9 +435,16 @@ describe( 'Mapping block attributes and menu item fields', () => { }, { attributes: { + label: 'Example Tag', + url: '/example-tag/', + description: '', + rel: '', + className: '', + title: '', id: 103, type: 'tag', kind: 'taxonomy', + opensInNewTab: false, }, clientId: 'navigation-link-block-client-id-4', innerBlocks: [], @@ -408,8 +453,15 @@ describe( 'Mapping block attributes and menu item fields', () => { }, { attributes: { + label: 'Example Custom Link', + url: 'https://wordpress.org', + description: '', + rel: '', + className: '', + title: '', type: 'custom', kind: 'custom', + opensInNewTab: true, }, clientId: 'navigation-link-block-client-id-5', innerBlocks: [], @@ -420,28 +472,79 @@ describe( 'Mapping block attributes and menu item fields', () => { const menuItems = [ { + title: { + raw: 'Example Page', + rendered: 'Example Page', + }, + url: '/example-page/', + description: 'Lorem ipsum dolor sit amet.', + xfn: [ 'friend', 'met' ], + classes: [ 'my-custom-class-one', 'my-custom-class-two' ], + attr_title: 'Example page link title attribute', object_id: 100, object: 'page', type: 'post_type', + target: '_blank', }, { + title: { + raw: 'Example Post', + rendered: 'Example Post', + }, + url: '/example-post/', + description: 'Consectetur adipiscing elit.', + xfn: [ 'friend' ], + classes: [ 'my-custom-class-one' ], + attr_title: 'Example post link title attribute', object_id: 101, object: 'post', type: 'post_type', + target: '', }, { + title: { + raw: 'Example Category', + rendered: 'Example Category', + }, + url: '/example-category/', + description: + 'Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.', + xfn: [ '' ], + classes: [ '' ], + attr_title: '', object_id: 102, object: 'category', type: 'taxonomy', + target: '_blank', }, { + title: { + raw: 'Example Tag', + rendered: 'Example Tag', + }, + url: '/example-tag/', + description: '', + xfn: [ '' ], + classes: [ '' ], + attr_title: '', object_id: 103, object: 'tag', type: 'taxonomy', + target: '', }, { + title: { + raw: 'Example Custom Link', + rendered: 'Example Custom Link', + }, + url: 'https://wordpress.org', + description: '', + xfn: [ '' ], + classes: [ '' ], + attr_title: '', object: 'custom', type: 'custom', + target: '_blank', }, ]; @@ -449,37 +552,58 @@ describe( 'Mapping block attributes and menu item fields', () => { describe( 'mapping block attributes to menu item fields', () => { it( 'maps block attributes to equivalent menu item fields', () => { - const expected = blockAttrs.map( ( attrs ) => { - return Object.entries( attrs ).reduce( - ( acc, [ key, value ] ) => { - acc = { - ...acc, - ...mapBlockAttributeToMenuItemField( key, value ), - }; - return acc; - }, - {} - ); - } ); - expect( expected ).toEqual( menuItems ); + const actual = blockAttrs.map( ( attrs ) => + blockAttributesToMenuItem( attrs ) + ); + + expect( actual ).toEqual( menuItems ); + } ); + + it( 'does not map block attribute "id" to menu item "object_id" field for custom (non-entity) links', () => { + const customLinkBlockAttributes = { + id: 123456, // added for test purposes only - should not occur. + title: 'Example Custom Link', + url: 'https://wordpress.org', + description: '', + xfn: [ '' ], + classes: [ '' ], + attr_title: '', + object: 'custom', + type: 'custom', + }; + const actual = blockAttributesToMenuItem( + customLinkBlockAttributes + ); + + expect( actual.object_id ).toBeUndefined(); } ); } ); describe( 'mapping menu item fields to block attributes', () => { - it( 'maps menu item fields equivalent block attributes', () => { - const expected = menuItems.map( ( attrs ) => { - return Object.entries( attrs ).reduce( - ( acc, [ key, value ] ) => { - acc = { - ...acc, - ...mapMenuItemFieldToBlockAttribute( key, value ), - }; - return acc; - }, - {} - ); - } ); - expect( expected ).toEqual( blockAttrs ); + it( 'maps menu item fields to equivalent block attributes', () => { + const actual = menuItems.map( ( fields ) => + menuItemToBlockAttributes( fields ) + ); + + expect( actual ).toEqual( blockAttrs ); + } ); + + it( 'does not map menu item "object_id" field to block attribute "id" for custom (non-entity) links', () => { + const customLinkMenuItem = { + title: 'Example Custom Link', + url: 'https://wordpress.org', + description: '', + xfn: [ '' ], + classes: [ '' ], + attr_title: '', + object_id: 123456, // added for test purposes. + object: 'custom', + type: 'custom', + target: '_blank', + }; + const actual = menuItemToBlockAttributes( customLinkMenuItem ); + + expect( actual.id ).toBeUndefined(); } ); } ); } ); diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index ba69a6d851a80a..80c75f860661e8 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -137,34 +137,7 @@ export function computeCustomizedAttribute( let attributes; if ( block.name === 'core/navigation-link' ) { - attributes = { - title: block.attributes?.label, - original_title: '', - url: block.attributes.url, - description: block.attributes.description, - xfn: block.attributes.rel?.split( ' ' ), - classes: block.attributes.className?.split( ' ' ), - attr_title: block.attributes.title, - ...( block.attributes?.type && - mapBlockAttributeToMenuItemField( - 'type', - block.attributes?.type - ) ), - ...( block.attributes?.id && - block.attributes?.type !== 'custom' && - mapBlockAttributeToMenuItemField( - 'id', - block.attributes?.id - ) ), - ...( block.attributes?.kind && - mapBlockAttributeToMenuItemField( - 'kind', - block.attributes?.kind - ) ), - target: block.attributes.opensInNewTab - ? NEW_TAB_TARGET_ATTRIBUTE - : '', - }; + attributes = blockAttributesToMenuItem( block.attributes ); } else { attributes = { type: 'block', @@ -188,84 +161,63 @@ export function computeCustomizedAttribute( } } -/** - * Maps a given block attribute to its equivalent menu item field. - * - * @param {string} blockAttributeName the name of the block attribute to be mapped. - * @param {string} blockAttributeValue the value of the block attribute to be mapped. - * @return {Object} block attribute mapped to correct menu item. - */ -export const mapBlockAttributeToMenuItemField = function mapBlockAttributeToMenuItemField( - blockAttributeName, - blockAttributeValue -) { - const MAPPING = { - id: 'object_id', - type: 'object', - // `nav_menu_item` may be one of: - // 1. `post_type`, - // 2. `post_type_archive`, - // 3. `taxonomy`, - // 4. `custom`. - kind: { - attr: 'type', // the equivalent on `nav_menu_item` is "type" but it is stored with underscores whereas the block attr uses hyphens. - mapper: () => blockAttributeValue.replace( '-', '_' ), - }, - }; - - return convertAttribute( MAPPING, blockAttributeName, blockAttributeValue ); -}; - -/** - * Maps a given menu item field to its equivalent block attribute. - * - * @param {string} menuItemField the name of the menu item field to be mapped. - * @param {string} menuItemVal the value of the menu item field to be mapped. - * @return {Object} menu item field mapped to correct block attribute. - */ -export const mapMenuItemFieldToBlockAttribute = function mapMenuItemFieldToBlockAttribute( - menuItemField, - menuItemVal -) { - const MAPPING = { - object_id: 'id', - object: 'type', - // `nav_menu_item` may be one of: - // 1. `post_type`, - // 2. `post_type_archive`, - // 3. `taxonomy`, - // 4. `custom`. - type: { - attr: 'kind', // the equivalent on `nav_menu_item` is "type" but it is stored with underscores whereas the block attr uses hyphens. - mapper: () => menuItemVal.replace( '_', '-' ), +export const blockAttributesToMenuItem = ( { + label, + url, + description, + rel, + className, + title, + type, + id, + kind, + opensInNewTab, +} ) => { + return { + title: { + rendered: label, + raw: label, }, + xfn: rel?.trim().split( ' ' ), + classes: className?.trim().split( ' ' ), + attr_title: title, + object: type || 'custom', + ...( 'custom' !== type && { + object_id: id, + } ), + description, + url, + type: kind?.replace( '-', '_' ) ?? 'custom', + target: opensInNewTab ? NEW_TAB_TARGET_ATTRIBUTE : '', }; - - return convertAttribute( MAPPING, menuItemField, menuItemVal ); }; -/** - * Converts a given key in an object to its mapped equivalent. - * - * Typically used to convert block attributes to their equivalent menu item fields. - * - * @param {Object} mapping a key/value mapping of attributes to their equivalents. - * @param {string} key the name of the key within the mapping to be converted. - * @param {string} val the value to be assigned to the converted field. - * @return {Object} the converted field object. - */ -const convertAttribute = function convertAttribute( mapping, key, val ) { - // If there is no mapping for this key then default to assuming a 1:1 relationship. - if ( ! mapping[ key ] ) { - return { - [ key ]: val, - }; - } - - const left = mapping[ key ].attr ?? mapping[ key ]; - const right = mapping[ key ].mapper?.() ?? val; - +export const menuItemToBlockAttributes = ( { + title: menuItemTitleField, + xfn, + classes, + // eslint-disable-next-line camelcase + attr_title, + object, + // eslint-disable-next-line camelcase + object_id, + description, + url, + type: menuItemTypeField, + target, +} ) => { return { - [ left ]: right, + label: menuItemTitleField.rendered, + rel: xfn.join( ' ' ).trim(), + className: classes.join( ' ' ).trim(), + title: attr_title, + type: object || 'custom', + ...( 'custom' !== object && { + id: object_id, + } ), + description, + url, + kind: menuItemTypeField?.replace( '_', '-' ) ?? 'custom', + opensInNewTab: target === NEW_TAB_TARGET_ATTRIBUTE, }; }; From 0873d9c3d454ad703c2d3b9287750833448a0b3f Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 29 Apr 2021 13:46:37 +0100 Subject: [PATCH 29/48] Doc blocks --- packages/edit-navigation/src/store/utils.js | 35 +++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index 80c75f860661e8..370d7fffe210e2 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -161,6 +161,22 @@ export function computeCustomizedAttribute( } } +/** + * Convert block attributes to menu item fields. + * + * @param {Object} blockAttributes the block attributes of the block to be converted into menu item fields. + * @param {string} blockAttributes.label the visual name of the block shown in the UI. + * @param {string} blockAttributes.url the URL for the link. + * @param {string} blockAttributes.description a link description. + * @param {string} blockAttributes.rel the XFN relationship expressed in the link of this menu item. + * @param {string} blockAttributes.className the custom CSS classname attributes for this block. + * @param {string} blockAttributes.title the HTML title attribute for the block's link. + * @param {string} blockAttributes.type the type of variation of the block used (eg: 'Post', 'Custom', 'Category'...etc). + * @param {number} blockAttributes.id the ID of the entity optionally associated with the block's link (eg: the Post ID). + * @param {string} blockAttributes.kind the family of objects originally represented, such as 'post-type' or 'taxonomy'. + * @param {boolean} blockAttributes.opensInNewTab whether or not the block's link should open in a new tab. + * @return {Object} the menu item (converted from block attributes). + */ export const blockAttributesToMenuItem = ( { label, url, @@ -192,6 +208,25 @@ export const blockAttributesToMenuItem = ( { }; }; +/** + * Convert block attributes to menu item. + * + * For more documentation on the individual fields present on a menu item please see: + * https://core.trac.wordpress.org/browser/tags/5.7.1/src/wp-includes/nav-menu.php#L789 + * + * @param {Object} menuItem the menu item to be converted to block attributes. + * @param {Object} menuItem.title stores the raw and rendered versions of the title/label for this menu item. + * @param {Array} menuItem.xfn the XFN relationships expressed in the link of this menu item. + * @param {Array} menuItem.classes the HTML class attributes for this menu item. + * @param {string} menuItem.attr_title the HTML title attribute for this menu item. + * @param {string} menuItem.object The type of object originally represented, such as 'category', 'post', or 'attachment'. + * @param {string} menuItem.object_id The DB ID of the original object this menu item represents, e.g. ID for posts and term_id for categories. + * @param {string} menuItem.description The description of this menu item. + * @param {string} menuItem.url The URL to which this menu item points. + * @param {string} menuItem.type The family of objects originally represented, such as 'post_type' or 'taxonomy'. + * @param {string} menuItem.target The target attribute of the link element for this menu item. + * @return {Object} the block attributes converted from the menu item. + */ export const menuItemToBlockAttributes = ( { title: menuItemTitleField, xfn, From 9caccfd985b4666e5946116b4527a005e706c56f Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 29 Apr 2021 14:01:31 +0100 Subject: [PATCH 30/48] Reuse menu to blocks conversion util on navigation-link block --- .../navigation/map-menu-items-to-blocks.js | 83 +++++++++++++++++++ .../src/navigation/placeholder.js | 65 +-------------- 2 files changed, 85 insertions(+), 63 deletions(-) create mode 100644 packages/block-library/src/navigation/map-menu-items-to-blocks.js diff --git a/packages/block-library/src/navigation/map-menu-items-to-blocks.js b/packages/block-library/src/navigation/map-menu-items-to-blocks.js new file mode 100644 index 00000000000000..d3e0b88cbd1bb8 --- /dev/null +++ b/packages/block-library/src/navigation/map-menu-items-to-blocks.js @@ -0,0 +1,83 @@ +/** + * WordPress dependencies + */ +import { createBlock, parse } from '@wordpress/blocks'; + +/** + * Convert block attributes to menu item. + * + * For more documentation on the individual fields present on a menu item please see: + * https://core.trac.wordpress.org/browser/tags/5.7.1/src/wp-includes/nav-menu.php#L789 + * + * @param {Object} menuItem the menu item to be converted to block attributes. + * @param {Object} menuItem.title stores the raw and rendered versions of the title/label for this menu item. + * @param {Array} menuItem.xfn the XFN relationships expressed in the link of this menu item. + * @param {Array} menuItem.classes the HTML class attributes for this menu item. + * @param {string} menuItem.attr_title the HTML title attribute for this menu item. + * @param {string} menuItem.object The type of object originally represented, such as 'category', 'post', or 'attachment'. + * @param {string} menuItem.object_id The DB ID of the original object this menu item represents, e.g. ID for posts and term_id for categories. + * @param {string} menuItem.description The description of this menu item. + * @param {string} menuItem.url The URL to which this menu item points. + * @param {string} menuItem.type The family of objects originally represented, such as 'post_type' or 'taxonomy'. + * @param {string} menuItem.target The target attribute of the link element for this menu item. + * @return {Object} the block attributes converted from the menu item. + */ +export const menuItemToBlockAttributes = ( { + title: menuItemTitleField, + xfn, + classes, + // eslint-disable-next-line camelcase + attr_title, + object, + // eslint-disable-next-line camelcase + object_id, + description, + url, + type: menuItemTypeField, + target, +} ) => { + return { + label: menuItemTitleField.rendered, + rel: xfn.join( ' ' ).trim(), + className: classes.join( ' ' ).trim(), + title: attr_title, + type: object || 'custom', + ...( 'custom' !== object && { + id: object_id, + } ), + description, + url, + kind: menuItemTypeField?.replace( '_', '-' ) ?? 'custom', + opensInNewTab: target === '_blank', + }; +}; + +/** + * A recursive function that maps menu item nodes to blocks. + * + * @param {Object[]} menuItems An array of menu items. + * @return {WPBlock[]} An array of blocks. + */ +export default function mapMenuItemsToBlocks( menuItems ) { + return menuItems.map( ( menuItem ) => { + if ( menuItem.type === 'block' ) { + const [ block ] = parse( menuItem.content.raw ); + + if ( ! block ) { + return createBlock( 'core/freeform', { + content: menuItem.content, + } ); + } + + return block; + } + + const attributes = menuItemToBlockAttributes( menuItem ); + + const innerBlocks = menuItem.children?.length + ? mapMenuItemsToBlocks( menuItem.children ) + : []; + + return createBlock( 'core/navigation-link', attributes, innerBlocks ); + } ); +} diff --git a/packages/block-library/src/navigation/placeholder.js b/packages/block-library/src/navigation/placeholder.js index eab8e661371e73..aabb615d12ab1f 100644 --- a/packages/block-library/src/navigation/placeholder.js +++ b/packages/block-library/src/navigation/placeholder.js @@ -1,12 +1,7 @@ -/** - * External dependencies - */ -import { some } from 'lodash'; - /** * WordPress dependencies */ -import { createBlock, parse } from '@wordpress/blocks'; +import { createBlock } from '@wordpress/blocks'; import { Button, DropdownMenu, @@ -29,65 +24,9 @@ import { store as coreStore } from '@wordpress/core-data'; * Internal dependencies */ import createDataTree from './create-data-tree'; +import mapMenuItemsToBlocks from './map-menu-items-to-blocks'; import PlaceholderPreview from './placeholder-preview'; -/** - * A recursive function that maps menu item nodes to blocks. - * - * @param {Object[]} menuItems An array of menu items. - * @return {WPBlock[]} An array of blocks. - */ -function mapMenuItemsToBlocks( menuItems ) { - return menuItems.map( ( menuItem ) => { - if ( menuItem.type === 'block' ) { - const [ block ] = parse( menuItem.content.raw ); - - if ( ! block ) { - return createBlock( 'core/freeform', { - content: menuItem.content, - } ); - } - - return block; - } - - const attributes = { - label: ! menuItem.title.rendered - ? __( '(no title)' ) - : menuItem.title.rendered, - opensInNewTab: menuItem.target === '_blank', - type: menuItem?.object, - ...( menuItem?.object !== 'custom' && { - id: menuItem?.object_id, - } ), - // the equivalent on `nav_menu_item` is "type" but it is stored with underscores whereas the block attr uses hyphens. - kind: menuItem.type.replace( '_', '-' ), - }; - - if ( menuItem.url ) { - attributes.url = menuItem.url; - } - - if ( menuItem.description ) { - attributes.description = menuItem.description; - } - - if ( menuItem.xfn?.length && some( menuItem.xfn ) ) { - attributes.rel = menuItem.xfn.join( ' ' ); - } - - if ( menuItem.classes?.length && some( menuItem.classes ) ) { - attributes.className = menuItem.classes.join( ' ' ); - } - - const innerBlocks = menuItem.children?.length - ? mapMenuItemsToBlocks( menuItem.children ) - : []; - - return createBlock( 'core/navigation-link', attributes, innerBlocks ); - } ); -} - /** * Convert a flat menu item structure to a nested blocks structure. * From 72508108132e83bfee3fb6b1b61f1b069b7f6c1c Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 29 Apr 2021 14:32:52 +0100 Subject: [PATCH 31/48] Fix resolver unit tests --- packages/edit-navigation/src/store/test/resolvers.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/edit-navigation/src/store/test/resolvers.js b/packages/edit-navigation/src/store/test/resolvers.js index 6e962f967f5500..8819fc447cbe36 100644 --- a/packages/edit-navigation/src/store/test/resolvers.js +++ b/packages/edit-navigation/src/store/test/resolvers.js @@ -139,6 +139,9 @@ describe( 'getNavigationPostForMenu', () => { rel: 'nofollow', description: 'description', title: 'link title', + opensInNewTab: false, + kind: 'custom', + type: 'custom', }, clientId: 'client-id-0', innerBlocks: [], @@ -153,6 +156,8 @@ describe( 'getNavigationPostForMenu', () => { description: '', title: '', opensInNewTab: true, + kind: 'custom', + type: 'custom', }, clientId: 'client-id-1', innerBlocks: [], From fde60035446a8e2476924d1da4227e579cb59226 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 29 Apr 2021 14:37:39 +0100 Subject: [PATCH 32/48] Add new resolver test to assert on entity type conversion --- .../src/store/test/resolvers.js | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/packages/edit-navigation/src/store/test/resolvers.js b/packages/edit-navigation/src/store/test/resolvers.js index 8819fc447cbe36..01dd870306b97f 100644 --- a/packages/edit-navigation/src/store/test/resolvers.js +++ b/packages/edit-navigation/src/store/test/resolvers.js @@ -108,6 +108,25 @@ describe( 'getNavigationPostForMenu', () => { attr_title: '', target: '_blank', }, + { + id: 102, + title: { + raw: 'My Example Page', + rendered: 'My Example Page', + }, + url: '/my-example-page/', + object: 'page', + object_id: 56789, + type: 'post-type', + menu_order: 3, + menus: [ 1 ], + parent: 0, + classes: [], + xfn: [], + description: '', + attr_title: '', + target: '_blank', + }, ]; expect( generator.next( menuItems ).value ).toEqual( { @@ -116,6 +135,7 @@ describe( 'getNavigationPostForMenu', () => { mapping: { 100: expect.stringMatching( /client-id-\d+/ ), 101: expect.stringMatching( /client-id-\d+/ ), + 102: expect.stringMatching( /client-id-\d+/ ), }, } ); @@ -163,6 +183,23 @@ describe( 'getNavigationPostForMenu', () => { innerBlocks: [], name: 'core/navigation-link', }, + { + attributes: { + label: 'My Example Page', + url: '/my-example-page/', + className: '', + rel: '', + description: '', + title: '', + opensInNewTab: true, + kind: 'post-type', + type: 'page', + id: 56789, + }, + clientId: 'client-id-2', + innerBlocks: [], + name: 'core/navigation-link', + }, ], name: 'core/navigation', }, From 70aa6ad1a82f9db5999bbdaaaa616e6db1bac6b2 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 29 Apr 2021 15:18:29 +0100 Subject: [PATCH 33/48] Update snapshots --- .../__snapshots__/navigation.test.js.snap | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap b/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap index 52204753521c4a..72771912dea747 100644 --- a/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap +++ b/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap @@ -2,36 +2,36 @@ exports[`Navigation Creating from existing Menus allows a navigation block to be created from existing menus 1`] = ` " - + - - + + - - - - - + + + + + - + - + - - - - + + + + - - + + " `; From 57e8697ea982e37999ae2bfce98951bab17415d8 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 29 Apr 2021 15:22:21 +0100 Subject: [PATCH 34/48] Fix nav editor snapshots to remove IDs from custom link types --- .../__snapshots__/navigation-editor.test.js.snap | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/e2e-tests/specs/experiments/__snapshots__/navigation-editor.test.js.snap b/packages/e2e-tests/specs/experiments/__snapshots__/navigation-editor.test.js.snap index b7cd4e2a823336..2978d5d63f9ffe 100644 --- a/packages/e2e-tests/specs/experiments/__snapshots__/navigation-editor.test.js.snap +++ b/packages/e2e-tests/specs/experiments/__snapshots__/navigation-editor.test.js.snap @@ -10,7 +10,7 @@ exports[`Navigation editor allows creation of a menu when there are no current m exports[`Navigation editor displays the first menu from the REST response when at least one menu exists 1`] = ` " - + @@ -38,8 +38,8 @@ exports[`Navigation editor displays the first menu from the REST response when a - - + + " `; From 761f67873e54a43fa6dd2b38367ec4fc6895d6c5 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 29 Apr 2021 16:03:03 +0100 Subject: [PATCH 35/48] Avoid overwriting the ID attribute when calling onChange with no ID prop --- .../block-library/src/navigation-link/edit.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js index a1ce7bb2781d9c..8903aac594c763 100644 --- a/packages/block-library/src/navigation-link/edit.js +++ b/packages/block-library/src/navigation-link/edit.js @@ -523,10 +523,10 @@ export default function NavigationLinkEdit( { title: newTitle = '', url: newURL = '', opensInNewTab: newOpensInNewTab, - id, + id: newId, kind: newKind = '', type: newType = '', - } = {} ) => + } = {} ) => { setAttributes( { url: encodeURI( newURL ), label: ( () => { @@ -552,7 +552,12 @@ export default function NavigationLinkEdit( { return escape( normalizedURL ); } )(), opensInNewTab: newOpensInNewTab, - id, + // `id` represents the DB ID of the entity which this link represents (eg: Post ID). + // Therefore we must not inadvertently set it to `undefined` if the `onChange` is called with no `id` value. + // This is possible when a setting changes such as the `opensInNewTab`. + ...( newId && { + id: newId, + } ), ...( newKind && { kind: newKind, } ), @@ -561,8 +566,8 @@ export default function NavigationLinkEdit( { newType !== 'post-format' && { type: newType, } ), - } ) - } + } ); + } } /> ) } From 83f42d116c07a1ce123eb66453db5af602a86ec9 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 30 Apr 2021 08:44:58 +0100 Subject: [PATCH 36/48] Adds full conversion cycle tests --- .../edit-navigation/src/store/test/utils.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/edit-navigation/src/store/test/utils.js b/packages/edit-navigation/src/store/test/utils.js index 6ec7b3fdff7fbb..a1893746d9d8d1 100644 --- a/packages/edit-navigation/src/store/test/utils.js +++ b/packages/edit-navigation/src/store/test/utils.js @@ -606,4 +606,22 @@ describe( 'Mapping block attributes and menu item fields', () => { expect( actual.id ).toBeUndefined(); } ); } ); + + describe( 'full conversion cycles', () => { + it( 'converts menu item fields to block attributes and back again without data loss', () => { + const actual = menuItems + .map( ( fields ) => menuItemToBlockAttributes( fields ) ) + .map( ( attrs ) => blockAttributesToMenuItem( attrs ) ); + + expect( actual ).toEqual( menuItems ); + } ); + + it( 'converts block attributes to menu item fields and back again without data loss', () => { + const actual = blockAttrs + .map( ( attrs ) => blockAttributesToMenuItem( attrs ) ) + .map( ( fields ) => menuItemToBlockAttributes( fields ) ); + + expect( actual ).toEqual( blockAttrs ); + } ); + } ); } ); From 667c8187b98b76c0b518b523d72704515e6ff72a Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 30 Apr 2021 08:52:49 +0100 Subject: [PATCH 37/48] =?UTF-8?q?Fix=20invalid=20test=20that=20wasn?= =?UTF-8?q?=E2=80=99t=20performing=20correct=20assertion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../edit-navigation/src/store/test/utils.js | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/packages/edit-navigation/src/store/test/utils.js b/packages/edit-navigation/src/store/test/utils.js index a1893746d9d8d1..bfb03f5f529d03 100644 --- a/packages/edit-navigation/src/store/test/utils.js +++ b/packages/edit-navigation/src/store/test/utils.js @@ -561,8 +561,28 @@ describe( 'Mapping block attributes and menu item fields', () => { it( 'does not map block attribute "id" to menu item "object_id" field for custom (non-entity) links', () => { const customLinkBlockAttributes = { - id: 123456, // added for test purposes only - should not occur. - title: 'Example Custom Link', + id: 12345, // added for test purposes only - should't exist. + type: 'custom', // custom type indicates we shouldn't need an `id` field. + kind: 'custom', // custom type indicates we shouldn't need an `id` field. + label: 'Example Custom Link', + url: 'https://wordpress.org', + description: '', + rel: '', + className: '', + title: '', + opensInNewTab: true, + }; + + const actual = blockAttributesToMenuItem( + customLinkBlockAttributes + ); + + // Check the actual conversion to menuItem happened successfully. + expect( actual ).toEqual( { + title: { + raw: 'Example Custom Link', + rendered: 'Example Custom Link', + }, url: 'https://wordpress.org', description: '', xfn: [ '' ], @@ -570,11 +590,10 @@ describe( 'Mapping block attributes and menu item fields', () => { attr_title: '', object: 'custom', type: 'custom', - }; - const actual = blockAttributesToMenuItem( - customLinkBlockAttributes - ); + target: '_blank', + } ); + // Assert `id` attr has not been converted to a `object_id` field for a "custom" type even if present. expect( actual.object_id ).toBeUndefined(); } ); } ); From 0bd64e82c88512b1231e5ddfa970844e3747a81f Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 30 Apr 2021 10:17:14 +0100 Subject: [PATCH 38/48] Refactor conversions to omit default values. --- .../edit-navigation/src/store/test/utils.js | 88 +++++++++++++++++++ packages/edit-navigation/src/store/utils.js | 79 ++++++++++++----- 2 files changed, 144 insertions(+), 23 deletions(-) diff --git a/packages/edit-navigation/src/store/test/utils.js b/packages/edit-navigation/src/store/test/utils.js index bfb03f5f529d03..db85fb5740167e 100644 --- a/packages/edit-navigation/src/store/test/utils.js +++ b/packages/edit-navigation/src/store/test/utils.js @@ -596,6 +596,52 @@ describe( 'Mapping block attributes and menu item fields', () => { // Assert `id` attr has not been converted to a `object_id` field for a "custom" type even if present. expect( actual.object_id ).toBeUndefined(); } ); + + it( 'gracefully handles undefined values by falling back to menu item defaults', () => { + const blockAttrsWithUndefinedValues = { + id: undefined, + type: undefined, + kind: undefined, + label: undefined, + url: undefined, + description: undefined, + rel: undefined, + className: undefined, + title: undefined, + opensInNewTab: undefined, + }; + + const actual = blockAttributesToMenuItem( + blockAttrsWithUndefinedValues + ); + + // Defaults are taken from https://core.trac.wordpress.org/browser/tags/5.7.1/src/wp-includes/nav-menu.php#L438. + expect( actual ).toEqual( + expect.objectContaining( { + title: { + raw: '', + rendered: '', + }, + url: '', + } ) + ); + + // Remaining values should not be present. + expect( Object.keys( actual ) ).not.toEqual( + expect.arrayContaining( [ + 'description', + 'xfn', + 'classes', + 'attr_title', + 'object', + 'type', + 'object_id', + 'target', + ] ) + ); + + expect( Object.values( actual ) ).not.toContain( undefined ); + } ); } ); describe( 'mapping menu item fields to block attributes', () => { @@ -624,6 +670,48 @@ describe( 'Mapping block attributes and menu item fields', () => { expect( actual.id ).toBeUndefined(); } ); + + it( 'gracefully handles undefined values by falling back to block attribute defaults', () => { + // Note that whilst Core provides default values for nav_menu_item's it is possible that these + // values could be manipulated via Plugins. As such we must account for unexpected values. + const menuItemsWithUndefinedValues = { + title: undefined, + url: undefined, + description: undefined, + xfn: undefined, + classes: undefined, + attr_title: undefined, + object_id: undefined, + object: undefined, + type: undefined, + target: undefined, + }; + + const actual = menuItemToBlockAttributes( + menuItemsWithUndefinedValues + ); + + expect( actual ).toEqual( + expect.objectContaining( { + label: '', + type: 'custom', + kind: 'custom', + } ) + ); + + expect( Object.keys( actual ) ).not.toEqual( + expect.arrayContaining( [ + 'rel', + 'className', + 'title', + 'id', + 'description', + 'opensInNewTab', + ] ) + ); + + expect( Object.values( actual ) ).not.toContain( undefined ); + } ); } ); describe( 'full conversion cycles', () => { diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index 370d7fffe210e2..da42ce8b10769c 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -164,6 +164,11 @@ export function computeCustomizedAttribute( /** * Convert block attributes to menu item fields. * + * Note that nav_menu_item has defaults provided in Core so in the case of undefined Block attributes + * we need only include a subset of values in the knowledge that the defaults will be provided in Core. + * + * See: https://core.trac.wordpress.org/browser/tags/5.7.1/src/wp-includes/nav-menu.php#L438. + * * @param {Object} blockAttributes the block attributes of the block to be converted into menu item fields. * @param {string} blockAttributes.label the visual name of the block shown in the UI. * @param {string} blockAttributes.url the URL for the link. @@ -178,12 +183,12 @@ export function computeCustomizedAttribute( * @return {Object} the menu item (converted from block attributes). */ export const blockAttributesToMenuItem = ( { - label, - url, + label = '', + url = '', description, rel, className, - title, + title: blockTitleAttr, type, id, kind, @@ -194,17 +199,33 @@ export const blockAttributesToMenuItem = ( { rendered: label, raw: label, }, - xfn: rel?.trim().split( ' ' ), - classes: className?.trim().split( ' ' ), - attr_title: title, - object: type || 'custom', - ...( 'custom' !== type && { - object_id: id, - } ), - description, url, - type: kind?.replace( '-', '_' ) ?? 'custom', - target: opensInNewTab ? NEW_TAB_TARGET_ATTRIBUTE : '', + ...( description?.length && { + description, + } ), + ...( rel?.length && { + xfn: rel?.trim().split( ' ' ), + } ), + ...( className?.length && { + classes: className?.trim().split( ' ' ), + } ), + ...( blockTitleAttr?.length && { + attr_title: blockTitleAttr, + } ), + ...( type?.length && { + object: type, + } ), + ...( kind?.length && { + type: kind?.replace( '-', '_' ), + } ), + // Only assign object_id if it's a entity type (ie: not "custom"). + ...( id && + 'custom' !== type && { + object_id: id, + } ), + ...( opensInNewTab && { + target: NEW_TAB_TARGET_ATTRIBUTE, + } ), }; }; @@ -242,17 +263,29 @@ export const menuItemToBlockAttributes = ( { target, } ) => { return { - label: menuItemTitleField.rendered, - rel: xfn.join( ' ' ).trim(), - className: classes.join( ' ' ).trim(), - title: attr_title, + label: menuItemTitleField?.rendered || '', type: object || 'custom', - ...( 'custom' !== object && { - id: object_id, + kind: menuItemTypeField?.replace( '_', '-' ) || 'custom', + url: url || '', + ...( xfn?.length && { + rel: xfn.join( ' ' ).trim(), + } ), + ...( classes?.length && { + className: classes.join( ' ' ).trim(), + } ), + ...( attr_title?.length && { + title: attr_title, + } ), + // eslint-disable-next-line camelcase + ...( object_id && + 'custom' !== object && { + id: object_id, + } ), + ...( description?.length && { + description, + } ), + ...( target === NEW_TAB_TARGET_ATTRIBUTE && { + opensInNewTab: true, } ), - description, - url, - kind: menuItemTypeField?.replace( '_', '-' ) ?? 'custom', - opensInNewTab: target === NEW_TAB_TARGET_ATTRIBUTE, }; }; From 9d88bf8ba437adcbd2046b22adcec8bf664e9693 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 30 Apr 2021 10:49:38 +0100 Subject: [PATCH 39/48] Update tests to expect default or omitted values upon conversion --- .../src/store/test/resolvers.js | 9 - .../edit-navigation/src/store/test/utils.js | 433 ++++++++++++------ packages/edit-navigation/src/store/utils.js | 14 +- 3 files changed, 290 insertions(+), 166 deletions(-) diff --git a/packages/edit-navigation/src/store/test/resolvers.js b/packages/edit-navigation/src/store/test/resolvers.js index 01dd870306b97f..d32e5d207cef0f 100644 --- a/packages/edit-navigation/src/store/test/resolvers.js +++ b/packages/edit-navigation/src/store/test/resolvers.js @@ -159,7 +159,6 @@ describe( 'getNavigationPostForMenu', () => { rel: 'nofollow', description: 'description', title: 'link title', - opensInNewTab: false, kind: 'custom', type: 'custom', }, @@ -171,10 +170,6 @@ describe( 'getNavigationPostForMenu', () => { attributes: { label: 'wp.org', url: 'http://wp.org', - className: '', - rel: '', - description: '', - title: '', opensInNewTab: true, kind: 'custom', type: 'custom', @@ -187,10 +182,6 @@ describe( 'getNavigationPostForMenu', () => { attributes: { label: 'My Example Page', url: '/my-example-page/', - className: '', - rel: '', - description: '', - title: '', opensInNewTab: true, kind: 'post-type', type: 'page', diff --git a/packages/edit-navigation/src/store/test/utils.js b/packages/edit-navigation/src/store/test/utils.js index db85fb5740167e..35b9c65b365b79 100644 --- a/packages/edit-navigation/src/store/test/utils.js +++ b/packages/edit-navigation/src/store/test/utils.js @@ -221,6 +221,7 @@ describe( 'computeCustomizedAttribute', () => { className: 'block classnames', rel: 'external', type: 'custom', + kind: 'custom', }, clientId: 'navigation-link-block-client-id-1', innerBlocks: [], @@ -235,6 +236,7 @@ describe( 'computeCustomizedAttribute', () => { className: '', rel: '', type: 'custom', + kind: 'custom', }, clientId: 'navigation-link-block-client-id-2', innerBlocks: [], @@ -328,12 +330,10 @@ describe( 'computeCustomizedAttribute', () => { }, url: 'http://wp.org', xfn: [ 'external' ], - target: '', type: 'custom', }, 'nav_menu_item[101]': { _invalid: false, - classes: [ '' ], id: 101, menu_item_parent: 0, menu_order: 2, @@ -347,13 +347,11 @@ describe( 'computeCustomizedAttribute', () => { }, object: 'custom', url: 'http://wp.com', - xfn: [ '' ], target: '_blank', type: 'custom', }, 'nav_menu_item[102]': { _invalid: false, - classes: [ '' ], id: 102, menu_item_parent: 0, menu_order: 3, @@ -369,17 +367,195 @@ describe( 'computeCustomizedAttribute', () => { object_id: 678, // equivalent: block.attributes.id type: 'post_type', // // equivalent: block.attributes.kind url: 'https://localhost:8889/page-example/', - xfn: [ '' ], - target: '', }, } ); } ); } ); describe( 'Mapping block attributes and menu item fields', () => { - const blocks = [ + const blocksToMenuItems = [ + { + block: { + attributes: { + label: 'Example Page', + url: '/example-page/', + description: 'Lorem ipsum dolor sit amet.', + rel: 'friend met', + className: 'my-custom-class-one my-custom-class-two', + title: 'Example page link title attribute', + id: 100, + type: 'page', + kind: 'post-type', + opensInNewTab: true, + }, + clientId: 'navigation-link-block-client-id-1', + innerBlocks: [], + isValid: true, + name: 'core/navigation-link', + }, + menuItem: { + title: { + raw: 'Example Page', + rendered: 'Example Page', + }, + url: '/example-page/', + description: 'Lorem ipsum dolor sit amet.', + xfn: [ 'friend', 'met' ], + classes: [ 'my-custom-class-one', 'my-custom-class-two' ], + attr_title: 'Example page link title attribute', + object_id: 100, + object: 'page', + type: 'post_type', + target: '_blank', + }, + }, { - attributes: { + block: { + attributes: { + label: 'Example Post', + url: '/example-post/', + description: 'Consectetur adipiscing elit.', + rel: 'friend', + className: 'my-custom-class-one', + title: 'Example post link title attribute', + id: 101, + type: 'post', + kind: 'post-type', + opensInNewTab: false, + }, + clientId: 'navigation-link-block-client-id-2', + innerBlocks: [], + isValid: true, + name: 'core/navigation-link', + }, + menuItem: { + title: { + raw: 'Example Post', + rendered: 'Example Post', + }, + url: '/example-post/', + description: 'Consectetur adipiscing elit.', + xfn: [ 'friend' ], + classes: [ 'my-custom-class-one' ], + attr_title: 'Example post link title attribute', + object_id: 101, + object: 'post', + type: 'post_type', + }, + }, + { + block: { + attributes: { + label: 'Example Category', + url: '/example-category/', + description: + 'Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.', + rel: '', + className: '', + title: '', + id: 102, + type: 'category', + kind: 'taxonomy', + opensInNewTab: true, + }, + clientId: 'navigation-link-block-client-id-3', + innerBlocks: [], + isValid: true, + name: 'core/navigation-link', + }, + menuItem: { + title: { + raw: 'Example Category', + rendered: 'Example Category', + }, + url: '/example-category/', + description: + 'Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.', + object_id: 102, + object: 'category', + type: 'taxonomy', + target: '_blank', + }, + }, + { + block: { + attributes: { + label: 'Example Tag', + url: '/example-tag/', + description: '', + rel: '', + className: '', + title: '', + id: 103, + type: 'tag', + kind: 'taxonomy', + opensInNewTab: false, + }, + clientId: 'navigation-link-block-client-id-4', + innerBlocks: [], + isValid: true, + name: 'core/navigation-link', + }, + menuItem: { + title: { + raw: 'Example Tag', + rendered: 'Example Tag', + }, + url: '/example-tag/', + object_id: 103, + object: 'tag', + type: 'taxonomy', + }, + }, + { + block: { + attributes: { + label: 'Example Custom Link', + url: 'https://wordpress.org', + description: '', + rel: '', + className: '', + title: '', + type: 'custom', + kind: 'custom', + opensInNewTab: true, + }, + clientId: 'navigation-link-block-client-id-5', + innerBlocks: [], + isValid: true, + name: 'core/navigation-link', + }, + menuItem: { + title: { + raw: 'Example Custom Link', + rendered: 'Example Custom Link', + }, + url: 'https://wordpress.org', + object: 'custom', + type: 'custom', + target: '_blank', + }, + }, + ]; + + const menuItemsToBlockAttrs = [ + { + menuItem: { + title: { + raw: 'Example Page', + rendered: 'Example Page', + }, + url: '/example-page/', + description: 'Lorem ipsum dolor sit amet.', + xfn: [ 'friend', 'met' ], + classes: [ 'my-custom-class-one', 'my-custom-class-two' ], + attr_title: 'Example page link title attribute', + object_id: 100, + object: 'page', + type: 'post_type', + target: '_blank', + }, + blockAttrs: { label: 'Example Page', url: '/example-page/', description: 'Lorem ipsum dolor sit amet.', @@ -391,13 +567,24 @@ describe( 'Mapping block attributes and menu item fields', () => { kind: 'post-type', opensInNewTab: true, }, - clientId: 'navigation-link-block-client-id-1', - innerBlocks: [], - isValid: true, - name: 'core/navigation-link', }, { - attributes: { + menuItem: { + title: { + raw: 'Example Post', + rendered: 'Example Post', + }, + url: '/example-post/', + description: 'Consectetur adipiscing elit.', + xfn: [ 'friend' ], + classes: [ 'my-custom-class-one' ], + attr_title: 'Example post link title attribute', + object_id: 101, + object: 'post', + type: 'post_type', + target: '', + }, + blockAttrs: { label: 'Example Post', url: '/example-post/', description: 'Consectetur adipiscing elit.', @@ -407,156 +594,99 @@ describe( 'Mapping block attributes and menu item fields', () => { id: 101, type: 'post', kind: 'post-type', - opensInNewTab: false, }, - clientId: 'navigation-link-block-client-id-2', - innerBlocks: [], - isValid: true, - name: 'core/navigation-link', }, { - attributes: { + menuItem: { + title: { + raw: 'Example Category', + rendered: 'Example Category', + }, + url: '/example-category/', + description: + 'Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.', + xfn: [ ' ', ' ' ], + classes: [ ' ', ' ' ], + attr_title: '', + object_id: 102, + object: 'category', + type: 'taxonomy', + target: '_blank', + }, + blockAttrs: { label: 'Example Category', url: '/example-category/', description: 'Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.', - rel: '', - className: '', - title: '', id: 102, type: 'category', kind: 'taxonomy', opensInNewTab: true, }, - clientId: 'navigation-link-block-client-id-3', - innerBlocks: [], - isValid: true, - name: 'core/navigation-link', }, { - attributes: { - label: 'Example Tag', + menuItem: { + title: { + raw: 'Example Tag', + rendered: 'Example Tag', + }, url: '/example-tag/', description: '', - rel: '', - className: '', - title: '', + xfn: [ '' ], + classes: [ '' ], + attr_title: '', + object_id: 103, + object: 'tag', + type: 'taxonomy', + target: '', + }, + blockAttrs: { + label: 'Example Tag', + url: '/example-tag/', id: 103, type: 'tag', kind: 'taxonomy', - opensInNewTab: false, }, - clientId: 'navigation-link-block-client-id-4', - innerBlocks: [], - isValid: true, - name: 'core/navigation-link', }, { - attributes: { - label: 'Example Custom Link', + menuItem: { + title: { + raw: 'Example Custom Link', + rendered: 'Example Custom Link', + }, url: 'https://wordpress.org', description: '', - rel: '', - className: '', - title: '', + xfn: [ '' ], + classes: [ '' ], + attr_title: '', + object: 'custom', + type: 'custom', + target: '_blank', + }, + blockAttrs: { + label: 'Example Custom Link', + url: 'https://wordpress.org', type: 'custom', kind: 'custom', opensInNewTab: true, }, - clientId: 'navigation-link-block-client-id-5', - innerBlocks: [], - isValid: true, - name: 'core/navigation-link', }, ]; - const menuItems = [ - { - title: { - raw: 'Example Page', - rendered: 'Example Page', - }, - url: '/example-page/', - description: 'Lorem ipsum dolor sit amet.', - xfn: [ 'friend', 'met' ], - classes: [ 'my-custom-class-one', 'my-custom-class-two' ], - attr_title: 'Example page link title attribute', - object_id: 100, - object: 'page', - type: 'post_type', - target: '_blank', - }, - { - title: { - raw: 'Example Post', - rendered: 'Example Post', - }, - url: '/example-post/', - description: 'Consectetur adipiscing elit.', - xfn: [ 'friend' ], - classes: [ 'my-custom-class-one' ], - attr_title: 'Example post link title attribute', - object_id: 101, - object: 'post', - type: 'post_type', - target: '', - }, - { - title: { - raw: 'Example Category', - rendered: 'Example Category', - }, - url: '/example-category/', - description: - 'Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.', - xfn: [ '' ], - classes: [ '' ], - attr_title: '', - object_id: 102, - object: 'category', - type: 'taxonomy', - target: '_blank', - }, - { - title: { - raw: 'Example Tag', - rendered: 'Example Tag', - }, - url: '/example-tag/', - description: '', - xfn: [ '' ], - classes: [ '' ], - attr_title: '', - object_id: 103, - object: 'tag', - type: 'taxonomy', - target: '', - }, - { - title: { - raw: 'Example Custom Link', - rendered: 'Example Custom Link', - }, - url: 'https://wordpress.org', - description: '', - xfn: [ '' ], - classes: [ '' ], - attr_title: '', - object: 'custom', - type: 'custom', - target: '_blank', - }, - ]; - - const blockAttrs = blocks.map( ( block ) => block.attributes ); - describe( 'mapping block attributes to menu item fields', () => { it( 'maps block attributes to equivalent menu item fields', () => { - const actual = blockAttrs.map( ( attrs ) => - blockAttributesToMenuItem( attrs ) + const [ actual, expected ] = blocksToMenuItems.reduce( + ( acc, item ) => { + acc[ 0 ].push( + blockAttributesToMenuItem( item.block.attributes ) + ); + acc[ 1 ].push( item.menuItem ); + return acc; + }, + [ [], [] ] ); - expect( actual ).toEqual( menuItems ); + expect( actual ).toEqual( expected ); } ); it( 'does not map block attribute "id" to menu item "object_id" field for custom (non-entity) links', () => { @@ -577,23 +707,19 @@ describe( 'Mapping block attributes and menu item fields', () => { customLinkBlockAttributes ); - // Check the actual conversion to menuItem happened successfully. + // Check the basic conversion to menuItem happened successfully. expect( actual ).toEqual( { title: { raw: 'Example Custom Link', rendered: 'Example Custom Link', }, url: 'https://wordpress.org', - description: '', - xfn: [ '' ], - classes: [ '' ], - attr_title: '', object: 'custom', type: 'custom', target: '_blank', } ); - // Assert `id` attr has not been converted to a `object_id` field for a "custom" type even if present. + // Assert `id` attr has NOT been converted to a `object_id` field for a "custom" type even if present. expect( actual.object_id ).toBeUndefined(); } ); @@ -646,11 +772,16 @@ describe( 'Mapping block attributes and menu item fields', () => { describe( 'mapping menu item fields to block attributes', () => { it( 'maps menu item fields to equivalent block attributes', () => { - const actual = menuItems.map( ( fields ) => - menuItemToBlockAttributes( fields ) + const [ actual, expected ] = menuItemsToBlockAttrs.reduce( + ( acc, item ) => { + acc[ 0 ].push( menuItemToBlockAttributes( item.menuItem ) ); + acc[ 1 ].push( item.blockAttrs ); + return acc; + }, + [ [], [] ] ); - expect( actual ).toEqual( blockAttrs ); + expect( actual ).toEqual( expected ); } ); it( 'does not map menu item "object_id" field to block attribute "id" for custom (non-entity) links', () => { @@ -714,21 +845,21 @@ describe( 'Mapping block attributes and menu item fields', () => { } ); } ); - describe( 'full conversion cycles', () => { - it( 'converts menu item fields to block attributes and back again without data loss', () => { - const actual = menuItems - .map( ( fields ) => menuItemToBlockAttributes( fields ) ) - .map( ( attrs ) => blockAttributesToMenuItem( attrs ) ); + // describe( 'full conversion cycles', () => { + // it( 'converts menu item fields to block attributes and back again without data loss', () => { + // const actual = menuItemsToBlockAttrs + // .map( ( fields ) => menuItemToBlockAttributes( fields ) ) + // .map( ( attrs ) => blockAttributesToMenuItem( attrs ) ); - expect( actual ).toEqual( menuItems ); - } ); + // expect( actual ).toEqual( menuItemsToBlockAttrs ); + // } ); - it( 'converts block attributes to menu item fields and back again without data loss', () => { - const actual = blockAttrs - .map( ( attrs ) => blockAttributesToMenuItem( attrs ) ) - .map( ( fields ) => menuItemToBlockAttributes( fields ) ); + // it( 'converts block attributes to menu item fields and back again without data loss', () => { + // const actual = blockAttrs + // .map( ( attrs ) => blockAttributesToMenuItem( attrs ) ) + // .map( ( fields ) => menuItemToBlockAttributes( fields ) ); - expect( actual ).toEqual( blockAttrs ); - } ); - } ); + // expect( actual ).toEqual( blockAttrs ); + // } ); + // } ); } ); diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index da42ce8b10769c..88a48a12ea8ebe 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -267,12 +267,14 @@ export const menuItemToBlockAttributes = ( { type: object || 'custom', kind: menuItemTypeField?.replace( '_', '-' ) || 'custom', url: url || '', - ...( xfn?.length && { - rel: xfn.join( ' ' ).trim(), - } ), - ...( classes?.length && { - className: classes.join( ' ' ).trim(), - } ), + ...( xfn?.length && + xfn.join( ' ' ).trim() && { + rel: xfn.join( ' ' ).trim(), + } ), + ...( classes?.length && + classes.join( ' ' ).trim() && { + className: classes.join( ' ' ).trim(), + } ), ...( attr_title?.length && { title: attr_title, } ), From b4dc56495b390c95c400405e00538a658bd4761b Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 30 Apr 2021 11:13:06 +0100 Subject: [PATCH 40/48] Delete unused tests --- .../edit-navigation/src/store/test/utils.js | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/packages/edit-navigation/src/store/test/utils.js b/packages/edit-navigation/src/store/test/utils.js index 35b9c65b365b79..57132c261050c1 100644 --- a/packages/edit-navigation/src/store/test/utils.js +++ b/packages/edit-navigation/src/store/test/utils.js @@ -844,22 +844,4 @@ describe( 'Mapping block attributes and menu item fields', () => { expect( Object.values( actual ) ).not.toContain( undefined ); } ); } ); - - // describe( 'full conversion cycles', () => { - // it( 'converts menu item fields to block attributes and back again without data loss', () => { - // const actual = menuItemsToBlockAttrs - // .map( ( fields ) => menuItemToBlockAttributes( fields ) ) - // .map( ( attrs ) => blockAttributesToMenuItem( attrs ) ); - - // expect( actual ).toEqual( menuItemsToBlockAttrs ); - // } ); - - // it( 'converts block attributes to menu item fields and back again without data loss', () => { - // const actual = blockAttrs - // .map( ( attrs ) => blockAttributesToMenuItem( attrs ) ) - // .map( ( fields ) => menuItemToBlockAttributes( fields ) ); - - // expect( actual ).toEqual( blockAttrs ); - // } ); - // } ); } ); From 7080f0d2bd081b03cf60ee49a5deed1c0dc074a8 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 30 Apr 2021 11:13:42 +0100 Subject: [PATCH 41/48] Code documentation for generator test steps --- packages/edit-navigation/src/store/test/resolvers.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/edit-navigation/src/store/test/resolvers.js b/packages/edit-navigation/src/store/test/resolvers.js index d32e5d207cef0f..fdbe8eddc97e5d 100644 --- a/packages/edit-navigation/src/store/test/resolvers.js +++ b/packages/edit-navigation/src/store/test/resolvers.js @@ -229,9 +229,9 @@ describe( 'getNavigationPostForMenu', () => { const generator = getNavigationPostForMenu( menuId ); // Advance generator - generator.next(); - generator.next(); - generator.next(); + generator.next(); // Gen step: yield persistPost + generator.next(); // Gen step: yield dispatch "getEntityRecord" + generator.next(); // Gen step: yield resolveMenuItems const menuItems = [ { @@ -286,9 +286,12 @@ describe( 'getNavigationPostForMenu', () => { }, ]; - // Feed a known value to the generator yield result + // // Gen step: yield 'SET_MENU_ITEM_TO_CLIENT_ID_MAPPING', + // By feeding `menuItems` to the generator this will overload the **result** of + // the call to yield resolveMenuItems( menuId ); generator.next( menuItems ); + // Gen step: yield persistPost const persistPostAction = generator.next().value; // Get the core/navigation-link blocks from the generated core/navigation block innerBlocks. From b3c1b1450430ac690b29363609d0a3badb6b5d19 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 30 Apr 2021 11:16:42 +0100 Subject: [PATCH 42/48] Update snapshot to remove omitted attributes --- .../navigation-editor.test.js.snap | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/e2e-tests/specs/experiments/__snapshots__/navigation-editor.test.js.snap b/packages/e2e-tests/specs/experiments/__snapshots__/navigation-editor.test.js.snap index 2978d5d63f9ffe..e9c360213219d0 100644 --- a/packages/e2e-tests/specs/experiments/__snapshots__/navigation-editor.test.js.snap +++ b/packages/e2e-tests/specs/experiments/__snapshots__/navigation-editor.test.js.snap @@ -10,36 +10,36 @@ exports[`Navigation editor allows creation of a menu when there are no current m exports[`Navigation editor displays the first menu from the REST response when at least one menu exists 1`] = ` " - + - - + + - - - - - + + + + + - + - + - - - - + + + + - - + + " `; From ca250c46a942c2160c7a186d91691e46830669fe Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 30 Apr 2021 11:25:01 +0100 Subject: [PATCH 43/48] =?UTF-8?q?Update=20block=E2=80=99s=20mapping=20code?= =?UTF-8?q?=20and=20update=20associated=20e2e=20snapshot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../navigation/map-menu-items-to-blocks.js | 34 +++++++++++++------ .../__snapshots__/navigation.test.js.snap | 32 ++++++++--------- 2 files changed, 40 insertions(+), 26 deletions(-) diff --git a/packages/block-library/src/navigation/map-menu-items-to-blocks.js b/packages/block-library/src/navigation/map-menu-items-to-blocks.js index d3e0b88cbd1bb8..e07430a3402f46 100644 --- a/packages/block-library/src/navigation/map-menu-items-to-blocks.js +++ b/packages/block-library/src/navigation/map-menu-items-to-blocks.js @@ -37,18 +37,32 @@ export const menuItemToBlockAttributes = ( { target, } ) => { return { - label: menuItemTitleField.rendered, - rel: xfn.join( ' ' ).trim(), - className: classes.join( ' ' ).trim(), - title: attr_title, + label: menuItemTitleField?.rendered || '', type: object || 'custom', - ...( 'custom' !== object && { - id: object_id, + kind: menuItemTypeField?.replace( '_', '-' ) || 'custom', + url: url || '', + ...( xfn?.length && + xfn.join( ' ' ).trim() && { + rel: xfn.join( ' ' ).trim(), + } ), + ...( classes?.length && + classes.join( ' ' ).trim() && { + className: classes.join( ' ' ).trim(), + } ), + ...( attr_title?.length && { + title: attr_title, + } ), + // eslint-disable-next-line camelcase + ...( object_id && + 'custom' !== object && { + id: object_id, + } ), + ...( description?.length && { + description, + } ), + ...( target === '_blank' && { + opensInNewTab: true, } ), - description, - url, - kind: menuItemTypeField?.replace( '_', '-' ) ?? 'custom', - opensInNewTab: target === '_blank', }; }; diff --git a/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap b/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap index 72771912dea747..52204753521c4a 100644 --- a/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap +++ b/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap @@ -2,36 +2,36 @@ exports[`Navigation Creating from existing Menus allows a navigation block to be created from existing menus 1`] = ` " - + - - + + - - - - - + + + + + - + - + - - - - + + + + - - + + " `; From 6a62baba9248f52f59b93247a07cbf378be856c8 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 30 Apr 2021 12:44:52 +0100 Subject: [PATCH 44/48] Fix bug with custom block label not being persisted correctly to nav menu item --- .../edit-navigation/src/store/test/utils.js | 65 ++++--------------- packages/edit-navigation/src/store/utils.js | 6 +- 2 files changed, 14 insertions(+), 57 deletions(-) diff --git a/packages/edit-navigation/src/store/test/utils.js b/packages/edit-navigation/src/store/test/utils.js index 57132c261050c1..084a39c7d74ff4 100644 --- a/packages/edit-navigation/src/store/test/utils.js +++ b/packages/edit-navigation/src/store/test/utils.js @@ -266,10 +266,7 @@ describe( 'computeCustomizedAttribute', () => { const menuItemsByClientId = { 'navigation-link-block-client-id-1': { id: 100, - title: { - raw: 'wp.com', - rendered: 'wp.com', - }, + title: 'wp.com', url: 'http://wp.com', menu_order: 1, menus: [ 1 ], @@ -278,10 +275,7 @@ describe( 'computeCustomizedAttribute', () => { }, 'navigation-link-block-client-id-2': { id: 101, - title: { - raw: 'wp.org', - rendered: 'wp.org', - }, + title: 'wp.org', url: 'http://wp.org', menu_order: 2, menus: [ 1 ], @@ -290,10 +284,7 @@ describe( 'computeCustomizedAttribute', () => { }, 'navigation-link-block-client-id-3': { id: 102, - title: { - raw: 'Page Example', - rendered: 'Page Example', - }, + title: 'Page Example', url: 'https://wordpress.org', menu_order: 3, menus: [ 1 ], @@ -324,10 +315,7 @@ describe( 'computeCustomizedAttribute', () => { object: 'custom', position: 1, status: 'publish', - title: { - raw: 'wp.org', - rendered: 'wp.org', - }, + title: 'wp.org', url: 'http://wp.org', xfn: [ 'external' ], type: 'custom', @@ -341,10 +329,7 @@ describe( 'computeCustomizedAttribute', () => { original_title: '', position: 2, status: 'publish', - title: { - raw: 'wp.com', - rendered: 'wp.com', - }, + title: 'wp.com', object: 'custom', url: 'http://wp.com', target: '_blank', @@ -359,10 +344,7 @@ describe( 'computeCustomizedAttribute', () => { original_title: '', position: 3, status: 'publish', - title: { - raw: 'Page Example', - rendered: 'Page Example', - }, + title: 'Page Example', object: 'page', // equivalent: block.attributes.type object_id: 678, // equivalent: block.attributes.id type: 'post_type', // // equivalent: block.attributes.kind @@ -394,10 +376,7 @@ describe( 'Mapping block attributes and menu item fields', () => { name: 'core/navigation-link', }, menuItem: { - title: { - raw: 'Example Page', - rendered: 'Example Page', - }, + title: 'Example Page', url: '/example-page/', description: 'Lorem ipsum dolor sit amet.', xfn: [ 'friend', 'met' ], @@ -429,10 +408,7 @@ describe( 'Mapping block attributes and menu item fields', () => { name: 'core/navigation-link', }, menuItem: { - title: { - raw: 'Example Post', - rendered: 'Example Post', - }, + title: 'Example Post', url: '/example-post/', description: 'Consectetur adipiscing elit.', xfn: [ 'friend' ], @@ -464,10 +440,7 @@ describe( 'Mapping block attributes and menu item fields', () => { name: 'core/navigation-link', }, menuItem: { - title: { - raw: 'Example Category', - rendered: 'Example Category', - }, + title: 'Example Category', url: '/example-category/', description: 'Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.', @@ -497,10 +470,7 @@ describe( 'Mapping block attributes and menu item fields', () => { name: 'core/navigation-link', }, menuItem: { - title: { - raw: 'Example Tag', - rendered: 'Example Tag', - }, + title: 'Example Tag', url: '/example-tag/', object_id: 103, object: 'tag', @@ -526,10 +496,7 @@ describe( 'Mapping block attributes and menu item fields', () => { name: 'core/navigation-link', }, menuItem: { - title: { - raw: 'Example Custom Link', - rendered: 'Example Custom Link', - }, + title: 'Example Custom Link', url: 'https://wordpress.org', object: 'custom', type: 'custom', @@ -709,10 +676,7 @@ describe( 'Mapping block attributes and menu item fields', () => { // Check the basic conversion to menuItem happened successfully. expect( actual ).toEqual( { - title: { - raw: 'Example Custom Link', - rendered: 'Example Custom Link', - }, + title: 'Example Custom Link', url: 'https://wordpress.org', object: 'custom', type: 'custom', @@ -744,10 +708,7 @@ describe( 'Mapping block attributes and menu item fields', () => { // Defaults are taken from https://core.trac.wordpress.org/browser/tags/5.7.1/src/wp-includes/nav-menu.php#L438. expect( actual ).toEqual( expect.objectContaining( { - title: { - raw: '', - rendered: '', - }, + title: '', url: '', } ) ); diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index 88a48a12ea8ebe..3e6a21a59b42d9 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -117,7 +117,6 @@ export function computeCustomizedAttribute( dataObject[ key ] = false; } } - return JSON.stringify( dataObject ); function blocksTreeToFlatList( innerBlocks, parentId = 0 ) { @@ -195,10 +194,7 @@ export const blockAttributesToMenuItem = ( { opensInNewTab, } ) => { return { - title: { - rendered: label, - raw: label, - }, + title: label, url, ...( description?.length && { description, From 7497fa6a524b4c366bea540b0dd619236dc89782 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 30 Apr 2021 13:17:38 +0100 Subject: [PATCH 45/48] =?UTF-8?q?Fix=20mapping=20of=20=E2=80=9Ctag?= =?UTF-8?q?=E2=80=9D=20to=20=E2=80=9Cpost=5Ftag=E2=80=9D=20and=20add=20tes?= =?UTF-8?q?t=20coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../edit-navigation/src/store/test/utils.js | 30 ++++++++++++++++++- packages/edit-navigation/src/store/utils.js | 16 ++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/packages/edit-navigation/src/store/test/utils.js b/packages/edit-navigation/src/store/test/utils.js index 084a39c7d74ff4..e9d0cfedcc6206 100644 --- a/packages/edit-navigation/src/store/test/utils.js +++ b/packages/edit-navigation/src/store/test/utils.js @@ -473,7 +473,7 @@ describe( 'Mapping block attributes and menu item fields', () => { title: 'Example Tag', url: '/example-tag/', object_id: 103, - object: 'tag', + object: 'post_tag', type: 'taxonomy', }, }, @@ -687,6 +687,20 @@ describe( 'Mapping block attributes and menu item fields', () => { expect( actual.object_id ).toBeUndefined(); } ); + it( 'correctly maps "tag" block type variation to "post_tag" value as expected in "object" type field', () => { + const tagLinkBlockVariation = { + id: 12345, // added for test purposes only - should't exist. + type: 'tag', // custom type indicates we shouldn't need an `id` field. + kind: 'taxonomy', // custom type indicates we shouldn't need an `id` field. + label: 'Example Tag', + url: '/example-tag/', + }; + + const actual = blockAttributesToMenuItem( tagLinkBlockVariation ); + + expect( actual.object ).toBe( 'post_tag' ); + } ); + it( 'gracefully handles undefined values by falling back to menu item defaults', () => { const blockAttrsWithUndefinedValues = { id: undefined, @@ -763,6 +777,20 @@ describe( 'Mapping block attributes and menu item fields', () => { expect( actual.id ).toBeUndefined(); } ); + it( 'correctly maps "post_tag" menu item object type to "tag" block type variation', () => { + const tagMenuItem = { + title: 'Example Tag', + url: '/example-tag/', + object_id: 123456, + object: 'post_tag', + type: 'taxonomy', + }; + + const actual = menuItemToBlockAttributes( tagMenuItem ); + + expect( actual.type ).toBe( 'tag' ); + } ); + it( 'gracefully handles undefined values by falling back to block attribute defaults', () => { // Note that whilst Core provides default values for nav_menu_item's it is possible that these // values could be manipulated via Plugins. As such we must account for unexpected values. diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index 3e6a21a59b42d9..295f1d956fa1af 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -193,6 +193,14 @@ export const blockAttributesToMenuItem = ( { kind, opensInNewTab, } ) => { + // For historical reasons, the `core/navigation-link` variation type is `tag` + // whereas WP Core expects `post_tag` as the `object` type. + // To avoid writing a block migration we perform a conversion here. + // See also inverse equivalent in `menuItemToBlockAttributes`. + if ( type && type === 'tag' ) { + type = 'post_tag'; + } + return { title: label, url, @@ -258,6 +266,14 @@ export const menuItemToBlockAttributes = ( { type: menuItemTypeField, target, } ) => { + // For historical reasons, the `core/navigation-link` variation type is `tag` + // whereas WP Core expects `post_tag` as the `object` type. + // To avoid writing a block migration we perform a conversion here. + // See also inverse equivalent in `blockAttributesToMenuItem`. + if ( object && object === 'post_tag' ) { + object = 'tag'; + } + return { label: menuItemTitleField?.rendered || '', type: object || 'custom', From 2893305b8f80fbc1c1714056d30336641e1491a0 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 4 May 2021 09:31:02 +0100 Subject: [PATCH 46/48] Add typedefs for nav_menu_item --- .../navigation/map-menu-items-to-blocks.js | 36 +++++++++++-------- packages/edit-navigation/src/store/utils.js | 36 +++++++++++-------- 2 files changed, 44 insertions(+), 28 deletions(-) diff --git a/packages/block-library/src/navigation/map-menu-items-to-blocks.js b/packages/block-library/src/navigation/map-menu-items-to-blocks.js index e07430a3402f46..298761ac0c6260 100644 --- a/packages/block-library/src/navigation/map-menu-items-to-blocks.js +++ b/packages/block-library/src/navigation/map-menu-items-to-blocks.js @@ -4,23 +4,31 @@ import { createBlock, parse } from '@wordpress/blocks'; /** - * Convert block attributes to menu item. - * + * A WP nav_menu_item object. * For more documentation on the individual fields present on a menu item please see: * https://core.trac.wordpress.org/browser/tags/5.7.1/src/wp-includes/nav-menu.php#L789 * - * @param {Object} menuItem the menu item to be converted to block attributes. - * @param {Object} menuItem.title stores the raw and rendered versions of the title/label for this menu item. - * @param {Array} menuItem.xfn the XFN relationships expressed in the link of this menu item. - * @param {Array} menuItem.classes the HTML class attributes for this menu item. - * @param {string} menuItem.attr_title the HTML title attribute for this menu item. - * @param {string} menuItem.object The type of object originally represented, such as 'category', 'post', or 'attachment'. - * @param {string} menuItem.object_id The DB ID of the original object this menu item represents, e.g. ID for posts and term_id for categories. - * @param {string} menuItem.description The description of this menu item. - * @param {string} menuItem.url The URL to which this menu item points. - * @param {string} menuItem.type The family of objects originally represented, such as 'post_type' or 'taxonomy'. - * @param {string} menuItem.target The target attribute of the link element for this menu item. - * @return {Object} the block attributes converted from the menu item. + * Changes made here should also be mirrored in packages/edit-navigation/src/store/utils.js. + * + * @typedef WPNavMenuItem + * + * @property {Object} title stores the raw and rendered versions of the title/label for this menu item. + * @property {Array} xfn the XFN relationships expressed in the link of this menu item. + * @property {Array} classes the HTML class attributes for this menu item. + * @property {string} attr_title the HTML title attribute for this menu item. + * @property {string} object The type of object originally represented, such as 'category', 'post', or 'attachment'. + * @property {string} object_id The DB ID of the original object this menu item represents, e.g. ID for posts and term_id for categories. + * @property {string} description The description of this menu item. + * @property {string} url The URL to which this menu item points. + * @property {string} type The family of objects originally represented, such as 'post_type' or 'taxonomy'. + * @property {string} target The target attribute of the link element for this menu item. + */ + +/** + * Convert block attributes to menu item. + * + * @param {WPNavMenuItem} menuItem the menu item to be converted to block attributes. + * @return {Object} the block attributes converted from the WPNavMenuItem item. */ export const menuItemToBlockAttributes = ( { title: menuItemTitleField, diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index 295f1d956fa1af..d7533e3b2d06eb 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -19,6 +19,27 @@ import { import { NEW_TAB_TARGET_ATTRIBUTE } from '../constants'; +/** + * A WP nav_menu_item object. + * For more documentation on the individual fields present on a menu item please see: + * https://core.trac.wordpress.org/browser/tags/5.7.1/src/wp-includes/nav-menu.php#L789 + * + * Changes made here should also be mirrored in packages/block-library/src/navigation/map-menu-items-to-blocks.js. + * + * @typedef WPNavMenuItem + * + * @property {Object} title stores the raw and rendered versions of the title/label for this menu item. + * @property {Array} xfn the XFN relationships expressed in the link of this menu item. + * @property {Array} classes the HTML class attributes for this menu item. + * @property {string} attr_title the HTML title attribute for this menu item. + * @property {string} object The type of object originally represented, such as 'category', 'post', or 'attachment'. + * @property {string} object_id The DB ID of the original object this menu item represents, e.g. ID for posts and term_id for categories. + * @property {string} description The description of this menu item. + * @property {string} url The URL to which this menu item points. + * @property {string} type The family of objects originally represented, such as 'post_type' or 'taxonomy'. + * @property {string} target The target attribute of the link element for this menu item. + */ + /** * Builds an ID for a new navigation post. * @@ -236,20 +257,7 @@ export const blockAttributesToMenuItem = ( { /** * Convert block attributes to menu item. * - * For more documentation on the individual fields present on a menu item please see: - * https://core.trac.wordpress.org/browser/tags/5.7.1/src/wp-includes/nav-menu.php#L789 - * - * @param {Object} menuItem the menu item to be converted to block attributes. - * @param {Object} menuItem.title stores the raw and rendered versions of the title/label for this menu item. - * @param {Array} menuItem.xfn the XFN relationships expressed in the link of this menu item. - * @param {Array} menuItem.classes the HTML class attributes for this menu item. - * @param {string} menuItem.attr_title the HTML title attribute for this menu item. - * @param {string} menuItem.object The type of object originally represented, such as 'category', 'post', or 'attachment'. - * @param {string} menuItem.object_id The DB ID of the original object this menu item represents, e.g. ID for posts and term_id for categories. - * @param {string} menuItem.description The description of this menu item. - * @param {string} menuItem.url The URL to which this menu item points. - * @param {string} menuItem.type The family of objects originally represented, such as 'post_type' or 'taxonomy'. - * @param {string} menuItem.target The target attribute of the link element for this menu item. + * @param {WPNavMenuItem} menuItem the menu item to be converted to block attributes. * @return {Object} the block attributes converted from the menu item. */ export const menuItemToBlockAttributes = ( { From 4ec0e71b14a195dfc1f09e46c14028fc3b43c85e Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 4 May 2021 09:53:19 +0100 Subject: [PATCH 47/48] Allow for unsetting of target attribute --- .../edit-navigation/src/store/test/utils.js | 37 +++++++++++++++++++ packages/edit-navigation/src/store/utils.js | 4 +- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/packages/edit-navigation/src/store/test/utils.js b/packages/edit-navigation/src/store/test/utils.js index e9d0cfedcc6206..180b2b45f20f35 100644 --- a/packages/edit-navigation/src/store/test/utils.js +++ b/packages/edit-navigation/src/store/test/utils.js @@ -319,6 +319,7 @@ describe( 'computeCustomizedAttribute', () => { url: 'http://wp.org', xfn: [ 'external' ], type: 'custom', + target: '', }, 'nav_menu_item[101]': { _invalid: false, @@ -349,6 +350,7 @@ describe( 'computeCustomizedAttribute', () => { object_id: 678, // equivalent: block.attributes.id type: 'post_type', // // equivalent: block.attributes.kind url: 'https://localhost:8889/page-example/', + target: '', }, } ); } ); @@ -417,6 +419,7 @@ describe( 'Mapping block attributes and menu item fields', () => { object_id: 101, object: 'post', type: 'post_type', + target: '', }, }, { @@ -475,6 +478,7 @@ describe( 'Mapping block attributes and menu item fields', () => { object_id: 103, object: 'post_tag', type: 'taxonomy', + target: '', }, }, { @@ -743,6 +747,39 @@ describe( 'Mapping block attributes and menu item fields', () => { expect( Object.values( actual ) ).not.toContain( undefined ); } ); + + it( 'allows for setting and unsetting of target property based on opensInNewTab arttribute boolean', () => { + const shared = { + id: 12345, // added for test purposes only - should't exist. + type: 'custom', // custom type indicates we shouldn't need an `id` field. + kind: 'custom', // custom type indicates we shouldn't need an `id` field. + label: 'Example', + url: '/example/', + }; + + const openInNewTabBlock = { + ...shared, + opensInNewTab: true, + }; + + const doNotOpenInNewTabBlock = { + ...shared, + opensInNewTab: false, + }; + + const shouldOpenInNewTab = blockAttributesToMenuItem( + openInNewTabBlock + ); + + const shouldNotOpenInNewTab = blockAttributesToMenuItem( + doNotOpenInNewTabBlock + ); + + expect( shouldOpenInNewTab.target ).toBe( '_blank' ); + + // Should also allow unsetting of an existing value. + expect( shouldNotOpenInNewTab.target ).toBe( '' ); + } ); } ); describe( 'mapping menu item fields to block attributes', () => { diff --git a/packages/edit-navigation/src/store/utils.js b/packages/edit-navigation/src/store/utils.js index d7533e3b2d06eb..3d26c111ca4a29 100644 --- a/packages/edit-navigation/src/store/utils.js +++ b/packages/edit-navigation/src/store/utils.js @@ -248,9 +248,7 @@ export const blockAttributesToMenuItem = ( { 'custom' !== type && { object_id: id, } ), - ...( opensInNewTab && { - target: NEW_TAB_TARGET_ATTRIBUTE, - } ), + target: opensInNewTab ? NEW_TAB_TARGET_ATTRIBUTE : '', }; }; From ef58f7a9c862e978612798665469dde6879e9752 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 4 May 2021 09:58:35 +0100 Subject: [PATCH 48/48] =?UTF-8?q?Remove=20=E2=80=9Ccustom=E2=80=9D=20defau?= =?UTF-8?q?lt=20for=20block=20variation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../block-library/src/navigation/map-menu-items-to-blocks.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/navigation/map-menu-items-to-blocks.js b/packages/block-library/src/navigation/map-menu-items-to-blocks.js index 298761ac0c6260..4b21aaa9838819 100644 --- a/packages/block-library/src/navigation/map-menu-items-to-blocks.js +++ b/packages/block-library/src/navigation/map-menu-items-to-blocks.js @@ -46,7 +46,9 @@ export const menuItemToBlockAttributes = ( { } ) => { return { label: menuItemTitleField?.rendered || '', - type: object || 'custom', + ...( object?.length && { + type: object, + } ), kind: menuItemTypeField?.replace( '_', '-' ) || 'custom', url: url || '', ...( xfn?.length &&