From a0550dcc8071978057aeebccbda92955b1169a97 Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Wed, 30 Oct 2024 16:54:39 +0200 Subject: [PATCH 1/6] Image block: Add support for "more" dropdown for additional tools in Write mode --- packages/block-library/src/image/image.js | 343 +++++++++++++++------- 1 file changed, 229 insertions(+), 114 deletions(-) diff --git a/packages/block-library/src/image/image.js b/packages/block-library/src/image/image.js index 89bf31f92664b..3118f8009a30c 100644 --- a/packages/block-library/src/image/image.js +++ b/packages/block-library/src/image/image.js @@ -15,8 +15,11 @@ import { __experimentalToolsPanelItem as ToolsPanelItem, __experimentalUseCustomUnits as useCustomUnits, Placeholder, + MenuItem, + ToolbarItem, + DropdownMenu, } from '@wordpress/components'; -import { useViewportMatch } from '@wordpress/compose'; +import { useViewportMatch, usePrevious } from '@wordpress/compose'; import { useSelect, useDispatch } from '@wordpress/data'; import { BlockControls, @@ -35,7 +38,7 @@ import { __, _x, sprintf, isRTL } from '@wordpress/i18n'; import { DOWN } from '@wordpress/keycodes'; import { getFilename } from '@wordpress/url'; import { getBlockBindingsSource, switchToBlockType } from '@wordpress/blocks'; -import { crop, overlayText, upload } from '@wordpress/icons'; +import { crop, overlayText, upload, chevronDown } from '@wordpress/icons'; import { store as noticesStore } from '@wordpress/notices'; import { store as coreStore } from '@wordpress/core-data'; @@ -69,6 +72,11 @@ const scaleOptions = [ }, ]; +const WRITEMODE_POPOVER_PROPS = { + placement: 'bottom-start', +}; +const WRITEMODE_CONTROLS_POPOVER_PROPS = { position: 'bottom right' }; + // If the image has a href, wrap in an tag to trigger any inherited link element styles. const ImageWrapper = ( { href, children } ) => { if ( ! href ) { @@ -94,6 +102,216 @@ const ImageWrapper = ( { href, children } ) => { ); }; +function ContentOnlyControls( { + attributes, + setAttributes, + isContentOnlyMode, + lockAltControls, + lockAltControlsMessage, + lockTitleControls, + lockTitleControlsMessage, +} ) { + const attributesToDisplayControls = [ 'alt', 'title' ]; + const [ activeControls, setActiveControls ] = useState( () => + attributesToDisplayControls.filter( ( key ) => { + return !! attributes[ key ]; + } ) + ); + const previousActiveControls = usePrevious( activeControls ); + if ( ! isContentOnlyMode ) { + return null; + } + + // Display controls in toolbar only if they have a value set or were selected from the dropdown. + let controls; + if ( !! activeControls.length ) { + controls = ( + + { activeControls.includes( 'alt' ) && ( + ( + { + if ( ! isOpen && event.keyCode === DOWN ) { + event.preventDefault(); + onToggle(); + } + } } + > + { _x( + 'Alternative text', + 'Alternative text for an image. Block toolbar label, a low character count is preferred.' + ) } + + ) } + renderContent={ () => ( + + setAttributes( { alt: value } ) + } + disabled={ lockAltControls } + help={ + lockAltControls ? ( + <>{ lockAltControlsMessage } + ) : ( + <> + + { __( + 'Describe the purpose of the image.' + ) } + +
+ { __( + 'Leave empty if decorative.' + ) } + + ) + } + __nextHasNoMarginBottom + /> + ) } + /> + ) } + { activeControls.includes( 'title' ) && ( + ( + { + if ( ! isOpen && event.keyCode === DOWN ) { + event.preventDefault(); + onToggle(); + } + } } + > + { __( 'Title' ) } + + ) } + renderContent={ () => ( + + setAttributes( { title: value } ) + } + disabled={ lockTitleControls } + help={ + lockTitleControls ? ( + <>{ lockTitleControlsMessage } + ) : ( + <> + { __( + 'Describe the role of this image on the page.' + ) } + + { __( + '(Note: many devices and browsers do not display this text.)' + ) } + + + ) + } + /> + ) } + /> + ) } + + ); + } + + return ( + // Add some extra controls for content attributes when content only mode is active. + // With content only mode active, the inspector is hidden, so users need another way + // to edit these attributes. + <> + { controls } + + + { ( toggleProps ) => ( + + { ( { onClose } ) => ( + <> + { + setActiveControls( ( prev ) => [ + ...prev, + 'alt', + ] ); + onClose(); + } } + disabled={ activeControls.includes( + 'alt' + ) } + > + { _x( + 'Alternative text', + 'Alternative text for an image. Block toolbar label, a low character count is preferred.' + ) } + + { + setActiveControls( ( prev ) => [ + ...prev, + 'title', + ] ); + onClose(); + } } + disabled={ activeControls.includes( + 'title' + ) } + > + { __( 'Title text' ) } + + + ) } + + ) } + + + + ); +} + export default function Image( { temporaryURL, attributes, @@ -621,118 +839,15 @@ export default function Image( { ) } - { isContentOnlyMode && ( - // Add some extra controls for content attributes when content only mode is active. - // With content only mode active, the inspector is hidden, so users need another way - // to edit these attributes. - - ( - { - if ( ! isOpen && event.keyCode === DOWN ) { - event.preventDefault(); - onToggle(); - } - } } - > - { _x( - 'Alternative text', - 'Alternative text for an image. Block toolbar label, a low character count is preferred.' - ) } - - ) } - renderContent={ () => ( - { lockAltControlsMessage } - ) : ( - <> - - { __( - 'Describe the purpose of the image.' - ) } - -
- { __( - 'Leave empty if decorative.' - ) } - - ) - } - __nextHasNoMarginBottom - /> - ) } - /> - { title && ( - ( - { - if ( - ! isOpen && - event.keyCode === DOWN - ) { - event.preventDefault(); - onToggle(); - } - } } - > - { __( 'Title' ) } - - ) } - renderContent={ () => ( - { lockTitleControlsMessage } - ) : ( - <> - { __( - 'Describe the role of this image on the page.' - ) } - - { __( - '(Note: many devices and browsers do not display this text.)' - ) } - - - ) - } - /> - ) } - /> - ) } - - ) } + Date: Thu, 31 Oct 2024 14:50:21 +0200 Subject: [PATCH 2/6] update design --- packages/block-library/src/image/editor.scss | 4 + packages/block-library/src/image/image.js | 326 ++++++++----------- 2 files changed, 136 insertions(+), 194 deletions(-) diff --git a/packages/block-library/src/image/editor.scss b/packages/block-library/src/image/editor.scss index 34f65d690d3d7..35b05a063c299 100644 --- a/packages/block-library/src/image/editor.scss +++ b/packages/block-library/src/image/editor.scss @@ -159,6 +159,10 @@ figure.wp-block-image:not(.wp-block) { } } +.wp-block-image__toolbar_content_textarea__container { + padding: $grid-unit; +} + .wp-block-image__toolbar_content_textarea { // Corresponds to the size of the textarea in the block inspector. width: 250px; diff --git a/packages/block-library/src/image/image.js b/packages/block-library/src/image/image.js index 3118f8009a30c..2dff24c70b5d1 100644 --- a/packages/block-library/src/image/image.js +++ b/packages/block-library/src/image/image.js @@ -10,7 +10,6 @@ import { TextControl, ToolbarButton, ToolbarGroup, - Dropdown, __experimentalToolsPanel as ToolsPanel, __experimentalToolsPanelItem as ToolsPanelItem, __experimentalUseCustomUnits as useCustomUnits, @@ -18,8 +17,9 @@ import { MenuItem, ToolbarItem, DropdownMenu, + Popover, } from '@wordpress/components'; -import { useViewportMatch, usePrevious } from '@wordpress/compose'; +import { useViewportMatch } from '@wordpress/compose'; import { useSelect, useDispatch } from '@wordpress/data'; import { BlockControls, @@ -35,7 +35,6 @@ import { } from '@wordpress/block-editor'; import { useEffect, useMemo, useState, useRef } from '@wordpress/element'; import { __, _x, sprintf, isRTL } from '@wordpress/i18n'; -import { DOWN } from '@wordpress/keycodes'; import { getFilename } from '@wordpress/url'; import { getBlockBindingsSource, switchToBlockType } from '@wordpress/blocks'; import { crop, overlayText, upload, chevronDown } from '@wordpress/icons'; @@ -75,7 +74,6 @@ const scaleOptions = [ const WRITEMODE_POPOVER_PROPS = { placement: 'bottom-start', }; -const WRITEMODE_CONTROLS_POPOVER_PROPS = { position: 'bottom right' }; // If the image has a href, wrap in an tag to trigger any inherited link element styles. const ImageWrapper = ( { href, children } ) => { @@ -111,204 +109,144 @@ function ContentOnlyControls( { lockTitleControls, lockTitleControlsMessage, } ) { - const attributesToDisplayControls = [ 'alt', 'title' ]; - const [ activeControls, setActiveControls ] = useState( () => - attributesToDisplayControls.filter( ( key ) => { - return !! attributes[ key ]; - } ) - ); - const previousActiveControls = usePrevious( activeControls ); + // Use internal state instead of a ref to make sure that the component + // re-renders when the popover's anchor updates. + const [ popoverAnchor, setPopoverAnchor ] = useState( null ); + const [ isAltDialogOpen, setIsAltDialogOpen ] = useState( false ); + const [ isTitleDialogOpen, setIsTitleDialogOpen ] = useState( false ); if ( ! isContentOnlyMode ) { return null; } - - // Display controls in toolbar only if they have a value set or were selected from the dropdown. - let controls; - if ( !! activeControls.length ) { - controls = ( - - { activeControls.includes( 'alt' ) && ( - ( - { - if ( ! isOpen && event.keyCode === DOWN ) { - event.preventDefault(); - onToggle(); - } - } } - > - { _x( - 'Alternative text', - 'Alternative text for an image. Block toolbar label, a low character count is preferred.' - ) } - - ) } - renderContent={ () => ( - - setAttributes( { alt: value } ) - } - disabled={ lockAltControls } - help={ - lockAltControls ? ( - <>{ lockAltControlsMessage } - ) : ( - <> - - { __( - 'Describe the purpose of the image.' - ) } - -
- { __( - 'Leave empty if decorative.' - ) } - - ) - } - __nextHasNoMarginBottom - /> - ) } - /> - ) } - { activeControls.includes( 'title' ) && ( - ( - { - if ( ! isOpen && event.keyCode === DOWN ) { - event.preventDefault(); - onToggle(); - } - } } - > - { __( 'Title' ) } - - ) } - renderContent={ () => ( - - setAttributes( { title: value } ) - } - disabled={ lockTitleControls } - help={ - lockTitleControls ? ( - <>{ lockTitleControlsMessage } - ) : ( - <> - { __( - 'Describe the role of this image on the page.' - ) } - - { __( - '(Note: many devices and browsers do not display this text.)' - ) } - - - ) - } - /> - ) } - /> - ) } - - ); - } - return ( // Add some extra controls for content attributes when content only mode is active. // With content only mode active, the inspector is hidden, so users need another way // to edit these attributes. - <> - { controls } - - - { ( toggleProps ) => ( - - { ( { onClose } ) => ( - <> - { - setActiveControls( ( prev ) => [ - ...prev, - 'alt', - ] ); - onClose(); - } } - disabled={ activeControls.includes( - 'alt' - ) } - > - { _x( - 'Alternative text', - 'Alternative text for an image. Block toolbar label, a low character count is preferred.' - ) } - - { - setActiveControls( ( prev ) => [ - ...prev, - 'title', - ] ); - onClose(); - } } - disabled={ activeControls.includes( - 'title' + + + { ( toggleProps ) => ( + + { ( { onClose } ) => ( + <> + { + setIsAltDialogOpen( true ); + onClose(); + } } + aria-pressed={ !! attributes.alt } + > + { _x( + 'Alternative text', + 'Alternative text for an image. Block toolbar label, a low character count is preferred.' + ) } + + { + setIsTitleDialogOpen( true ); + } } + aria-pressed={ !! attributes.title } + > + { __( 'Title text' ) } + + + ) } + + ) } + + { isAltDialogOpen && ( + setIsAltDialogOpen( false ) } + offset={ 13 } + variant="toolbar" + > +
+ + setAttributes( { alt: value } ) + } + disabled={ lockAltControls } + help={ + lockAltControls ? ( + <>{ lockAltControlsMessage } + ) : ( + <> + + { __( + 'Describe the purpose of the image.' + ) } + +
+ { __( 'Leave empty if decorative.' ) } + + ) + } + __nextHasNoMarginBottom + /> +
+ + ) } + { isTitleDialogOpen && ( + setIsTitleDialogOpen( false ) } + offset={ 13 } + variant="toolbar" + > +
+ + setAttributes( { + title: value, + } ) + } + disabled={ lockTitleControls } + help={ + lockTitleControls ? ( + <>{ lockTitleControlsMessage } + ) : ( + <> + { __( + 'Describe the role of this image on the page.' ) } - > - { __( 'Title text' ) } - - - ) } - - ) } - - - + + { __( + '(Note: many devices and browsers do not display this text.)' + ) } + + + ) + } + /> +
+ + ) } + ); } From 4e4122e7e1398b854369e0ef7be62acfdd274750 Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Thu, 31 Oct 2024 15:56:06 +0200 Subject: [PATCH 3/6] fix tests --- test/e2e/specs/editor/various/pattern-overrides.spec.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/e2e/specs/editor/various/pattern-overrides.spec.js b/test/e2e/specs/editor/various/pattern-overrides.spec.js index 5fbd0e66b5fd0..7ad9d76144da3 100644 --- a/test/e2e/specs/editor/various/pattern-overrides.spec.js +++ b/test/e2e/specs/editor/various/pattern-overrides.spec.js @@ -1131,7 +1131,8 @@ test.describe( 'Pattern Overrides', () => { /\/wp-content\/uploads\// ); await editor.showBlockToolbar(); - await editor.clickBlockToolbarButton( 'Alternative text' ); + await editor.clickBlockToolbarButton( 'More' ); + await page.getByRole( 'button', { name: 'Alternative text' } ).click(); await page .getByRole( 'textbox', { name: 'alternative text' } ) .fill( 'Test Image' ); From 49d5d2280541233ceb19f5733b8906c636331538 Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Thu, 31 Oct 2024 16:47:07 +0200 Subject: [PATCH 4/6] fix e2e test --- test/e2e/specs/editor/various/pattern-overrides.spec.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/e2e/specs/editor/various/pattern-overrides.spec.js b/test/e2e/specs/editor/various/pattern-overrides.spec.js index 7ad9d76144da3..6f4a592930052 100644 --- a/test/e2e/specs/editor/various/pattern-overrides.spec.js +++ b/test/e2e/specs/editor/various/pattern-overrides.spec.js @@ -1132,7 +1132,9 @@ test.describe( 'Pattern Overrides', () => { ); await editor.showBlockToolbar(); await editor.clickBlockToolbarButton( 'More' ); - await page.getByRole( 'button', { name: 'Alternative text' } ).click(); + await page + .getByRole( 'menuitem', { name: 'Alternative text' } ) + .click(); await page .getByRole( 'textbox', { name: 'alternative text' } ) .fill( 'Test Image' ); From bd10056d1caa97b3f0eb0b92514c04cbff9ab9c3 Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Thu, 31 Oct 2024 17:08:25 +0200 Subject: [PATCH 5/6] keep state inside BlockControls --- packages/block-library/src/image/image.js | 35 +++++++++++------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/packages/block-library/src/image/image.js b/packages/block-library/src/image/image.js index 2dff24c70b5d1..90334ffbaa7bd 100644 --- a/packages/block-library/src/image/image.js +++ b/packages/block-library/src/image/image.js @@ -103,7 +103,6 @@ const ImageWrapper = ( { href, children } ) => { function ContentOnlyControls( { attributes, setAttributes, - isContentOnlyMode, lockAltControls, lockAltControlsMessage, lockTitleControls, @@ -114,14 +113,8 @@ function ContentOnlyControls( { const [ popoverAnchor, setPopoverAnchor ] = useState( null ); const [ isAltDialogOpen, setIsAltDialogOpen ] = useState( false ); const [ isTitleDialogOpen, setIsTitleDialogOpen ] = useState( false ); - if ( ! isContentOnlyMode ) { - return null; - } return ( - // Add some extra controls for content attributes when content only mode is active. - // With content only mode active, the inspector is hidden, so users need another way - // to edit these attributes. - + <> { ( toggleProps ) => ( ) } - + ); } @@ -777,15 +770,21 @@ export default function Image( { ) } - + { isContentOnlyMode && ( + // Add some extra controls for content attributes when content only mode is active. + // With content only mode active, the inspector is hidden, so users need another way + // to edit these attributes. + + + + ) } Date: Fri, 1 Nov 2024 11:45:42 +0200 Subject: [PATCH 6/6] remove pressed status --- packages/block-library/src/image/image.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/block-library/src/image/image.js b/packages/block-library/src/image/image.js index 90334ffbaa7bd..a8d6595163552 100644 --- a/packages/block-library/src/image/image.js +++ b/packages/block-library/src/image/image.js @@ -134,7 +134,6 @@ function ContentOnlyControls( { setIsAltDialogOpen( true ); onClose(); } } - aria-pressed={ !! attributes.alt } > { _x( 'Alternative text', @@ -144,8 +143,8 @@ function ContentOnlyControls( { { setIsTitleDialogOpen( true ); + onClose(); } } - aria-pressed={ !! attributes.title } > { __( 'Title text' ) }