Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Image: Adds the block controls for uploading image. #64320

Merged
merged 16 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 86 additions & 33 deletions packages/block-library/src/image/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,21 @@ import { Placeholder } from '@wordpress/components';
import { useDispatch, useSelect } from '@wordpress/data';
import {
BlockIcon,
MediaPlaceholder,
useBlockProps,
MediaPlaceholder,
store as blockEditorStore,
__experimentalUseBorderProps as useBorderProps,
__experimentalGetShadowClassesAndStyles as getShadowClassesAndStyles,
useBlockEditingMode,
MediaReplaceFlow,
BlockControls,
__experimentalLinkControl as LinkControl,
} from '@wordpress/block-editor';
import { useEffect, useRef, useState } from '@wordpress/element';
import { __, sprintf } from '@wordpress/i18n';
import { image as icon, plugins as pluginsIcon } from '@wordpress/icons';
import { store as noticesStore } from '@wordpress/notices';
import { useResizeObserver } from '@wordpress/compose';

/**
* Internal dependencies
Expand Down Expand Up @@ -107,8 +111,16 @@ export function ImageEdit( {
align,
metadata,
} = attributes;

const [ temporaryURL, setTemporaryURL ] = useState( attributes.blob );

const [ resizeListener, sizes ] = useResizeObserver();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure this one is already included in the Placeholder component, so it may be best to make these width changes to that component itself, rather than locally here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we rely on that existing CSS class? It's fine that every placeholder gets the is-small changes you've made here, even if only the Image block makes use of them for now. Make sense?

Technically, yes. But in the long run, I'm thinking it could be cleaner and more flexible if this kind of responsive styling was handled solely by the consumer of Placeholder, using CSS container queries (rather than a resize observer). That way, consumers wouldn't have to depend on a set of arbitrary breakpoints that Placeholder defines.

I'm not intimately familiar with the requirements of this Image block in particular, but on first glance I would say it could make it simpler and more performant. Could be worth trying here. (This is a non-blocking comment, just suggesting in case it hadn't been considered.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mirka, this would be a good change, but since it applies more generally to the placeholder component, should we make the change in this PR or address it in a separate issue? Related: #64288

Copy link
Contributor

@ciampo ciampo Aug 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with Lena — this seems a good fit for CSS Container Queries

(edit: when I wrote my message, I hadn't seen @vipul0425 's reply, which was posted almost at the same time)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vipul0425 I don't have much insight into the current state of things, but it looks like you were able to achieve what you needed in this PR without resize observers or container queries, so yes it seems like something that can be addressed as a separate issue 👍

const parentWidth = sizes?.width;
const parsedWidth = width && parseInt( width.replace( /px$/, '' ), 10 );

const isSmallLayout =
( parentWidth && parentWidth < 160 ) || parsedWidth < 160;

const altRef = useRef();
useEffect( () => {
altRef.current = alt;
Expand Down Expand Up @@ -329,11 +341,15 @@ export function ImageEdit( {
<Placeholder
className={ clsx( 'block-editor-media-placeholder', {
[ borderProps.className ]:
!! borderProps.className && ! isSingleSelected,
!! borderProps.className &&
! isSingleSelected &&
! isSmallLayout,
} ) }
withIllustration
icon={ lockUrlControls ? pluginsIcon : icon }
label={ __( 'Image' ) }
icon={
! isSmallLayout && ( lockUrlControls ? pluginsIcon : icon )
}
label={ ! isSmallLayout && __( 'Image' ) }
instructions={
! lockUrlControls &&
__(
Expand Down Expand Up @@ -364,35 +380,72 @@ export function ImageEdit( {
};

return (
<figure { ...blockProps }>
<Image
temporaryURL={ temporaryURL }
attributes={ attributes }
setAttributes={ setAttributes }
isSingleSelected={ isSingleSelected }
insertBlocksAfter={ insertBlocksAfter }
onReplace={ onReplace }
onSelectImage={ onSelectImage }
onSelectURL={ onSelectURL }
onUploadError={ onUploadError }
context={ context }
clientId={ clientId }
blockEditingMode={ blockEditingMode }
parentLayoutType={ parentLayout?.type }
/>
<MediaPlaceholder
icon={ <BlockIcon icon={ icon } /> }
onSelect={ onSelectImage }
onSelectURL={ onSelectURL }
onError={ onUploadError }
placeholder={ placeholder }
accept="image/*"
allowedTypes={ ALLOWED_MEDIA_TYPES }
value={ { id, src } }
mediaPreview={ mediaPreview }
disableMediaButtons={ temporaryURL || url }
/>
</figure>
<>
<figure { ...blockProps }>
{ resizeListener }
{ ! ( temporaryURL || url ) && (
<BlockControls group="other">
<MediaReplaceFlow
t-hamano marked this conversation as resolved.
Show resolved Hide resolved
mediaId={ id }
mediaURL={ url }
allowedTypes={ ALLOWED_MEDIA_TYPES }
accept="image/*"
name={ __( 'Add image' ) }
onSelect={ onSelectImage }
onError={ onUploadError }
>
<form className="block-editor-media-flow__url-input has-siblings">
<span className="block-editor-media-replace-flow__image-url-label">
{ __( 'Insert from URL:' ) }
</span>

<LinkControl
value={ { url } }
settings={ [] }
showSuggestions={ false }
onChange={ ( value ) => {
onSelectURL( value.url );
} }
/>
</form>
</MediaReplaceFlow>
</BlockControls>
) }
<Image
temporaryURL={ temporaryURL }
attributes={ attributes }
setAttributes={ setAttributes }
isSingleSelected={ isSingleSelected }
insertBlocksAfter={ insertBlocksAfter }
onReplace={ onReplace }
onSelectImage={ onSelectImage }
onSelectURL={ onSelectURL }
onUploadError={ onUploadError }
context={ context }
clientId={ clientId }
blockEditingMode={ blockEditingMode }
parentLayoutType={ parentLayout?.type }
/>
{ ! temporaryURL &&
! url &&
( isSmallLayout ? (
placeholder( mediaPreview )
) : (
<MediaPlaceholder
icon={ <BlockIcon icon={ icon } /> }
onSelect={ onSelectImage }
onSelectURL={ onSelectURL }
onError={ onUploadError }
placeholder={ placeholder }
accept="image/*"
allowedTypes={ ALLOWED_MEDIA_TYPES }
value={ { id, src } }
mediaPreview={ mediaPreview }
disableMediaButtons={ temporaryURL || url }
/>
) ) }
</figure>
</>
);
}

Expand Down
3 changes: 1 addition & 2 deletions packages/block-library/src/image/editor.scss
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
// Provide special styling for the placeholder.
// @todo this particular minimal style of placeholder could be componentized further.
.wp-block-image.wp-block-image {

// Show Placeholder style on-select.
&.is-selected .block-editor-media-placeholder {
&.is-selected .block-editor-media-placeholder:not(.is-small) {
// Block UI appearance.
color: $gray-900;
background-color: $white;
Expand Down
Loading