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 <[email protected]> Co-authored-by: michalczaplinski <[email protected]> Co-authored-by: justintadlock <[email protected]> Co-authored-by: annezazu <[email protected]> Co-authored-by: gziolo <[email protected]> Co-authored-by: t-hamano <[email protected]> Co-authored-by: jeherve <[email protected]>
WordPress · Apr 9, 2024 · b54a426 · b54a426
1 parent b2a0853
commit b54a426
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 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
@@ -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,14 +242,75 @@ 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(
 			( destination ) => destination.linkDestination === linkDestination
 		) || {}
 	).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
@@ -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 && (