From 697495fe7d5bf46b64768012da69bb736e0f7fca Mon Sep 17 00:00:00 2001
From: Artemio Morales <artemio.morales@a8c.com>
Date: Mon, 29 Jan 2024 15:35:31 -0500
Subject: [PATCH] Image block: Move UI for lightbox from sidebar to the content
 toolbar alongside link settings (#57608)

* Add first pass at Expand on Click in toolbar

* Style changes

* Fix lightbox setting check for toolbar

* Remove old UI

* Sync toolbar UI with global styles settings

* Compact buttons

* Update style.scss

* Update UI

* Make input width smaller

* Remove chevron in Expand on Click popover

* Improve markup and styles

* Remove erroneous closing of popover after link is removed

* Improve button label for removing lightbox

* Update label name in test

---------

Co-authored-by: Rich Tabor <hi@richtabor.com>
---
 .../components/media-replace-flow/style.scss  |   2 +-
 .../url-popover/image-url-input-ui.js         | 123 ++++++++++++------
 .../src/components/url-popover/index.js       |   2 +
 .../src/components/url-popover/link-editor.js |   1 +
 .../src/components/url-popover/link-viewer.js |   1 +
 .../src/components/url-popover/style.scss     |  82 +++++++-----
 packages/block-library/src/image/image.js     |  53 +++-----
 test/e2e/specs/editor/blocks/image.spec.js    |   5 +-
 8 files changed, 163 insertions(+), 106 deletions(-)

diff --git a/packages/block-editor/src/components/media-replace-flow/style.scss b/packages/block-editor/src/components/media-replace-flow/style.scss
index 61df542cf5840..21fc489b8a2c6 100644
--- a/packages/block-editor/src/components/media-replace-flow/style.scss
+++ b/packages/block-editor/src/components/media-replace-flow/style.scss
@@ -27,7 +27,7 @@
 	}
 
 	.block-editor-link-control {
-		width: 300px; // Hardcoded width avoids resizing of control when switching between preview/edit.
+		width: 200px; // Hardcoded width avoids resizing of control when switching between preview/edit.
 
 		.block-editor-url-input {
 			padding: 0; // Cancel unnecessary default 1px padding in this case.
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 7caa218658b24..f6946b2c8e65d 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
@@ -6,15 +6,19 @@ import { useRef, useState } from '@wordpress/element';
 import {
 	ToolbarButton,
 	Button,
-	NavigableMenu,
 	MenuItem,
 	ToggleControl,
 	TextControl,
-	SVG,
-	Path,
+	MenuGroup,
 	__experimentalVStack as VStack,
 } from '@wordpress/components';
-import { link as linkIcon, close } from '@wordpress/icons';
+import {
+	link as linkIcon,
+	image,
+	page,
+	fullscreen,
+	linkOff,
+} from '@wordpress/icons';
 
 /**
  * Internal dependencies
@@ -27,14 +31,6 @@ const LINK_DESTINATION_MEDIA = 'media';
 const LINK_DESTINATION_ATTACHMENT = 'attachment';
 const NEW_TAB_REL = [ 'noreferrer', 'noopener' ];
 
-const icon = (
-	<SVG viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg">
-		<Path d="M0,0h24v24H0V0z" fill="none" />
-		<Path d="m19 5v14h-14v-14h14m0-2h-14c-1.1 0-2 0.9-2 2v14c0 1.1 0.9 2 2 2h14c1.1 0 2-0.9 2-2v-14c0-1.1-0.9-2-2-2z" />
-		<Path d="m14.14 11.86l-3 3.87-2.14-2.59-3 3.86h12l-3.86-5.14z" />
-	</SVG>
-);
-
 const ImageURLInputUI = ( {
 	linkDestination,
 	onChangeUrl,
@@ -45,6 +41,9 @@ const ImageURLInputUI = ( {
 	linkTarget,
 	linkClass,
 	rel,
+	showLightboxSetting,
+	lightboxEnabled,
+	onSetLightbox,
 } ) => {
 	const [ isOpen, setIsOpen ] = useState( false );
 	// Use internal state instead of a ref to make sure that the component
@@ -138,6 +137,7 @@ const ImageURLInputUI = ( {
 				onChangeUrl( {
 					href: urlInput,
 					linkDestination: selectedDestination,
+					lightbox: { enabled: false },
 				} );
 			}
 			stopEditLink();
@@ -157,22 +157,17 @@ const ImageURLInputUI = ( {
 		const linkDestinations = [
 			{
 				linkDestination: LINK_DESTINATION_MEDIA,
-				title: __( 'Media File' ),
+				title: __( 'Link to image file' ),
 				url: mediaType === 'image' ? mediaUrl : undefined,
-				icon,
+				icon: image,
 			},
 		];
 		if ( mediaType === 'image' && mediaLink ) {
 			linkDestinations.push( {
 				linkDestination: LINK_DESTINATION_ATTACHMENT,
-				title: __( 'Attachment Page' ),
+				title: __( 'Link to attachment page' ),
 				url: mediaType === 'image' ? mediaLink : undefined,
-				icon: (
-					<SVG viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg">
-						<Path d="M0 0h24v24H0V0z" fill="none" />
-						<Path d="M14 2H6c-1.1 0-1.99.9-1.99 2L4 20c0 1.1.89 2 1.99 2H18c1.1 0 2-.9 2-2V8l-6-6zM6 20V4h7v5h5v11H6z" />
-					</SVG>
-				),
+				icon: page,
 			} );
 		}
 		return linkDestinations;
@@ -225,7 +220,7 @@ const ImageURLInputUI = ( {
 			/>
 			<TextControl
 				__nextHasNoMarginBottom
-				label={ __( 'Link CSS Class' ) }
+				label={ __( 'Link CSS class' ) }
 				value={ linkClass || '' }
 				onChange={ onSetLinkClass }
 			/>
@@ -233,6 +228,7 @@ const ImageURLInputUI = ( {
 	);
 
 	const linkEditorValue = urlInput !== null ? urlInput : url;
+	const showLinkEditor = ( ! linkEditorValue && ! lightboxEnabled ) === true;
 
 	const urlLabel = (
 		getLinkDestinations().find(
@@ -245,7 +241,7 @@ const ImageURLInputUI = ( {
 			<ToolbarButton
 				icon={ linkIcon }
 				className="components-toolbar__control"
-				label={ url ? __( 'Edit link' ) : __( 'Insert link' ) }
+				label={ url ? __( 'Edit link' ) : __( 'Add link' ) }
 				aria-expanded={ isOpen }
 				onClick={ openLinkUI }
 				ref={ setPopoverAnchor }
@@ -256,14 +252,17 @@ const ImageURLInputUI = ( {
 					anchor={ popoverAnchor }
 					onFocusOutside={ onFocusOutside() }
 					onClose={ closeLinkUI }
-					renderSettings={ () => advancedOptions }
+					renderSettings={
+						! lightboxEnabled ? () => advancedOptions : null
+					}
 					additionalControls={
-						! linkEditorValue && (
-							<NavigableMenu>
+						showLinkEditor && (
+							<MenuGroup>
 								{ getLinkDestinations().map( ( link ) => (
 									<MenuItem
 										key={ link.linkDestination }
 										icon={ link.icon }
+										iconPosition="left"
 										onClick={ () => {
 											setUrlInput( null );
 											onSetHref( link.url );
@@ -273,20 +272,45 @@ const ImageURLInputUI = ( {
 										{ link.title }
 									</MenuItem>
 								) ) }
-							</NavigableMenu>
+								{ showLightboxSetting && (
+									<MenuItem
+										key="expand-on-click"
+										className="block-editor-url-popover__expand-on-click"
+										icon={ fullscreen }
+										info={ __(
+											'Scale the image with a lightbox effect.'
+										) }
+										iconPosition="left"
+										onClick={ () => {
+											setUrlInput( null );
+											onChangeUrl( {
+												linkDestination:
+													LINK_DESTINATION_NONE,
+												href: '',
+											} );
+											onSetLightbox( true );
+											stopEditLink();
+										} }
+									>
+										{ __( 'Expand on click' ) }
+									</MenuItem>
+								) }
+							</MenuGroup>
 						)
 					}
 				>
-					{ ( ! url || isEditingLink ) && (
-						<URLPopover.LinkEditor
-							className="block-editor-format-toolbar__link-container-content"
-							value={ linkEditorValue }
-							onChangeInputValue={ setUrlInput }
-							onSubmit={ onSubmitLinkChange() }
-							autocompleteRef={ autocompleteRef }
-						/>
+					{ ( ! url || isEditingLink ) && ! lightboxEnabled && (
+						<>
+							<URLPopover.LinkEditor
+								className="block-editor-format-toolbar__link-container-content"
+								value={ linkEditorValue }
+								onChangeInputValue={ setUrlInput }
+								onSubmit={ onSubmitLinkChange() }
+								autocompleteRef={ autocompleteRef }
+							/>
+						</>
 					) }
-					{ url && ! isEditingLink && (
+					{ url && ! isEditingLink && ! lightboxEnabled && (
 						<>
 							<URLPopover.LinkViewer
 								className="block-editor-format-toolbar__link-container-content"
@@ -295,12 +319,37 @@ const ImageURLInputUI = ( {
 								urlLabel={ urlLabel }
 							/>
 							<Button
-								icon={ close }
+								icon={ linkOff }
 								label={ __( 'Remove link' ) }
 								onClick={ onLinkRemove }
+								size="compact"
 							/>
 						</>
 					) }
+					{ ! url && ! isEditingLink && lightboxEnabled && (
+						<div className="block-editor-url-popover__expand-on-click">
+							<div className="fullscreen-icon">
+								{ fullscreen }
+							</div>
+							<div className="text">
+								<p>{ __( 'Expand on click' ) }</p>
+								<p className="description">
+									{ __(
+										'Scales the image with a lightbox effect'
+									) }
+								</p>
+							</div>
+							<Button
+								icon={ linkOff }
+								className="remove-link"
+								label={ __( 'Disable expand on click' ) }
+								onClick={ () => {
+									onSetLightbox( false );
+								} }
+								size="compact"
+							/>
+						</div>
+					) }
 				</URLPopover>
 			) }
 		</>
diff --git a/packages/block-editor/src/components/url-popover/index.js b/packages/block-editor/src/components/url-popover/index.js
index 071d2c7e00b8b..a62d1bb750b15 100644
--- a/packages/block-editor/src/components/url-popover/index.js
+++ b/packages/block-editor/src/components/url-popover/index.js
@@ -70,6 +70,7 @@ function URLPopover( {
 			focusOnMount={ focusOnMount }
 			placement={ computedPlacement }
 			shift
+			variant="toolbar"
 			{ ...popoverProps }
 		>
 			<div className="block-editor-url-popover__input-container">
@@ -82,6 +83,7 @@ function URLPopover( {
 							label={ __( 'Link settings' ) }
 							onClick={ toggleSettingsVisibility }
 							aria-expanded={ isSettingsExpanded }
+							size="compact"
 						/>
 					) }
 				</div>
diff --git a/packages/block-editor/src/components/url-popover/link-editor.js b/packages/block-editor/src/components/url-popover/link-editor.js
index b0502fbb2a402..9acae97615efc 100644
--- a/packages/block-editor/src/components/url-popover/link-editor.js
+++ b/packages/block-editor/src/components/url-popover/link-editor.js
@@ -40,6 +40,7 @@ export default function LinkEditor( {
 				icon={ keyboardReturn }
 				label={ __( 'Apply' ) }
 				type="submit"
+				size="compact"
 			/>
 		</form>
 	);
diff --git a/packages/block-editor/src/components/url-popover/link-viewer.js b/packages/block-editor/src/components/url-popover/link-viewer.js
index d34c9baa37734..20db486e19caa 100644
--- a/packages/block-editor/src/components/url-popover/link-viewer.js
+++ b/packages/block-editor/src/components/url-popover/link-viewer.js
@@ -41,6 +41,7 @@ export default function LinkViewer( {
 					icon={ edit }
 					label={ __( 'Edit' ) }
 					onClick={ onEditLinkClick }
+					size="compact"
 				/>
 			) }
 		</div>
diff --git a/packages/block-editor/src/components/url-popover/style.scss b/packages/block-editor/src/components/url-popover/style.scss
index b0aacc5ff6e41..4038058abeb19 100644
--- a/packages/block-editor/src/components/url-popover/style.scss
+++ b/packages/block-editor/src/components/url-popover/style.scss
@@ -1,53 +1,39 @@
 .block-editor-url-popover__additional-controls {
-	border-top: $border-width solid $gray-300;
+	border-top: $border-width solid $gray-900;
+	padding: $grid-unit-10 $grid-unit-10;
 }
 
-.block-editor-url-popover__additional-controls > div[role="menu"] .components-button:not(:disabled):not([aria-disabled="true"]):not(.is-secondary) > svg {
-	box-shadow: none;
-}
-
-.block-editor-url-popover__additional-controls div[role="menu"] > .components-button {
-	padding-left: $grid-unit-15;
+.block-editor-url-popover__input-container {
+	padding: $grid-unit-10;
+	padding-left: $grid-unit-20;
 }
 
 .block-editor-url-popover__row {
 	display: flex;
+	gap: $grid-unit-05;
 }
 
 // Any children of the popover-row that are not the settings-toggle
 // should take up as much space as possible.
 .block-editor-url-popover__row > :not(.block-editor-url-popover__settings-toggle) {
 	flex-grow: 1;
+	gap: $grid-unit-05;
 }
 
-// Mimic toolbar component styles for the icons in this popover.
-.block-editor-url-popover .components-button.has-icon {
-	padding: 3px;
+.block-editor-url-popover__additional-controls .components-button.has-icon {
+	padding-left: $grid-unit-10;
+	padding-right: $grid-unit-10;
+	height: auto;
+	text-align: left;
 
 	> svg {
-		padding: 5px;
-		border-radius: $radius-block-ui;
-		height: 30px;
-		width: 30px;
-	}
-
-	&:not(:disabled):focus {
-		box-shadow: none;
-
-		> svg {
-			@include block-toolbar-button-style__focus();
-		}
+		margin-right: $grid-unit-10;
 	}
 }
 
 .block-editor-url-popover__settings-toggle {
 	flex-shrink: 0;
 
-	// Add a left divider to the toggle button.
-	border-radius: 0;
-	border-left: $border-width solid $gray-300;
-	margin-left: 1px;
-
 	&[aria-expanded="true"] .dashicon {
 		transform: rotate(180deg);
 	}
@@ -56,7 +42,7 @@
 .block-editor-url-popover__settings {
 	display: block;
 	padding: $grid-unit-20;
-	border-top: $border-width solid $gray-300;
+	border-top: $border-width solid $gray-900;
 }
 
 .block-editor-url-popover__link-editor,
@@ -65,16 +51,50 @@
 }
 
 .block-editor-url-popover__link-viewer-url {
-	margin: $grid-unit-10 - $border-width;
+	display: flex;
+	align-items: center;
 	flex-grow: 1;
 	flex-shrink: 1;
 	overflow: hidden;
 	text-overflow: ellipsis;
 	white-space: nowrap;
-	min-width: 150px;
-	max-width: 500px;
+	margin-right: $grid-unit-10;
 
 	&.has-invalid-link {
 		color: $alert-red;
 	}
 }
+
+.block-editor-url-popover__expand-on-click {
+	display: flex;
+	align-items: center;
+	min-width: $modal-min-width;
+	overflow: hidden;
+	white-space: nowrap;
+
+	.fullscreen-icon {
+		padding-right: $grid-unit-05;
+
+		> svg {
+			width: $icon-size;
+			height: $icon-size;
+		}
+	}
+
+	.text {
+		flex-grow: 1;
+
+		p {
+			margin: 0;
+		}
+	}
+
+	.description {
+		color: $gray-600;
+	}
+
+	.remove-link {
+		margin-right: $grid-unit-10;
+	}
+
+}
diff --git a/packages/block-library/src/image/image.js b/packages/block-library/src/image/image.js
index 673c95f075edf..d8287676d27f7 100644
--- a/packages/block-library/src/image/image.js
+++ b/packages/block-library/src/image/image.js
@@ -7,7 +7,6 @@ import {
 	ResizableBox,
 	Spinner,
 	TextareaControl,
-	ToggleControl,
 	TextControl,
 	ToolbarButton,
 	ToolbarGroup,
@@ -281,6 +280,22 @@ export default function Image( {
 		setAttributes( props );
 	}
 
+	function onSetLightbox( enable ) {
+		if ( enable && ! lightboxSetting?.enabled ) {
+			setAttributes( {
+				lightbox: { enabled: true },
+			} );
+		} else if ( ! enable && lightboxSetting?.enabled ) {
+			setAttributes( {
+				lightbox: { enabled: false },
+			} );
+		} else {
+			setAttributes( {
+				lightbox: undefined,
+			} );
+		}
+	}
+
 	function onSetTitle( value ) {
 		// This is the HTML title attribute, separate from the media object
 		// title.
@@ -350,15 +365,12 @@ export default function Image( {
 
 	const [ lightboxSetting ] = useSettings( 'lightbox' );
 
-	const showLightboxToggle =
+	const showLightboxSetting =
 		!! lightbox || lightboxSetting?.allowEditing === true;
 
 	const lightboxChecked =
 		!! lightbox?.enabled || ( ! lightbox && !! lightboxSetting?.enabled );
 
-	const lightboxToggleDisabled =
-		linkDestination && linkDestination !== 'none';
-
 	const dimensionsControl = (
 		<DimensionsTool
 			value={ { width, height, scale, aspectRatio } }
@@ -463,6 +475,9 @@ export default function Image( {
 							linkTarget={ linkTarget }
 							linkClass={ linkClass }
 							rel={ rel }
+							showLightboxSetting={ showLightboxSetting }
+							lightboxEnabled={ lightboxChecked }
+							onSetLightbox={ onSetLightbox }
 						/>
 					) }
 				{ allowCrop && (
@@ -559,34 +574,6 @@ export default function Image( {
 							options={ imageSizeOptions }
 						/>
 					) }
-					{ showLightboxToggle && (
-						<ToolsPanelItem
-							hasValue={ () => !! lightbox }
-							label={ __( 'Expand on click' ) }
-							onDeselect={ () => {
-								setAttributes( { lightbox: undefined } );
-							} }
-							isShownByDefault={ true }
-						>
-							<ToggleControl
-								label={ __( 'Expand on click' ) }
-								checked={ lightboxChecked }
-								onChange={ ( newValue ) => {
-									setAttributes( {
-										lightbox: { enabled: newValue },
-									} );
-								} }
-								disabled={ lightboxToggleDisabled }
-								help={
-									lightboxToggleDisabled
-										? __(
-												'“Expand on click” scales the image up, and can’t be combined with a link.'
-										  )
-										: ''
-								}
-							/>
-						</ToolsPanelItem>
-					) }
 				</ToolsPanel>
 			</InspectorControls>
 			<InspectorControls group="advanced">
diff --git a/test/e2e/specs/editor/blocks/image.spec.js b/test/e2e/specs/editor/blocks/image.spec.js
index 9080a6dc19402..d47f8f3490edc 100644
--- a/test/e2e/specs/editor/blocks/image.spec.js
+++ b/test/e2e/specs/editor/blocks/image.spec.js
@@ -670,10 +670,7 @@ test.describe( 'Image', () => {
 			imageBlock.locator( 'data-testid=form-file-upload-input' )
 		);
 
-		await page
-			.getByLabel( 'Block tools' )
-			.getByLabel( 'Insert link' )
-			.click();
+		await page.getByLabel( 'Block tools' ).getByLabel( 'Add link' ).click();
 
 		// This form lacks distinguishing qualities other than the
 		// class name, so we use page.locator() instead of page.getByRole()