Widgets: Fix creating and editing non-multi widgets (#32978)

* Widgets: Fix creating and editing non-multi widgets

* Add regression e2e tests

* Lint

* Lint

* Deactivate gutenberg-test-marquee-widget after test runs

Co-authored-by: Adam Zieliński <[email protected]>

packages/e2e-tests/plugins/marquee-function-widget.php

@@ -0,0 +1,51 @@
+<?php
+/**
+ * Plugin Name: Gutenberg Test Marquee Widget
+ * Plugin URI: https://github.com/WordPress/gutenberg
+ * Author: Gutenberg Team
+ *
+ * @package gutenberg-test-marquee-widget
+ */
+
+/**
+ * Add a non-WP_Widget marquee widget.
+ */
+function marquee_greeting_init() {
+	wp_register_sidebar_widget(
+		'marquee_greeting',
+		'Marquee Greeting',
+		function() {
+			$greeting = get_option( 'marquee_greeting', 'Hello!' );
+			printf( '<marquee>%s</marquee>', esc_html( $greeting ) );
+		}
+	);
+
+	wp_register_widget_control(
+		'marquee_greeting',
+		'Marquee Greeting',
+		function() {
+			if ( isset( $_POST['marquee-greeting'] ) ) {
+				update_option(
+					'marquee_greeting',
+					sanitize_text_field( $_POST['marquee-greeting'] )
+				);
+			}
+
+			$greeting = get_option( 'marquee_greeting' );
+			?>
+			<p>
+				<label for="marquee-greeting">Greeting:</label>
+				<input
+					id="marquee-greeting"
+					class="widefat"
+					name="marquee-greeting"
+					type="text"
+					value="<?php echo esc_attr( $greeting ); ?>"
+					placeholder="Hello!"
+				/>
+			</p>
+			<?php
+		}
+	);
+}
+add_action( 'init', 'marquee_greeting_init' );

packages/e2e-tests/specs/widgets/editing-widgets.test.js

@@ -16,7 +16,7 @@ import {
  * External dependencies
  */
 // eslint-disable-next-line no-restricted-imports
-import { find, findAll } from 'puppeteer-testing-library';
+import { find, findAll, waitFor } from 'puppeteer-testing-library';
 import { groupBy, mapValues } from 'lodash';
 
 describe( 'Widgets screen', () => {
@@ -394,6 +394,111 @@ describe( 'Widgets screen', () => {
 	` );
 	} );
 
+	async function addMarquee() {
+		// There will be 2 matches here.
+		// One is the in-between inserter,
+		// and the other one is the button block appender.
+		const [ inlineInserterButton ] = await findAll( {
+			role: 'combobox',
+			name: 'Add block',
+		} );
+		await inlineInserterButton.click();
+
+		// TODO: Convert to find() API from puppeteer-testing-library.
+		const inserterSearchBox = await page.waitForSelector(
+			'aria/Search for blocks and patterns[role="searchbox"]'
+		);
+		await expect( inserterSearchBox ).toHaveFocus();
+
+		await page.keyboard.type( 'Marquee' );
+
+		const inlineQuickInserter = await find( {
+			role: 'listbox',
+			name: 'Blocks',
+		} );
+		const marqueeBlockOption = await find(
+			{
+				role: 'option',
+			},
+			{
+				root: inlineQuickInserter,
+			}
+		);
+		await marqueeBlockOption.click();
+	}
+
+	it( 'Should add and save the marquee widget', async () => {
+		await activatePlugin( 'gutenberg-test-marquee-widget' );
+		await visitAdminPage( 'widgets.php' );
+
+		await addMarquee();
+
+		await find( {
+			selector: '[data-block][data-type="core/legacy-widget"]',
+		} );
+
+		const greetingsInput = await find( {
+			selector: '#marquee-greeting',
+		} );
+		await greetingsInput.click();
+		await page.keyboard.type( 'Howdy' );
+
+		await saveWidgets();
+
+		let editedSerializedWidgetAreas = await getSerializedWidgetAreas();
+		await expect( editedSerializedWidgetAreas ).toMatchInlineSnapshot( `
+		Object {
+		  "sidebar-1": "<marquee>Hello!</marquee>",
+		}
+	` );
+
+		await page.reload();
+
+		editedSerializedWidgetAreas = await getSerializedWidgetAreas();
+		await expect( editedSerializedWidgetAreas ).toMatchInlineSnapshot( `
+		Object {
+		  "sidebar-1": "<marquee>Hello!</marquee>",
+		}
+	` );
+
+		// Add another marquee, it shouldn't be saved
+		await addMarquee();
+
+		// It takes a moment to load the form, let's wait for it.
+		await waitFor( async () => {
+			const marquees = await findAll( {
+				selector: '[id=marquee-greeting]',
+			} );
+			if ( marquees.length === 1 ) {
+				throw new Error();
+			}
+		} );
+
+		const marquees = await findAll( {
+			selector: '[id=marquee-greeting]',
+		} );
+
+		expect( marquees ).toHaveLength( 2 );
+		await marquees[ 1 ].click();
+		await page.keyboard.type( 'Second howdy' );
+
+		await saveWidgets();
+		editedSerializedWidgetAreas = await getSerializedWidgetAreas();
+		await expect( editedSerializedWidgetAreas ).toMatchInlineSnapshot( `
+		Object {
+		  "sidebar-1": "<marquee>Hello!</marquee>",
+		}
+	` );
+
+		await page.reload();
+		const marqueesAfter = await findAll( {
+			selector: '[id=marquee-greeting]',
+		} );
+		expect( marqueesAfter ).toHaveLength( 1 );
+
+		await deactivatePlugin( 'gutenberg-test-marquee-widget' );
+	} );
+
 	// Disable reason: We temporary skip this test until we can figure out why it fails sometimes.
 	// eslint-disable-next-line jest/no-disabled-tests
 	it.skip( 'Should duplicate the widgets', async () => {
@@ -423,6 +528,7 @@ describe( 'Widgets screen', () => {
 		  "sidebar-1": "<div class=\\"widget widget_block widget_text\\"><div class=\\"widget-content\\">
 		<p>First Paragraph</p>
 		</div></div>",
+		  "wp_inactive_widgets": "",
 		}
 	` );
 		const initialWidgets = await getWidgetAreaWidgets();
@@ -493,6 +599,7 @@ describe( 'Widgets screen', () => {
 		<div class=\\"widget widget_block widget_text\\"><div class=\\"widget-content\\">
 		<p>First Paragraph</p>
 		</div></div>",
+		  "wp_inactive_widgets": "",
 		}
 	` );
 		const editedWidgets = await getWidgetAreaWidgets();

packages/edit-widgets/src/store/actions.js

@@ -148,13 +148,15 @@ export function* saveWidgetArea( widgetAreaId ) {
 		const widgetId = getWidgetIdFromBlock( block );
 		const oldWidget = widgets[ widgetId ];
 		const widget = transformBlockToWidget( block, oldWidget );
+
 		// We'll replace the null widgetId after save, but we track it here
 		// since order is important.
 		sidebarWidgetsIds.push( widgetId );
 
-		// We need to check for the id in the widget object here, because a deleted
-		// and restored widget won't have this id.
-		if ( widget.id ) {
+		// Check oldWidget as widgetId might refer to an ID which has been
+		// deleted, e.g. if a deleted block is restored via undo after saving.
+		if ( oldWidget ) {
+			// Update an existing widget.
 			yield dispatch(
 				'core',
 				'editEntityRecord',
@@ -184,6 +186,7 @@ export function* saveWidgetArea( widgetAreaId ) {
 				saveEditedEntityRecord( 'root', 'widget', widgetId )
 			);
 		} else {
+			// Create a new widget.
 			batchTasks.push( ( { saveEntityRecord } ) =>
 				saveEntityRecord( 'root', 'widget', {
 					...widget,

packages/widgets/src/blocks/legacy-widget/edit/control.js

@@ -52,7 +52,10 @@ export default class Control {
 		// a fake but unique number.
 		this.number = ++lastNumber;
 
-		this.handleFormChange = debounce( this.saveForm.bind( this ), 200 );
+		this.handleFormChange = debounce(
+			this.handleFormChange.bind( this ),
+			200
+		);
 		this.handleFormSubmit = this.handleFormSubmit.bind( this );
 
 		this.initDOM();
@@ -214,6 +217,18 @@ export default class Control {
 		}
 	}
 
+	/**
+	 * Perform a save when a multi widget's form is changed. Non-multi widgets
+	 * are saved manually.
+	 *
+	 * @access private
+	 */
+	handleFormChange() {
+		if ( this.idBase ) {
+			this.saveForm();
+		}
+	}
+
 	/**
 	 * Perform a save when the control's form is manually submitted.
 	 *

packages/widgets/src/blocks/legacy-widget/edit/index.js

@@ -131,7 +131,8 @@ function NotEmpty( {
 		);
 	}
 
-	const mode = isNavigationMode || ! isSelected ? 'preview' : 'edit';
+	const mode =
+		idBase && ( isNavigationMode || ! isSelected ) ? 'preview' : 'edit';
 
 	return (
 		<>