From b54a42667640adf4477bd89a85285dbd583b633e Mon Sep 17 00:00:00 2001
From: Artemio Morales <artemio.morales@a8c.com>
Date: Fri, 22 Mar 2024 14:37:21 -0400
Subject: [PATCH] Fix lightbox UI disallow editing (#59890)

* Check that lightbox can be edited before rendering lightbox UI

* Refactor to reduce duplicate code

* Fix and clarify component rendering logic

Fix issue wherein lightbox popover was rendering
erroneously when a link had been configured.

* Reset lightbox attributes when removing link

* Show lightbox UI if block-level override differs from default

In some cases, such as when lightbox settings already exist for
a block when global lightbox settings in theme.json change, we
should allow users to see the lightbox UI and change the settings
if they conflict with the global settings, even if the lightbox UI
is disabled globally. This prevents a block from getting stuck with
legacy lightbox settings and allows users to reset the block-level
lightbox settings if need be in these edge cases.

Note: We do not display the UI if the block-level settings exist
and match the global settings, as the block will behave as expected
in those circumstances and showing the UI in those circumstances
would likely just be confusing.

* Handle edge case of removing existing link when lightbox is fully enabled

* Fix focus loss preventing end-to-end test from passing

* Add link to PR in comment

Co-authored-by: artemiomorales <artemiosans@git.wordpress.org>
Co-authored-by: michalczaplinski <czapla@git.wordpress.org>
Co-authored-by: justintadlock <greenshady@git.wordpress.org>
Co-authored-by: annezazu <annezazu@git.wordpress.org>
Co-authored-by: gziolo <gziolo@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: jeherve <jeherve@git.wordpress.org>
---
 .../url-popover/image-url-input-ui.js         | 119 ++++++++++--------
 packages/block-library/src/image/image.js     |  22 +++-
 2 files changed, 89 insertions(+), 52 deletions(-)

diff --git a/packages/block-editor/src/components/url-popover/image-url-input-ui.js b/packages/block-editor/src/components/url-popover/image-url-input-ui.js
index ae59daa1f502b..a7b40b42f6dd7 100644
--- a/packages/block-editor/src/components/url-popover/image-url-input-ui.js
+++ b/packages/block-editor/src/components/url-popover/image-url-input-ui.js
@@ -46,6 +46,7 @@ const ImageURLInputUI = ( {
 	showLightboxSetting,
 	lightboxEnabled,
 	onSetLightbox,
+	resetLightbox,
 } ) => {
 	const [ isOpen, setIsOpen ] = useState( false );
 	// Use internal state instead of a ref to make sure that the component
@@ -241,7 +242,9 @@ const ImageURLInputUI = ( {
 	);
 
 	const linkEditorValue = urlInput !== null ? urlInput : url;
-	const showLinkEditor = ( ! linkEditorValue && ! lightboxEnabled ) === true;
+	const hideLightboxPanel =
+		! lightboxEnabled || ( lightboxEnabled && ! showLightboxSetting );
+	const showLinkEditor = ! linkEditorValue && hideLightboxPanel;
 
 	const urlLabel = (
 		getLinkDestinations().find(
@@ -249,6 +252,65 @@ const ImageURLInputUI = ( {
 		) || {}
 	).title;
 
+	const PopoverChildren = () => {
+		if (
+			lightboxEnabled &&
+			showLightboxSetting &&
+			! url &&
+			! isEditingLink
+		) {
+			return (
+				<div className="block-editor-url-popover__expand-on-click">
+					<Icon icon={ fullscreen } />
+					<div className="text">
+						<p>{ __( 'Expand on click' ) }</p>
+						<p className="description">
+							{ __( 'Scales the image with a lightbox effect' ) }
+						</p>
+					</div>
+					<Button
+						icon={ linkOff }
+						label={ __( 'Disable expand on click' ) }
+						onClick={ () => {
+							onSetLightbox( false );
+						} }
+						size="compact"
+					/>
+				</div>
+			);
+		} else if ( ! url || isEditingLink ) {
+			return (
+				<URLPopover.LinkEditor
+					className="block-editor-format-toolbar__link-container-content"
+					value={ linkEditorValue }
+					onChangeInputValue={ setUrlInput }
+					onSubmit={ onSubmitLinkChange() }
+					autocompleteRef={ autocompleteRef }
+				/>
+			);
+		} else if ( url && ! isEditingLink ) {
+			return (
+				<>
+					<URLPopover.LinkViewer
+						className="block-editor-format-toolbar__link-container-content"
+						url={ url }
+						onEditLinkClick={ startEditLink }
+						urlLabel={ urlLabel }
+					/>
+					<Button
+						icon={ linkOff }
+						label={ __( 'Remove link' ) }
+						onClick={ () => {
+							onLinkRemove();
+							resetLightbox();
+						} }
+						size="compact"
+					/>
+				</>
+			);
+		}
+	};
+
 	return (
 		<>
 			<ToolbarButton
@@ -258,7 +320,9 @@ const ImageURLInputUI = ( {
 				aria-expanded={ isOpen }
 				onClick={ openLinkUI }
 				ref={ setPopoverAnchor }
-				isActive={ !! url || lightboxEnabled }
+				isActive={
+					!! url || ( lightboxEnabled && showLightboxSetting )
+				}
 			/>
 			{ isOpen && (
 				<URLPopover
@@ -267,7 +331,7 @@ const ImageURLInputUI = ( {
 					onFocusOutside={ onFocusOutside() }
 					onClose={ closeLinkUI }
 					renderSettings={
-						! lightboxEnabled ? () => advancedOptions : null
+						hideLightboxPanel ? () => advancedOptions : null
 					}
 					additionalControls={
 						showLinkEditor && (
@@ -314,54 +378,7 @@ const ImageURLInputUI = ( {
 					}
 					offset={ 13 }
 				>
-					{ ( ! url || isEditingLink ) && ! lightboxEnabled && (
-						<>
-							<URLPopover.LinkEditor
-								className="block-editor-format-toolbar__link-container-content"
-								value={ linkEditorValue }
-								onChangeInputValue={ setUrlInput }
-								onSubmit={ onSubmitLinkChange() }
-								autocompleteRef={ autocompleteRef }
-							/>
-						</>
-					) }
-					{ url && ! isEditingLink && ! lightboxEnabled && (
-						<>
-							<URLPopover.LinkViewer
-								className="block-editor-format-toolbar__link-container-content"
-								url={ url }
-								onEditLinkClick={ startEditLink }
-								urlLabel={ urlLabel }
-							/>
-							<Button
-								icon={ linkOff }
-								label={ __( 'Remove link' ) }
-								onClick={ onLinkRemove }
-								size="compact"
-							/>
-						</>
-					) }
-					{ ! url && ! isEditingLink && lightboxEnabled && (
-						<div className="block-editor-url-popover__expand-on-click">
-							<Icon icon={ fullscreen } />
-							<div className="text">
-								<p>{ __( 'Expand on click' ) }</p>
-								<p className="description">
-									{ __(
-										'Scales the image with a lightbox effect'
-									) }
-								</p>
-							</div>
-							<Button
-								icon={ linkOff }
-								label={ __( 'Disable expand on click' ) }
-								onClick={ () => {
-									onSetLightbox( false );
-								} }
-								size="compact"
-							/>
-						</div>
-					) }
+					{ PopoverChildren() }
 				</URLPopover>
 			) }
 		</>
diff --git a/packages/block-library/src/image/image.js b/packages/block-library/src/image/image.js
index e55f6bfa03d73..6423edcecb6a7 100644
--- a/packages/block-library/src/image/image.js
+++ b/packages/block-library/src/image/image.js
@@ -274,6 +274,22 @@ export default function Image( {
 		}
 	}
 
+	function resetLightbox() {
+		// When deleting a link from an image while lightbox settings
+		// are enabled by default, we should disable the lightbox,
+		// otherwise the resulting UX looks like a mistake.
+		// See https://github.com/WordPress/gutenberg/pull/59890/files#r1532286123.
+		if ( lightboxSetting?.enabled && lightboxSetting?.allowEditing ) {
+			setAttributes( {
+				lightbox: { enabled: false },
+			} );
+		} else {
+			setAttributes( {
+				lightbox: undefined,
+			} );
+		}
+	}
+
 	function onSetTitle( value ) {
 		// This is the HTML title attribute, separate from the media object
 		// title.
@@ -348,7 +364,10 @@ export default function Image( {
 	const [ lightboxSetting ] = useSettings( 'lightbox' );
 
 	const showLightboxSetting =
-		!! lightbox || lightboxSetting?.allowEditing === true;
+		// If a block-level override is set, we should give users the option to
+		// remove that override, even if the lightbox UI is disabled in the settings.
+		( !! lightbox && lightbox?.enabled !== lightboxSetting?.enabled ) ||
+		lightboxSetting?.allowEditing;
 
 	const lightboxChecked =
 		!! lightbox?.enabled || ( ! lightbox && !! lightboxSetting?.enabled );
@@ -498,6 +517,7 @@ export default function Image( {
 							showLightboxSetting={ showLightboxSetting }
 							lightboxEnabled={ lightboxChecked }
 							onSetLightbox={ onSetLightbox }
+							resetLightbox={ resetLightbox }
 						/>
 					) }
 				{ allowCrop && (