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: Reflect media deletion in the editor #41220

Closed
Closed
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export function MediaPlaceholder( {
onError,
onSelect,
onCancel,
onClose = noop,
onSelectURL,
onDoubleClick,
onFilesPreUpload = noop,
Expand Down Expand Up @@ -327,6 +328,7 @@ export function MediaPlaceholder( {
gallery={ multiple && onlyAllowsImages() }
multiple={ multiple }
onSelect={ onSelect }
onClose={ onClose }
allowedTypes={ allowedTypes }
mode={ 'browse' }
value={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const MediaReplaceFlow = ( {
onError,
onSelect,
onSelectURL,
onCloseModal = noop,
onToggleFeaturedImage,
useFeaturedImage,
onFilesUpload = noop,
Expand All @@ -58,6 +59,7 @@ const MediaReplaceFlow = ( {
multiple = false,
addToGallery,
handleUpload = true,
buttonVariant,
} ) => {
const [ mediaURLValue, setMediaURLValue ] = useState( mediaURL );
const mediaUpload = useSelect( ( select ) => {
Expand Down Expand Up @@ -153,6 +155,7 @@ const MediaReplaceFlow = ( {
aria-haspopup="true"
onClick={ onToggle }
onKeyDown={ openOnArrowDown }
variant={ buttonVariant }
>
{ name }
</ToolbarButton>
Expand All @@ -170,6 +173,7 @@ const MediaReplaceFlow = ( {
selectMedia( media, onClose )
}
allowedTypes={ allowedTypes }
onClose={ onCloseModal }
render={ ( { open } ) => (
<MenuItem
icon={ mediaIcon }
Expand Down
169 changes: 155 additions & 14 deletions packages/block-library/src/image/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,19 @@ import { get, has, omit, pick } from 'lodash';
* WordPress dependencies
*/
import { getBlobByURL, isBlobURL, revokeBlobURL } from '@wordpress/blob';
import { withNotices } from '@wordpress/components';
import {
Button,
withNotices,
Placeholder,
__experimentalHStack as HStack,
} from '@wordpress/components';
import { useSelect } from '@wordpress/data';
import {
BlockAlignmentControl,
BlockControls,
BlockIcon,
MediaPlaceholder,
MediaReplaceFlow,
useBlockProps,
store as blockEditorStore,
} from '@wordpress/block-editor';
Expand All @@ -26,6 +32,7 @@ import { image as icon } from '@wordpress/icons';
* Internal dependencies
*/
import Image from './image';
import { isMediaFileDeleted } from './utils';

/**
* Module constants
Expand Down Expand Up @@ -85,6 +92,84 @@ function hasDefaultSize( image, defaultSize ) {
);
}

/**
* Checks if a media attachment object has been "destroyed",
* that is, removed from the media library. The core Media Library
* adds a `destroyed` property to a deleted attachment object in the media collection.
*
* @param {number} id The attachment id.
*
* @return {boolean} Whether the image has been destroyed.
*/
export function isMediaDestroyed( id ) {
const attachment = window?.wp?.media?.attachment( id ) || {};
return attachment.destroyed;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we force the return here to be boolean matching the JSDoc return type?

Suggested change
return attachment.destroyed;
return !! attachment.destroyed;

}

/**
* A placeholder component that displays if an image has been deleted or cannot be loaded.
*
* @param {Object} props Component props.
* @param {number} props.id The attachment id.
* @param {string} props.url Image url.
* @param {Function} props.onClear Clears the image attributes.
* @param {Function} props.onSelect Callback for <MediaReplaceFlow /> when an image is selected.
* @param {Function} props.onSelectURL Callback for <MediaReplaceFlow /> when an image URL is selected.
* @param {Function} props.onUploadError Callback for <MediaReplaceFlow /> when an upload fails.
* @param {Function} props.onCloseModal Callback for <MediaReplaceFlow /> when the media library overlay is closed.
*
* @return {boolean} Whether the image has been destroyed.
*/
const ImageErrorPlaceholder = ( {
id,
url,
onClear,
onSelect,
onSelectURL,
onUploadError,
onCloseModal,
} ) => (
<Placeholder
className="wp-block-image__error-placeholder"
icon={ icon }
label={ __( 'This image could not be loaded' ) }
>
<p>{ url }</p>
<p>
{ __(
'This might be due to a network error, or the image may have been deleted.'
) }
</p>
<HStack justify="flex-start" spacing={ 2 }>
<MediaReplaceFlow
mediaId={ id }
mediaURL={ url }
allowedTypes={ ALLOWED_MEDIA_TYPES }
accept="image/*"
onSelect={ ( media ) => {
onClear();
onSelect( media );
} }
onSelectURL={ ( selectedUrl ) => {
onClear();
onSelectURL( selectedUrl );
} }
onError={ onUploadError }
onCloseModal={ onCloseModal }
buttonVariant="primary"
/>
<Button
className="wp-block-image__error-placeholder__button"
title={ __( 'Clear and replace image' ) }
variant="secondary"
onClick={ onClear }
>
{ __( 'Clear' ) }
</Button>
</HStack>
</Placeholder>
);

export function ImageEdit( {
attributes,
setAttributes,
Expand All @@ -108,6 +193,7 @@ export function ImageEdit( {
sizeSlug,
} = attributes;
const [ temporaryURL, setTemporaryURL ] = useState();
const [ hasImageLoadError, setHasImageLoadError ] = useState( false );

const altRef = useRef();
useEffect( () => {
Expand All @@ -125,6 +211,45 @@ export function ImageEdit( {
return pick( getSettings(), [ 'imageDefaultSize', 'mediaUpload' ] );
}, [] );

// A callback passed to MediaUpload,
// fired when the media modal closes.
function onCloseModal() {
if ( isMediaDestroyed( attributes?.id ) ) {
setHasImageLoadError( true );
}
}

function clearImageAttributes() {
setHasImageLoadError( false );
setAttributes( {
src: undefined,
id: undefined,
url: undefined,
} );
}

/**
* Runs an error callback if the image does not load.
* If the error callback is triggered, we infer that that image
* has been deleted.
*
* @param {boolean} isReplaced Whether the image has been replaced.
*/
function onImageError( isReplaced = false ) {
// If the image block was not replaced with an embed,
// and it's not an external image,
// and it's been deleted from the database,
// clear the attributes and trigger an error placeholder.
if ( id && ! isReplaced && ! isExternalImage( id, url ) ) {
isMediaFileDeleted( id ).then( ( isFileDeleted ) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm wondering if this will solve the reported issue in #41161 at all... if the image triggers the error handler, it could be due to a network error in which case the fetch request would fail.

🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a situation we could mock within some unit tests for the image block?

if ( isFileDeleted ) {
noticeOperations.removeAllNotices();
setHasImageLoadError( true );
}
} );
}
}

function onUploadError( message ) {
noticeOperations.removeAllNotices();
noticeOperations.createErrorNotice( message );
Expand Down Expand Up @@ -309,7 +434,18 @@ export function ImageEdit( {

return (
<figure { ...blockProps }>
{ ( temporaryURL || url ) && (
{ hasImageLoadError && (
<ImageErrorPlaceholder
id={ id }
url={ url }
onClear={ clearImageAttributes }
onSelect={ onSelectImage }
onSelectURL={ onSelectURL }
onUploadError={ onUploadError }
onCloseModal={ onCloseModal }
/>
) }
{ ! hasImageLoadError && ( temporaryURL || url ) && (
<Image
temporaryURL={ temporaryURL }
attributes={ attributes }
Expand All @@ -323,6 +459,8 @@ export function ImageEdit( {
containerRef={ ref }
context={ context }
clientId={ clientId }
onCloseModal={ onCloseModal }
onImageLoadError={ onImageError }
/>
) }
{ ! url && (
Expand All @@ -333,18 +471,21 @@ export function ImageEdit( {
/>
</BlockControls>
) }
<MediaPlaceholder
icon={ <BlockIcon icon={ icon } /> }
onSelect={ onSelectImage }
onSelectURL={ onSelectURL }
notices={ noticeUI }
onError={ onUploadError }
accept="image/*"
allowedTypes={ ALLOWED_MEDIA_TYPES }
value={ { id, src } }
mediaPreview={ mediaPreview }
disableMediaButtons={ temporaryURL || url }
/>
{ ! hasImageLoadError && (
<MediaPlaceholder
icon={ <BlockIcon icon={ icon } /> }
onSelect={ onSelectImage }
onSelectURL={ onSelectURL }
notices={ noticeUI }
onError={ onUploadError }
onClose={ onCloseModal }
accept="image/*"
allowedTypes={ ALLOWED_MEDIA_TYPES }
value={ { id, src } }
mediaPreview={ mediaPreview }
disableMediaButtons={ temporaryURL || url }
/>
) }
</figure>
);
}
Expand Down
4 changes: 4 additions & 0 deletions packages/block-library/src/image/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,7 @@ figure.wp-block-image:not(.wp-block) {
padding-right: 0;
}
}

.wp-block-image__error-placeholder .wp-block-image__error-placeholder__button {
margin: 0 0 0 $grid-unit-10;
}
13 changes: 11 additions & 2 deletions packages/block-library/src/image/image.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ import { store as coreStore } from '@wordpress/core-data';
*/
import { createUpgradedEmbedBlock } from '../embed/util';
import useClientWidth from './use-client-width';
import { isExternalImage } from './edit';
import { isExternalImage, isMediaDestroyed } from './edit';

/**
* Module constants
Expand Down Expand Up @@ -76,9 +76,11 @@ export default function Image( {
isSelected,
insertBlocksAfter,
onReplace,
onCloseModal,
onSelectImage,
onSelectURL,
onUploadError,
onImageLoadError,
containerRef,
context,
clientId,
Expand Down Expand Up @@ -216,10 +218,13 @@ export default function Image( {
// Check if there's an embed block that handles this URL, e.g., instagram URL.
// See: https://github.com/WordPress/gutenberg/pull/11472
const embedBlock = createUpgradedEmbedBlock( { attributes: { url } } );
const shouldReplace = undefined !== embedBlock;

if ( undefined !== embedBlock ) {
if ( shouldReplace ) {
onReplace( embedBlock );
}

onImageLoadError( shouldReplace );
}

function onSetHref( props ) {
Expand Down Expand Up @@ -291,6 +296,9 @@ export default function Image( {
if ( ! isSelected ) {
setIsEditingImage( false );
}
if ( isSelected && isMediaDestroyed( id ) ) {
onImageLoadError();
}
}, [ isSelected ] );

const canEditImage = id && naturalWidth && naturalHeight && imageEditing;
Expand Down Expand Up @@ -354,6 +362,7 @@ export default function Image( {
onSelect={ onSelectImage }
onSelectURL={ onSelectURL }
onError={ onUploadError }
onCloseModal={ onCloseModal }
/>
</BlockControls>
) }
Expand Down
23 changes: 23 additions & 0 deletions packages/block-library/src/image/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
*/
import { isEmpty, get } from 'lodash';

/**
* WordPress dependencies
*/
import apiFetch from '@wordpress/api-fetch';

/**
* Internal dependencies
*/
Expand Down Expand Up @@ -72,3 +77,21 @@ export function getImageSizeAttributes( image, size ) {

return {};
}

/**
* Performs a GET request on an image file to confirm whether it has been deleted from the database.
*
* @param {number=} mediaId The id of the image.
* @return {Promise} Media Object Promise.
*/
export async function isMediaFileDeleted( mediaId ) {
try {
const response = await apiFetch( {
path: `/wp/v2/media/${ mediaId }`,
} );
const isMediaFileAvailable = response && response?.id === mediaId;
return ! isMediaFileAvailable;
} catch ( err ) {
return true;
}
}