diff --git a/packages/block-library/src/heading/index.js b/packages/block-library/src/heading/index.js index 3752ca70bc714..c137f38210bf8 100644 --- a/packages/block-library/src/heading/index.js +++ b/packages/block-library/src/heading/index.js @@ -30,15 +30,16 @@ export const settings = { const { content, level } = attributes; const customName = attributes?.metadata?.name; + const hasContent = content?.length > 0; // In the list view, use the block's content as the label. // If the content is empty, fall back to the default label. - if ( context === 'list-view' && ( customName || content ) ) { - return attributes?.metadata?.name || content; + if ( context === 'list-view' && ( customName || hasContent ) ) { + return customName || content; } if ( context === 'accessibility' ) { - return ! content || content.length === 0 + return ! hasContent ? sprintf( /* translators: accessibility text. %s: heading level. */ __( 'Level %s. Empty.' ), diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js index e091b52c68a5c..bc772671219fe 100644 --- a/packages/block-library/src/navigation-link/edit.js +++ b/packages/block-library/src/navigation-link/edit.js @@ -333,6 +333,7 @@ export default function NavigationLinkEdit( { isKeyboardEvent.primary( event, 'k' ) || ( ( ! url || isDraft || isInvalid ) && event.keyCode === ENTER ) ) { + event.preventDefault(); setIsLinkOpen( true ); } } diff --git a/packages/block-library/src/navigation-submenu/edit.js b/packages/block-library/src/navigation-submenu/edit.js index 507ea64940c3a..401d9512797b0 100644 --- a/packages/block-library/src/navigation-submenu/edit.js +++ b/packages/block-library/src/navigation-submenu/edit.js @@ -279,6 +279,7 @@ export default function NavigationSubmenuEdit( { function onKeyDown( event ) { if ( isKeyboardEvent.primary( event, 'k' ) ) { + event.preventDefault(); setIsLinkOpen( true ); } } diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index 06a89f13933de..b5bb08cc456dc 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -342,16 +342,7 @@ function Navigation( { const [ detectedOverlayColor, setDetectedOverlayColor ] = useState(); const onSelectClassicMenu = async ( classicMenu ) => { - const navMenu = await convertClassicMenu( - classicMenu.id, - classicMenu.name, - 'draft' - ); - if ( navMenu ) { - handleUpdateMenu( navMenu.id, { - focusNavigationBlock: true, - } ); - } + return convertClassicMenu( classicMenu.id, classicMenu.name, 'draft' ); }; const onSelectNavigationMenu = ( menuId ) => { @@ -402,6 +393,9 @@ function Navigation( { showClassicMenuConversionNotice( __( 'Classic menu imported successfully.' ) ); + handleUpdateMenu( createNavigationMenuPost?.id, { + focusNavigationBlock: true, + } ); } if ( classicMenuConversionStatus === CLASSIC_MENU_CONVERSION_ERROR ) { @@ -414,6 +408,8 @@ function Navigation( { classicMenuConversionError, hideClassicMenuConversionNotice, showClassicMenuConversionNotice, + createNavigationMenuPost?.id, + handleUpdateMenu, ] ); useEffect( () => { @@ -866,50 +862,52 @@ function Navigation( { ) } - { isLoading && ( - + + { isLoading && (
-
- ) } + ) } - { ! isLoading && ( - - - - { isEntityAvailable && ( - - ) } - - - ) } + { ! isLoading && ( + <> + + + { isEntityAvailable && ( + + ) } + + + ) } +
); diff --git a/packages/block-library/src/navigation/index.php b/packages/block-library/src/navigation/index.php index f28f017f0238c..ae00eabc27f31 100644 --- a/packages/block-library/src/navigation/index.php +++ b/packages/block-library/src/navigation/index.php @@ -135,9 +135,9 @@ private static function get_markup_for_inner_block( $inner_block ) { if ( static::does_block_need_a_list_item_wrapper( $inner_block ) ) { return '
  • ' . $inner_block_content . '
  • '; } - - return $inner_block_content; } + + return $inner_block_content; } /** @@ -1464,6 +1464,14 @@ function block_core_navigation_set_ignored_hooked_blocks_metadata( $inner_blocks * @return stdClass The updated post object. */ function block_core_navigation_update_ignore_hooked_blocks_meta( $post ) { + /* + * In this scenario the user has likely tried to create a navigation via the REST API. + * In which case we won't have a post ID to work with and store meta against. + */ + if ( empty( $post->ID ) ) { + return $post; + } + /* * We run the Block Hooks mechanism to inject the `metadata.ignoredHookedBlocks` attribute into * all anchor blocks. For the root level, we create a mock Navigation and extract them from there. diff --git a/packages/e2e-tests/plugins/interactive-blocks/deferred-store/block.json b/packages/e2e-tests/plugins/interactive-blocks/deferred-store/block.json new file mode 100644 index 0000000000000..088572086f000 --- /dev/null +++ b/packages/e2e-tests/plugins/interactive-blocks/deferred-store/block.json @@ -0,0 +1,15 @@ +{ + "$schema": "https://schemas.wp.org/trunk/block.json", + "apiVersion": 2, + "name": "test/deferred-store", + "title": "E2E Interactivity tests - deferred store", + "category": "text", + "icon": "heart", + "description": "", + "supports": { + "interactivity": true + }, + "textdomain": "e2e-interactivity", + "viewScriptModule": "file:./view.js", + "render": "file:./render.php" +} diff --git a/packages/e2e-tests/plugins/interactive-blocks/deferred-store/render.php b/packages/e2e-tests/plugins/interactive-blocks/deferred-store/render.php new file mode 100644 index 0000000000000..0d8c2cbeb5125 --- /dev/null +++ b/packages/e2e-tests/plugins/interactive-blocks/deferred-store/render.php @@ -0,0 +1,15 @@ + + +
    '!dlrow ,olleH' ) ); ?> +> + + +
    diff --git a/packages/e2e-tests/plugins/interactive-blocks/deferred-store/view.asset.php b/packages/e2e-tests/plugins/interactive-blocks/deferred-store/view.asset.php new file mode 100644 index 0000000000000..db23afdf657a1 --- /dev/null +++ b/packages/e2e-tests/plugins/interactive-blocks/deferred-store/view.asset.php @@ -0,0 +1 @@ + array( '@wordpress/interactivity' ) ); diff --git a/packages/e2e-tests/plugins/interactive-blocks/deferred-store/view.js b/packages/e2e-tests/plugins/interactive-blocks/deferred-store/view.js new file mode 100644 index 0000000000000..8474757a0bd8b --- /dev/null +++ b/packages/e2e-tests/plugins/interactive-blocks/deferred-store/view.js @@ -0,0 +1,20 @@ +/** + * WordPress dependencies + */ +import { store, getContext } from '@wordpress/interactivity'; + +document.addEventListener( 'DOMContentLoaded', () => { + setTimeout( () => { + store( 'test/deferred-store', { + state: { + reversedText() { + return [ ...getContext().text ].reverse().join( '' ); + }, + + get reversedTextGetter() { + return [ ...getContext().text ].reverse().join( '' ); + }, + }, + } ); + }, 50 ); +} ); diff --git a/packages/e2e-tests/plugins/interactive-blocks/store/render.php b/packages/e2e-tests/plugins/interactive-blocks/store/render.php index 295872137dbda..a15f8f0c14b97 100644 --- a/packages/e2e-tests/plugins/interactive-blocks/store/render.php +++ b/packages/e2e-tests/plugins/interactive-blocks/store/render.php @@ -1,6 +1,6 @@
    +
    0 && + sucessfullyInstalledFontFaces?.length === 0 + ) { await fetchUninstallFontFamily( installedFontFamily.id ); } @@ -411,15 +425,15 @@ function FontLibraryProvider( { children } ) { const isFaceActivated = isFontActivated( font.slug, - face.fontStyle, - face.fontWeight, + face?.fontStyle, + face?.fontWeight, font.source ); if ( isFaceActivated ) { loadFontFaceInBrowser( face, - getDisplaySrcFromFontFace( face.src ), + getDisplaySrcFromFontFace( face?.src ), 'all' ); } else { diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/utils/index.js b/packages/edit-site/src/components/global-styles/font-library-modal/utils/index.js index 07153bcd1b9c3..b3deb8fcec499 100644 --- a/packages/edit-site/src/components/global-styles/font-library-modal/utils/index.js +++ b/packages/edit-site/src/components/global-styles/font-library-modal/utils/index.js @@ -134,9 +134,9 @@ export function unloadFontFaceInBrowser( fontFace, removeFrom = 'all' ) { const unloadFontFace = ( fonts ) => { fonts.forEach( ( f ) => { if ( - f.family === formatFontFaceName( fontFace.fontFamily ) && - f.weight === fontFace.fontWeight && - f.style === fontFace.fontStyle + f.family === formatFontFaceName( fontFace?.fontFamily ) && + f.weight === fontFace?.fontWeight && + f.style === fontFace?.fontStyle ) { fonts.delete( f ); } diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/utils/preview-styles.js b/packages/edit-site/src/components/global-styles/font-library-modal/utils/preview-styles.js index 3ab6618988c0d..7e7ac3a1811b5 100644 --- a/packages/edit-site/src/components/global-styles/font-library-modal/utils/preview-styles.js +++ b/packages/edit-site/src/components/global-styles/font-library-modal/utils/preview-styles.js @@ -87,6 +87,10 @@ export function formatFontFamily( input ) { * formatFontFaceName(", 'Open Sans', 'Helvetica Neue', sans-serif") => "Open Sans" */ export function formatFontFaceName( input ) { + if ( ! input ) { + return ''; + } + let output = input.trim(); if ( output.includes( ',' ) ) { output = output diff --git a/packages/interactivity/CHANGELOG.md b/packages/interactivity/CHANGELOG.md index 1df9f0b7f1ec6..fefc154b23a0f 100644 --- a/packages/interactivity/CHANGELOG.md +++ b/packages/interactivity/CHANGELOG.md @@ -4,6 +4,8 @@ ### Bug Fixes +- Prevent non-objects from being set in store state. ([#59886](https://github.com/WordPress/gutenberg/pull/59886)) +- Ensure that stores are available for subscription before hydration. ([#59842](https://github.com/WordPress/gutenberg/pull/59842)) - Ensure scope is restored when catching exceptions thrown in async generator actions. ([#59708](https://github.com/WordPress/gutenberg/pull/59708)) ## 5.2.0 (2024-03-06) diff --git a/packages/interactivity/src/hooks.tsx b/packages/interactivity/src/hooks.tsx index 726579f50176d..64492ac3bbb59 100644 --- a/packages/interactivity/src/hooks.tsx +++ b/packages/interactivity/src/hooks.tsx @@ -15,7 +15,7 @@ import type { VNode, Context, RefObject } from 'preact'; /** * Internal dependencies */ -import { stores } from './store'; +import { store, stores, universalUnlock } from './store'; interface DirectiveEntry { value: string | Object; namespace: string; @@ -259,8 +259,14 @@ export const directive = ( // Resolve the path to some property of the store object. const resolve = ( path, namespace ) => { + let resolvedStore = stores.get( namespace ); + if ( typeof resolvedStore === 'undefined' ) { + resolvedStore = store( namespace, undefined, { + lock: universalUnlock, + } ); + } let current = { - ...stores.get( namespace ), + ...resolvedStore, context: getScope().context[ namespace ], }; path.split( '.' ).forEach( ( p ) => ( current = current[ p ] ) ); diff --git a/packages/interactivity/src/store.ts b/packages/interactivity/src/store.ts index 28600abd3c4db..5710b7bc09ea1 100644 --- a/packages/interactivity/src/store.ts +++ b/packages/interactivity/src/store.ts @@ -202,7 +202,7 @@ interface StoreOptions { lock?: boolean | string; } -const universalUnlock = +export const universalUnlock = 'I acknowledge that using a private store means my plugin will inevitably break on the next store release.'; /** @@ -274,7 +274,10 @@ export function store( if ( lock !== universalUnlock ) { storeLocks.set( namespace, lock ); } - const rawStore = { state: deepSignal( state ), ...block }; + const rawStore = { + state: deepSignal( isObject( state ) ? state : {} ), + ...block, + }; const proxiedStore = new Proxy( rawStore, handlers ); rawStores.set( namespace, rawStore ); stores.set( namespace, proxiedStore ); diff --git a/phpunit/blocks/block-navigation-block-hooks-test.php b/phpunit/blocks/block-navigation-block-hooks-test.php index e0e7bd45cfc17..ba30c2e26d25c 100644 --- a/phpunit/blocks/block-navigation-block-hooks-test.php +++ b/phpunit/blocks/block-navigation-block-hooks-test.php @@ -59,7 +59,7 @@ public function tear_down() { */ public function test_block_core_navigation_update_ignore_hooked_blocks_meta_preserves_entities() { if ( ! function_exists( 'set_ignored_hooked_blocks_metadata' ) ) { - $this->markTestSkipped( 'Test skipped on WordPress versions that do not included required Block Hooks functionalit.' ); + $this->markTestSkipped( 'Test skipped on WordPress versions that do not included required Block Hooks functionality.' ); } register_block_type( @@ -92,4 +92,34 @@ public function test_block_core_navigation_update_ignore_hooked_blocks_meta_pres 'Block was not added to ignored hooked blocks metadata.' ); } + + /** + * @covers ::gutenberg_block_core_navigation_update_ignore_hooked_blocks_meta + */ + public function test_block_core_navigation_dont_modify_no_post_id() { + if ( ! function_exists( 'set_ignored_hooked_blocks_metadata' ) ) { + $this->markTestSkipped( 'Test skipped on WordPress versions that do not included required Block Hooks functionality.' ); + } + + register_block_type( + 'tests/my-block', + array( + 'block_hooks' => array( + 'core/navigation' => 'last_child', + ), + ) + ); + + $original_markup = ''; + $post = new stdClass(); + $post->post_content = $original_markup; + + $post = gutenberg_block_core_navigation_update_ignore_hooked_blocks_meta( $post ); + + $this->assertSame( + $original_markup, + $post->post_content, + 'Post content did not match the original markup.' + ); + } } diff --git a/test/e2e/specs/editor/blocks/heading.spec.js b/test/e2e/specs/editor/blocks/heading.spec.js index f0271a8f6e897..f968b09d64369 100644 --- a/test/e2e/specs/editor/blocks/heading.spec.js +++ b/test/e2e/specs/editor/blocks/heading.spec.js @@ -292,6 +292,56 @@ test.describe( 'Heading', () => { ] ); } ); + test( 'Should have proper label in the list view', async ( { + editor, + page, + } ) => { + await editor.insertBlock( { name: 'core/heading' } ); + + await editor.publishPost(); + await page.reload(); + + await page + .getByRole( 'toolbar', { name: 'Document tools' } ) + .getByRole( 'button', { name: 'Document Overview' } ) + .click(); + + const listView = page.getByRole( 'treegrid', { + name: 'Block navigation structure', + } ); + + await expect( + listView.getByRole( 'link' ), + 'should show default block name if the content is empty' + ).toHaveText( 'Heading' ); + + await editor.canvas + .getByRole( 'document', { + name: 'Block: Heading', + } ) + .fill( 'Heading content' ); + + await expect( + listView.getByRole( 'link' ), + 'should show content' + ).toHaveText( 'Heading content' ); + + await editor.openDocumentSettingsSidebar(); + + await page.getByRole( 'button', { name: 'Advanced' } ).click(); + + await page + .getByRole( 'textbox', { + name: 'Block name', + } ) + .fill( 'My new name' ); + + await expect( + listView.getByRole( 'link' ), + 'should show custom name' + ).toHaveText( 'My new name' ); + } ); + test.describe( 'Block transforms', () => { test.describe( 'FROM paragraph', () => { test( 'should preserve the content', async ( { editor } ) => { diff --git a/test/e2e/specs/editor/blocks/navigation-list-view.spec.js b/test/e2e/specs/editor/blocks/navigation-list-view.spec.js index fd7113fe17095..c97ea3f1b17b5 100644 --- a/test/e2e/specs/editor/blocks/navigation-list-view.spec.js +++ b/test/e2e/specs/editor/blocks/navigation-list-view.spec.js @@ -543,6 +543,48 @@ test.describe( 'Navigation block - List view editing', () => { // we have unmounted the list view and then remounted it). await expect( linkControl.getSearchInput() ).toBeHidden(); } ); + + test( `can create a new menu without losing focus`, async ( { + page, + editor, + requestUtils, + } ) => { + await requestUtils.createNavigationMenu( navMenuBlocksFixture ); + + await editor.insertBlock( { name: 'core/navigation' } ); + + await editor.openDocumentSettingsSidebar(); + + await page.getByLabel( 'Test Menu' ).click(); + + await page.keyboard.press( 'ArrowUp' ); + + await expect( + page.getByRole( 'menuitem', { name: 'Create new menu' } ) + ).toBeFocused(); + + await page.keyboard.press( 'Enter' ); + + // Check that the menu was created + await expect( + page + .getByTestId( 'snackbar' ) + .getByText( 'Navigation Menu successfully created.' ) + ).toBeVisible(); + await expect( + page.getByText( 'This navigation menu is empty.' ) + ).toBeVisible(); + + // Move focus to the appender + await page.keyboard.press( 'ArrowDown' ); + await expect( + editor.canvas + .getByRole( 'document', { + name: 'Block: Navigation', + } ) + .getByLabel( 'Add block' ) + ).toBeFocused(); + } ); } ); class LinkControl { diff --git a/test/e2e/specs/interactivity/deferred-store.spec.ts b/test/e2e/specs/interactivity/deferred-store.spec.ts new file mode 100644 index 0000000000000..4521322e61dfc --- /dev/null +++ b/test/e2e/specs/interactivity/deferred-store.spec.ts @@ -0,0 +1,36 @@ +/** + * Internal dependencies + */ +import { test, expect } from './fixtures'; + +test.describe( 'deferred store', () => { + test.beforeAll( async ( { interactivityUtils: utils } ) => { + await utils.activatePlugins(); + await utils.addPostWithBlock( 'test/deferred-store' ); + } ); + test.beforeEach( async ( { interactivityUtils: utils, page } ) => { + await page.goto( utils.getLink( 'test/deferred-store' ) ); + } ); + test.afterAll( async ( { interactivityUtils: utils } ) => { + await utils.deactivatePlugins(); + await utils.deleteAllPosts(); + } ); + + test( 'Ensure that a store can be subscribed to before it is initialized', async ( { + page, + } ) => { + const resultInput = page.getByTestId( 'result' ); + await expect( resultInput ).toHaveText( '' ); + await expect( resultInput ).toHaveText( 'Hello, world!' ); + } ); + + // There is a known issue for deferred getters right now. + // eslint-disable-next-line playwright/no-skipped-test + test.skip( 'Ensure that a state getter can be subscribed to before it is initialized', async ( { + page, + } ) => { + const resultInput = page.getByTestId( 'result-getter' ); + await expect( resultInput ).toHaveText( '' ); + await expect( resultInput ).toHaveText( 'Hello, world!' ); + } ); +} ); diff --git a/test/e2e/specs/interactivity/store.spec.ts b/test/e2e/specs/interactivity/store.spec.ts index a97cf8c055be1..fc94103168752 100644 --- a/test/e2e/specs/interactivity/store.spec.ts +++ b/test/e2e/specs/interactivity/store.spec.ts @@ -3,7 +3,7 @@ */ import { test, expect } from './fixtures'; -test.describe( 'data-wp-bind', () => { +test.describe( 'store', () => { test.beforeAll( async ( { interactivityUtils: utils } ) => { await utils.activatePlugins(); await utils.addPostWithBlock( 'test/store' ); @@ -22,4 +22,11 @@ test.describe( 'data-wp-bind', () => { const el = page.getByTestId( 'non-plain object' ); await expect( el ).toHaveText( 'true' ); } ); + + test( 'Ensures that state cannot be set to a non-object', async ( { + page, + } ) => { + const element = page.getByTestId( 'state-0' ); + await expect( element ).toHaveText( 'right' ); + } ); } );