Fix incorrect Link To value mask caused by stale route parameters

Originally, ImageLinkDestinationsScreen relied upon (1) differing route parameter names from LinkPicker and (2) explicitly set `linkDestination` while LinkPicker did not. Stale route parameters resulted in erroneous attribute updates when swapping the link destination from Media File > Custom URL > Media File. This change addresses the stale route parameters by (1) unifying the route parameter names between the two components and (2) unifying the approach for setting `linkDestination` to be computed based upon the URL. The URL, represented as the `inputValue` route parameter, is now the sole route parameter.
WordPress · Nov 1, 2021 · cf4027f · cf4027f
1 parent 2fdd8c1
commit cf4027f
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 12 deletions.
diff --git a/packages/block-library/src/image/edit.native.js b/packages/block-library/src/image/edit.native.js
@@ -90,15 +90,29 @@ function LinkSettings( {
 
 	// Persist attributes passed from child screen
 	useEffect( () => {
-		const { linkDestination: newLinkDestination, url: newUrl } =
-			route.params || {};
-		if ( newLinkDestination || newUrl ) {
-			setMappedAttributes( {
-				url: newUrl,
-				linkDestination: newLinkDestination,
-			} );
+		const { inputValue: newUrl } = route.params || {};
+
+		let newLinkDestination;
+		switch ( newUrl ) {
+			case attributes.url:
+				newLinkDestination = LINK_DESTINATION_MEDIA;
+				break;
+			case image?.link:
+				newLinkDestination = LINK_DESTINATION_ATTACHMENT;
+				break;
+			case '':
+				newLinkDestination = LINK_DESTINATION_NONE;
+				break;
+			default:
+				newLinkDestination = LINK_DESTINATION_CUSTOM;
+				break;
 		}
-	}, [ route.params?.url, route.params?.linkDestination ] );
+
+		setMappedAttributes( {
+			url: newUrl,
+			linkDestination: newLinkDestination,
+		} );
+	}, [ route.params?.inputValue ] );
 
 	let valueMask;
 	switch ( linkDestination ) {

diff --git a/packages/block-library/src/image/test/edit.native.js b/packages/block-library/src/image/test/edit.native.js
@@ -124,6 +124,40 @@ describe( 'Image Block', () => {
 		expect( getEditorHtml() ).toBe( expectedHtml );
 	} );
 
+	it( 'swaps the link between destinations', async () => {
+		const initialHtml = `
+		<!-- wp:image {"id":1,"sizeSlug":"large","linkDestination":"none","className":"is-style-default"} -->
+		<figure class="wp-block-image size-large is-style-default">
+			<img src="https://cldup.com/cXyG__fTLN.jpg" alt="" class="wp-image-1"/>
+		<figcaption>Mountain</figcaption></figure>
+		<!-- /wp:image -->`;
+		const screen = await initializeEditor( { initialHtml } );
+		// We must await the image fetch via `getMedia`
+		await act( () => apiFetchPromise );
+
+		fireEvent.press( screen.getByA11yLabel( /Image Block/ ) );
+		// Awaiting navigation event seemingly required due to React Navigation bug
+		// https://git.io/Ju35Z
+		await act( () =>
+			fireEvent.press( screen.getByA11yLabel( 'Open Settings' ) )
+		);
+		fireEvent.press( screen.getByText( 'None' ) );
+		fireEvent.press( screen.getByText( 'Media File' ) );
+		fireEvent.press( screen.getByText( 'Custom URL' ) );
+		fireEvent.changeText(
+			screen.getByPlaceholderText( 'Search or type URL' ),
+			'wordpress.org'
+		);
+		fireEvent.press( screen.getByA11yLabel( 'Apply' ) );
+		fireEvent.press( screen.getByText( 'Custom URL' ) );
+		fireEvent.press( screen.getByText( 'Media File' ) );
+
+		const expectedHtml = `<!-- wp:image {"id":1,"sizeSlug":"large","linkDestination":"media","className":"is-style-default"} -->
+<figure class="wp-block-image size-large is-style-default"><a href="https://cldup.com/cXyG__fTLN.jpg"><img src="https://cldup.com/cXyG__fTLN.jpg" alt="" class="wp-image-1"/></a><figcaption>Mountain</figcaption></figure>
+<!-- /wp:image -->`;
+		expect( getEditorHtml() ).toBe( expectedHtml );
+	} );
+
 	it( 'sets link target', async () => {
 		const initialHtml = `
 		<!-- wp:image {"id":1,"sizeSlug":"large","linkDestination":"custom","className":"is-style-default"} -->

diff --git a/packages/components/src/mobile/link-settings/image-link-destinations-screen.native.js b/packages/components/src/mobile/link-settings/image-link-destinations-screen.native.js
@@ -88,10 +88,12 @@ function ImageLinkDestinationsScreen( props ) {
 		}
 
 		navigation.navigate( blockSettingsScreens.settings, {
-			inputValue: '', // Reset to avoid stale value in link picker input
-			text: undefined, // Reset to avoid stale value in link picker input
-			url: newUrl,
-			linkDestination: newLinkDestination,
+			// The `inputValue` name is reused from LinkPicker, as it helps avoid
+			// bugs from stale values remaining in the React Navigation route
+			// parameters
+			inputValue: newUrl,
+			// Clear link text value that may be set from LinkPicker
+			text: '',
 		} );
 	};