From 1cbe5aa463f3a3f88b8f8d7ba0e321ce05f96ca0 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Wed, 27 Oct 2021 11:40:28 +1100 Subject: [PATCH 01/11] initial commit. listening out for changes to the attachments collection and passing to onRemove prop. --- .../src/components/media-placeholder/index.js | 2 ++ packages/block-library/src/image/edit.js | 17 +++++++++++++++++ .../src/components/media-upload/index.js | 16 +++++++++++++++- 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/media-placeholder/index.js b/packages/block-editor/src/components/media-placeholder/index.js index f13206870f815e..9c154dec425091 100644 --- a/packages/block-editor/src/components/media-placeholder/index.js +++ b/packages/block-editor/src/components/media-placeholder/index.js @@ -70,6 +70,7 @@ export function MediaPlaceholder( { disableMediaButtons, onError, onSelect, + onRemove = noop, onCancel, onSelectURL, onDoubleClick, @@ -328,6 +329,7 @@ export function MediaPlaceholder( { gallery={ multiple && onlyAllowsImages() } multiple={ multiple } onSelect={ onSelect } + onRemove={ onRemove } allowedTypes={ allowedTypes } value={ Array.isArray( value ) diff --git a/packages/block-library/src/image/edit.js b/packages/block-library/src/image/edit.js index 7f29a8d1078c51..f6630f23730336 100644 --- a/packages/block-library/src/image/edit.js +++ b/packages/block-library/src/image/edit.js @@ -132,6 +132,22 @@ export function ImageEdit( { noticeOperations.createErrorNotice( message ); } + function onRemoveImage( attachment ) { + if ( + attachment?.id === attributes?.id && + attachment?.destroyed === true + ) { + setAttributes( { + url: undefined, + alt: undefined, + id: undefined, + title: undefined, + caption: undefined, + } ); + setTemporaryURL( undefined ); + } + } + function onSelectImage( media ) { if ( ! media || ! media.url ) { setAttributes( { @@ -337,6 +353,7 @@ export function ImageEdit( { } onSelect={ onSelectImage } + onRemove={ onRemoveImage } onSelectURL={ onSelectURL } notices={ noticeUI } onError={ onUploadError } diff --git a/packages/media-utils/src/components/media-upload/index.js b/packages/media-utils/src/components/media-upload/index.js index a822956dd5b34e..19548e2e9c6073 100644 --- a/packages/media-utils/src/components/media-upload/index.js +++ b/packages/media-utils/src/components/media-upload/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { castArray, defaults, pick } from 'lodash'; +import { castArray, defaults, pick, noop } from 'lodash'; /** * WordPress dependencies @@ -239,6 +239,9 @@ class MediaUpload extends Component { this.onSelect = this.onSelect.bind( this ); this.onUpdate = this.onUpdate.bind( this ); this.onClose = this.onClose.bind( this ); + this.onRemoveSelectedAttachment = this.onRemoveSelectedAttachment.bind( + this + ); if ( gallery ) { this.buildAndSetGalleryFrame(); @@ -270,6 +273,11 @@ class MediaUpload extends Component { this.frame.on( 'update', this.onUpdate ); this.frame.on( 'open', this.onOpen ); this.frame.on( 'close', this.onClose ); + this.frame.listenTo( + wp.media.model.Attachments.all, + 'remove', + this.onRemoveSelectedAttachment + ); } /** @@ -374,6 +382,12 @@ class MediaUpload extends Component { onSelect( multiple ? attachment : attachment[ 0 ] ); } + onRemoveSelectedAttachment( attachment ) { + const { onRemove = noop } = this.props; + console.log( 'onRemoveSelectedAttachment', attachment ); + onRemove( attachment ); + } + onOpen() { this.updateCollection(); From ce2d06de1b24fb7e1049573e4e0ebe732a040f1a Mon Sep 17 00:00:00 2001 From: ramonjd Date: Wed, 27 Oct 2021 21:03:33 +1100 Subject: [PATCH 02/11] Removing console.log --- packages/media-utils/src/components/media-upload/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/media-utils/src/components/media-upload/index.js b/packages/media-utils/src/components/media-upload/index.js index 19548e2e9c6073..039732da4e7805 100644 --- a/packages/media-utils/src/components/media-upload/index.js +++ b/packages/media-utils/src/components/media-upload/index.js @@ -384,7 +384,6 @@ class MediaUpload extends Component { onRemoveSelectedAttachment( attachment ) { const { onRemove = noop } = this.props; - console.log( 'onRemoveSelectedAttachment', attachment ); onRemove( attachment ); } From 5318480514c2b082d573efb646a25d782e4548fc Mon Sep 17 00:00:00 2001 From: ramonjd Date: Wed, 27 Oct 2021 21:05:00 +1100 Subject: [PATCH 03/11] Reverting component changes --- .../src/components/media-placeholder/index.js | 2 -- packages/block-library/src/image/edit.js | 17 ----------------- 2 files changed, 19 deletions(-) diff --git a/packages/block-editor/src/components/media-placeholder/index.js b/packages/block-editor/src/components/media-placeholder/index.js index 9c154dec425091..f13206870f815e 100644 --- a/packages/block-editor/src/components/media-placeholder/index.js +++ b/packages/block-editor/src/components/media-placeholder/index.js @@ -70,7 +70,6 @@ export function MediaPlaceholder( { disableMediaButtons, onError, onSelect, - onRemove = noop, onCancel, onSelectURL, onDoubleClick, @@ -329,7 +328,6 @@ export function MediaPlaceholder( { gallery={ multiple && onlyAllowsImages() } multiple={ multiple } onSelect={ onSelect } - onRemove={ onRemove } allowedTypes={ allowedTypes } value={ Array.isArray( value ) diff --git a/packages/block-library/src/image/edit.js b/packages/block-library/src/image/edit.js index f6630f23730336..7f29a8d1078c51 100644 --- a/packages/block-library/src/image/edit.js +++ b/packages/block-library/src/image/edit.js @@ -132,22 +132,6 @@ export function ImageEdit( { noticeOperations.createErrorNotice( message ); } - function onRemoveImage( attachment ) { - if ( - attachment?.id === attributes?.id && - attachment?.destroyed === true - ) { - setAttributes( { - url: undefined, - alt: undefined, - id: undefined, - title: undefined, - caption: undefined, - } ); - setTemporaryURL( undefined ); - } - } - function onSelectImage( media ) { if ( ! media || ! media.url ) { setAttributes( { @@ -353,7 +337,6 @@ export function ImageEdit( { } onSelect={ onSelectImage } - onRemove={ onRemoveImage } onSelectURL={ onSelectURL } notices={ noticeUI } onError={ onUploadError } From 1e9992b139469ca200b46077bcb5a42657c63bfb Mon Sep 17 00:00:00 2001 From: ramonjd Date: Wed, 27 Oct 2021 22:17:31 +1100 Subject: [PATCH 04/11] Adding a store to media utils --- packages/block-library/src/image/edit.js | 21 +++++++++++++++ .../src/components/media-upload/index.js | 18 ++++++++++--- packages/media-utils/src/index.js | 1 + packages/media-utils/src/store/actions.js | 14 ++++++++++ packages/media-utils/src/store/index.js | 26 +++++++++++++++++++ packages/media-utils/src/store/reducer.js | 25 ++++++++++++++++++ packages/media-utils/src/store/selectors.js | 18 +++++++++++++ 7 files changed, 120 insertions(+), 3 deletions(-) create mode 100644 packages/media-utils/src/store/actions.js create mode 100644 packages/media-utils/src/store/index.js create mode 100644 packages/media-utils/src/store/reducer.js create mode 100644 packages/media-utils/src/store/selectors.js diff --git a/packages/block-library/src/image/edit.js b/packages/block-library/src/image/edit.js index 7f29a8d1078c51..e27369e2b5e2c6 100644 --- a/packages/block-library/src/image/edit.js +++ b/packages/block-library/src/image/edit.js @@ -21,6 +21,7 @@ import { import { useEffect, useRef, useState } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { image as icon } from '@wordpress/icons'; +import { store as mediaUtilsStore } from '@wordpress/media-utils'; /* global wp */ @@ -124,9 +125,29 @@ export function ImageEdit( { const ref = useRef(); const { imageDefaultSize, mediaUpload } = useSelect( ( select ) => { const { getSettings } = select( blockEditorStore ); + const { getDeletedAttachment } = select( mediaUtilsStore ); return pick( getSettings(), [ 'imageDefaultSize', 'mediaUpload' ] ); }, [] ); + const { isImageDeleted } = useSelect( ( select ) => { + const { getDeletedAttachment } = select( mediaUtilsStore ); + console.log( 'useSelect', getDeletedAttachment( id ) ); + return !! getDeletedAttachment( id ); + }, [ id ] ); + + useEffect( () => { + if ( isImageDeleted === true ) { + console.log( 'useEffect', isImageDeleted ); + setAttributes( { + url: undefined, + alt: undefined, + id: undefined, + title: undefined, + caption: undefined, + } ); + } + }, [ isImageDeleted ] ); + function onUploadError( message ) { noticeOperations.removeAllNotices(); noticeOperations.createErrorNotice( message ); diff --git a/packages/media-utils/src/components/media-upload/index.js b/packages/media-utils/src/components/media-upload/index.js index 039732da4e7805..656d0fa44237ae 100644 --- a/packages/media-utils/src/components/media-upload/index.js +++ b/packages/media-utils/src/components/media-upload/index.js @@ -8,9 +8,14 @@ import { castArray, defaults, pick, noop } from 'lodash'; */ import { Component } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; - +import { useDispatch } from '@wordpress/data'; const { wp } = window; +/** + * Internal dependencies + */ +import { store as mediaUtilsStore } from '../../store'; + const DEFAULT_EMPTY_GALLERY = []; /** @@ -383,8 +388,15 @@ class MediaUpload extends Component { } onRemoveSelectedAttachment( attachment ) { - const { onRemove = noop } = this.props; - onRemove( attachment ); + if ( attachment.destroyed ) { + const { onRemove = noop } = this.props; + // @TODO We can't dispatch outside the body of a function component. + // Maybe we should turn this into a function component first. + const { removeAttachment } = useDispatch( mediaUtilsStore ); + console.log( 'onRemoveSelectedAttachment', attachment ); + removeAttachment( attachment ); + onRemove( attachment ); + } } onOpen() { diff --git a/packages/media-utils/src/index.js b/packages/media-utils/src/index.js index 590a7f4c9d188d..4e1ed4aac6b170 100644 --- a/packages/media-utils/src/index.js +++ b/packages/media-utils/src/index.js @@ -1,2 +1,3 @@ export * from './components'; export * from './utils'; +export { store } from './store'; diff --git a/packages/media-utils/src/store/actions.js b/packages/media-utils/src/store/actions.js new file mode 100644 index 00000000000000..384b31f2a9b54d --- /dev/null +++ b/packages/media-utils/src/store/actions.js @@ -0,0 +1,14 @@ +/** + * Returns an action object used in signalling that an attachment, e.g., an image, is to be removed. + * + * @param {string} id Attachment unique identifier. + * + * @return {Object} Action object. + */ +export function removeAttachment( attachment ) { + console.log( 'removeAttachment', attachment ); + return { + type: 'REMOVE_ATTACHMENT', + attachment, + }; +} diff --git a/packages/media-utils/src/store/index.js b/packages/media-utils/src/store/index.js new file mode 100644 index 00000000000000..cf17263fc472b5 --- /dev/null +++ b/packages/media-utils/src/store/index.js @@ -0,0 +1,26 @@ +/** + * WordPress dependencies + */ +import { createReduxStore, register } from '@wordpress/data'; + +/** + * Internal dependencies + */ +import reducer from './reducer'; +import * as actions from './actions'; +import * as selectors from './selectors'; + +/** + * Store definition for the notices namespace. + * + * @see https://github.com/WordPress/gutenberg/blob/HEAD/packages/data/README.md#createReduxStore + * + * @type {Object} + */ +export const store = createReduxStore( 'core/media-utils', { + reducer, + actions, + selectors, +} ); + +register( store ); diff --git a/packages/media-utils/src/store/reducer.js b/packages/media-utils/src/store/reducer.js new file mode 100644 index 00000000000000..a7dbcd7ceffa47 --- /dev/null +++ b/packages/media-utils/src/store/reducer.js @@ -0,0 +1,25 @@ +/** + * WordPress dependencies + */ +import { combineReducers } from '@wordpress/data'; + +/** + * Reducer that tracks which attachments are in a guide. Each guide is represented by + * an array which contains the tip identifiers contained within that guide. + * + * @param {Array} state Current state. + * @param {Object} action Dispatched action. + * + * @return {Array} Updated state. + */ +export function deletedAttachments( state = [], action ) { + console.log( 'deletedAttachments', state, action ); + switch ( action.type ) { + case 'REMOVE_ATTACHMENT': + return [ ...state, action.attachment ]; + } + + return state; +} + +export default combineReducers( { deletedAttachments } ); diff --git a/packages/media-utils/src/store/selectors.js b/packages/media-utils/src/store/selectors.js new file mode 100644 index 00000000000000..e4e443b48b9ce8 --- /dev/null +++ b/packages/media-utils/src/store/selectors.js @@ -0,0 +1,18 @@ +/** + * External dependencies + */ +import createSelector from 'rememo'; +import { find } from 'lodash'; + +/** + * Returns an object describing the deleted attachment, otherwise null. + * + * @param {Object} state Global application state. + * @param {string} id The id to query. + * + * @return {?Object} Information about the associated guide. + */ +export const getDeletedAttachment = createSelector( + ( state, id ) => find( state.deletedAttachments, [ 'id', id ] ) ?? null, + ( state ) => [ state.deletedAttachments ] +); From 2f4a0d89df96d481fa5ad035171b194ff8480a3e Mon Sep 17 00:00:00 2001 From: ramonjd Date: Mon, 15 Nov 2021 13:41:48 +1100 Subject: [PATCH 05/11] Rolling back historical experimental commits. Now relying on the `onClose` event of the media upload, and the media object to test if the media has been "destroyed." --- .../src/components/media-placeholder/index.js | 2 + .../components/media-replace-flow/index.js | 2 + packages/block-library/src/image/edit.js | 77 +++++++++++++------ packages/block-library/src/image/image.js | 2 + .../src/components/media-upload/index.js | 29 +------ packages/media-utils/src/index.js | 1 - packages/media-utils/src/store/actions.js | 14 ---- packages/media-utils/src/store/index.js | 26 ------- packages/media-utils/src/store/reducer.js | 25 ------ packages/media-utils/src/store/selectors.js | 18 ----- 10 files changed, 60 insertions(+), 136 deletions(-) delete mode 100644 packages/media-utils/src/store/actions.js delete mode 100644 packages/media-utils/src/store/index.js delete mode 100644 packages/media-utils/src/store/reducer.js delete mode 100644 packages/media-utils/src/store/selectors.js diff --git a/packages/block-editor/src/components/media-placeholder/index.js b/packages/block-editor/src/components/media-placeholder/index.js index f13206870f815e..123c8c5b0070ff 100644 --- a/packages/block-editor/src/components/media-placeholder/index.js +++ b/packages/block-editor/src/components/media-placeholder/index.js @@ -75,6 +75,7 @@ export function MediaPlaceholder( { onDoubleClick, onFilesPreUpload = noop, onHTMLDrop = noop, + onClose = noop, children, mediaLibraryButton, placeholder, @@ -328,6 +329,7 @@ export function MediaPlaceholder( { gallery={ multiple && onlyAllowsImages() } multiple={ multiple } onSelect={ onSelect } + onClose={ onClose } allowedTypes={ allowedTypes } value={ Array.isArray( value ) diff --git a/packages/block-editor/src/components/media-replace-flow/index.js b/packages/block-editor/src/components/media-replace-flow/index.js index 3cd8e0f1755ae7..160019ea529998 100644 --- a/packages/block-editor/src/components/media-replace-flow/index.js +++ b/packages/block-editor/src/components/media-replace-flow/index.js @@ -39,6 +39,7 @@ const MediaReplaceFlow = ( { onSelect, onSelectURL, onFilesUpload = noop, + onCloseModal = noop, name = __( 'Replace' ), createNotice, removeNotice, @@ -136,6 +137,7 @@ const MediaReplaceFlow = ( { value={ mediaId } onSelect={ ( media ) => selectMedia( media ) } allowedTypes={ allowedTypes } + onClose={ onCloseModal } render={ ( { open } ) => ( { __( 'Open Media Library' ) } diff --git a/packages/block-library/src/image/edit.js b/packages/block-library/src/image/edit.js index e27369e2b5e2c6..f0aee5276dbc08 100644 --- a/packages/block-library/src/image/edit.js +++ b/packages/block-library/src/image/edit.js @@ -21,7 +21,7 @@ import { import { useEffect, useRef, useState } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { image as icon } from '@wordpress/icons'; -import { store as mediaUtilsStore } from '@wordpress/media-utils'; +import { store as coreStore } from '@wordpress/core-data'; /* global wp */ @@ -88,6 +88,20 @@ function hasDefaultSize( image, defaultSize ) { ); } +/** + * Checks if a media attachment object has been "destroyed", + * that is, removed from the media library. The core Media Library + * add 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. + */ +function isMediaDestroyed( id ) { + const attachment = wp?.media?.attachment( id ) || {}; + return attachment.destroyed; +} + export function ImageEdit( { attributes, setAttributes, @@ -125,28 +139,46 @@ export function ImageEdit( { const ref = useRef(); const { imageDefaultSize, mediaUpload } = useSelect( ( select ) => { const { getSettings } = select( blockEditorStore ); - const { getDeletedAttachment } = select( mediaUtilsStore ); return pick( getSettings(), [ 'imageDefaultSize', 'mediaUpload' ] ); }, [] ); - const { isImageDeleted } = useSelect( ( select ) => { - const { getDeletedAttachment } = select( mediaUtilsStore ); - console.log( 'useSelect', getDeletedAttachment( id ) ); - return !! getDeletedAttachment( id ); - }, [ id ] ); + const media = useSelect( + ( select ) => { + return select( coreStore ).getMedia( id ); + }, + [ id ] + ); + // Also check for destroyed media when the image is selected, + // If the image is used elsewhere in a post, + // the onCloseModal callback won't fire. + // Also check on media object itself in case + // the page had reloaded and the media attachment collection state no longer exists. useEffect( () => { - if ( isImageDeleted === true ) { - console.log( 'useEffect', isImageDeleted ); - setAttributes( { - url: undefined, - alt: undefined, - id: undefined, - title: undefined, - caption: undefined, - } ); + if ( isSelected ) { + if ( isMediaDestroyed( attributes?.id ) || ! media?.id ) { + clearImageAttributes(); + } } - }, [ isImageDeleted ] ); + }, [ isSelected, attributes?.id, media?.id ] ); + + function clearImageAttributes() { + setAttributes( { + url: undefined, + alt: undefined, + id: undefined, + title: undefined, + caption: undefined, + } ); + } + + // A callback passed to MediaUpload, + // fired when the media modal closes. + function onCloseModal() { + if ( isMediaDestroyed( attributes?.id ) ) { + clearImageAttributes(); + } + } function onUploadError( message ) { noticeOperations.removeAllNotices(); @@ -155,14 +187,7 @@ export function ImageEdit( { function onSelectImage( media ) { if ( ! media || ! media.url ) { - setAttributes( { - url: undefined, - alt: undefined, - id: undefined, - title: undefined, - caption: undefined, - } ); - + clearImageAttributes(); return; } @@ -345,6 +370,7 @@ export function ImageEdit( { containerRef={ ref } context={ context } clientId={ clientId } + onCloseModal={ onCloseModal } /> ) } { ! url && ( @@ -361,6 +387,7 @@ export function ImageEdit( { onSelectURL={ onSelectURL } notices={ noticeUI } onError={ onUploadError } + onClose={ onCloseModal } accept="image/*" allowedTypes={ ALLOWED_MEDIA_TYPES } value={ { id, src } } diff --git a/packages/block-library/src/image/image.js b/packages/block-library/src/image/image.js index 8c49687685d87b..b6aa45e5a2c944 100644 --- a/packages/block-library/src/image/image.js +++ b/packages/block-library/src/image/image.js @@ -72,6 +72,7 @@ export default function Image( { isSelected, insertBlocksAfter, onReplace, + onCloseModal, onSelectImage, onSelectURL, onUploadError, @@ -352,6 +353,7 @@ export default function Image( { onSelect={ onSelectImage } onSelectURL={ onSelectURL } onError={ onUploadError } + onCloseModal={ onCloseModal } /> ) } diff --git a/packages/media-utils/src/components/media-upload/index.js b/packages/media-utils/src/components/media-upload/index.js index 656d0fa44237ae..a822956dd5b34e 100644 --- a/packages/media-utils/src/components/media-upload/index.js +++ b/packages/media-utils/src/components/media-upload/index.js @@ -1,20 +1,15 @@ /** * External dependencies */ -import { castArray, defaults, pick, noop } from 'lodash'; +import { castArray, defaults, pick } from 'lodash'; /** * WordPress dependencies */ import { Component } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; -import { useDispatch } from '@wordpress/data'; -const { wp } = window; -/** - * Internal dependencies - */ -import { store as mediaUtilsStore } from '../../store'; +const { wp } = window; const DEFAULT_EMPTY_GALLERY = []; @@ -244,9 +239,6 @@ class MediaUpload extends Component { this.onSelect = this.onSelect.bind( this ); this.onUpdate = this.onUpdate.bind( this ); this.onClose = this.onClose.bind( this ); - this.onRemoveSelectedAttachment = this.onRemoveSelectedAttachment.bind( - this - ); if ( gallery ) { this.buildAndSetGalleryFrame(); @@ -278,11 +270,6 @@ class MediaUpload extends Component { this.frame.on( 'update', this.onUpdate ); this.frame.on( 'open', this.onOpen ); this.frame.on( 'close', this.onClose ); - this.frame.listenTo( - wp.media.model.Attachments.all, - 'remove', - this.onRemoveSelectedAttachment - ); } /** @@ -387,18 +374,6 @@ class MediaUpload extends Component { onSelect( multiple ? attachment : attachment[ 0 ] ); } - onRemoveSelectedAttachment( attachment ) { - if ( attachment.destroyed ) { - const { onRemove = noop } = this.props; - // @TODO We can't dispatch outside the body of a function component. - // Maybe we should turn this into a function component first. - const { removeAttachment } = useDispatch( mediaUtilsStore ); - console.log( 'onRemoveSelectedAttachment', attachment ); - removeAttachment( attachment ); - onRemove( attachment ); - } - } - onOpen() { this.updateCollection(); diff --git a/packages/media-utils/src/index.js b/packages/media-utils/src/index.js index 4e1ed4aac6b170..590a7f4c9d188d 100644 --- a/packages/media-utils/src/index.js +++ b/packages/media-utils/src/index.js @@ -1,3 +1,2 @@ export * from './components'; export * from './utils'; -export { store } from './store'; diff --git a/packages/media-utils/src/store/actions.js b/packages/media-utils/src/store/actions.js deleted file mode 100644 index 384b31f2a9b54d..00000000000000 --- a/packages/media-utils/src/store/actions.js +++ /dev/null @@ -1,14 +0,0 @@ -/** - * Returns an action object used in signalling that an attachment, e.g., an image, is to be removed. - * - * @param {string} id Attachment unique identifier. - * - * @return {Object} Action object. - */ -export function removeAttachment( attachment ) { - console.log( 'removeAttachment', attachment ); - return { - type: 'REMOVE_ATTACHMENT', - attachment, - }; -} diff --git a/packages/media-utils/src/store/index.js b/packages/media-utils/src/store/index.js deleted file mode 100644 index cf17263fc472b5..00000000000000 --- a/packages/media-utils/src/store/index.js +++ /dev/null @@ -1,26 +0,0 @@ -/** - * WordPress dependencies - */ -import { createReduxStore, register } from '@wordpress/data'; - -/** - * Internal dependencies - */ -import reducer from './reducer'; -import * as actions from './actions'; -import * as selectors from './selectors'; - -/** - * Store definition for the notices namespace. - * - * @see https://github.com/WordPress/gutenberg/blob/HEAD/packages/data/README.md#createReduxStore - * - * @type {Object} - */ -export const store = createReduxStore( 'core/media-utils', { - reducer, - actions, - selectors, -} ); - -register( store ); diff --git a/packages/media-utils/src/store/reducer.js b/packages/media-utils/src/store/reducer.js deleted file mode 100644 index a7dbcd7ceffa47..00000000000000 --- a/packages/media-utils/src/store/reducer.js +++ /dev/null @@ -1,25 +0,0 @@ -/** - * WordPress dependencies - */ -import { combineReducers } from '@wordpress/data'; - -/** - * Reducer that tracks which attachments are in a guide. Each guide is represented by - * an array which contains the tip identifiers contained within that guide. - * - * @param {Array} state Current state. - * @param {Object} action Dispatched action. - * - * @return {Array} Updated state. - */ -export function deletedAttachments( state = [], action ) { - console.log( 'deletedAttachments', state, action ); - switch ( action.type ) { - case 'REMOVE_ATTACHMENT': - return [ ...state, action.attachment ]; - } - - return state; -} - -export default combineReducers( { deletedAttachments } ); diff --git a/packages/media-utils/src/store/selectors.js b/packages/media-utils/src/store/selectors.js deleted file mode 100644 index e4e443b48b9ce8..00000000000000 --- a/packages/media-utils/src/store/selectors.js +++ /dev/null @@ -1,18 +0,0 @@ -/** - * External dependencies - */ -import createSelector from 'rememo'; -import { find } from 'lodash'; - -/** - * Returns an object describing the deleted attachment, otherwise null. - * - * @param {Object} state Global application state. - * @param {string} id The id to query. - * - * @return {?Object} Information about the associated guide. - */ -export const getDeletedAttachment = createSelector( - ( state, id ) => find( state.deletedAttachments, [ 'id', id ] ) ?? null, - ( state ) => [ state.deletedAttachments ] -); From 6c2e8736cfb777696193453f6bb6fd5c1fc39899 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Tue, 16 Nov 2021 22:18:01 +1100 Subject: [PATCH 06/11] Testing setting a temporary media object so we can keep track of uploaded media --- packages/block-library/src/image/edit.js | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/block-library/src/image/edit.js b/packages/block-library/src/image/edit.js index f0aee5276dbc08..1894bb04ca6251 100644 --- a/packages/block-library/src/image/edit.js +++ b/packages/block-library/src/image/edit.js @@ -93,7 +93,7 @@ function hasDefaultSize( image, defaultSize ) { * that is, removed from the media library. The core Media Library * add a `destroyed` property to a deleted attachment object in the media collection. * - * @param {Number} id The attachment id. + * @param {number} id The attachment id. * * @return {boolean} Whether the image has been destroyed. */ @@ -125,6 +125,7 @@ export function ImageEdit( { sizeSlug, } = attributes; const [ temporaryURL, setTemporaryURL ] = useState(); + const [ temporaryMediaId, setTemporaryMediaId ] = useState(); const altRef = useRef(); useEffect( () => { @@ -142,7 +143,7 @@ export function ImageEdit( { return pick( getSettings(), [ 'imageDefaultSize', 'mediaUpload' ] ); }, [] ); - const media = useSelect( + const mediaObject = useSelect( ( select ) => { return select( coreStore ).getMedia( id ); }, @@ -155,12 +156,18 @@ export function ImageEdit( { // Also check on media object itself in case // the page had reloaded and the media attachment collection state no longer exists. useEffect( () => { - if ( isSelected ) { - if ( isMediaDestroyed( attributes?.id ) || ! media?.id ) { + // If we can find the media in the store, + // remove the temporary one in state. + if ( temporaryMediaId && temporaryMediaId === mediaObject?.id ) { + setTemporaryMediaId( undefined ); + return; + } + if ( isSelected && !! id && ! temporaryMediaId ) { + if ( isMediaDestroyed( id ) || ! mediaObject?.id ) { clearImageAttributes(); } } - }, [ isSelected, attributes?.id, media?.id ] ); + }, [ isSelected, id, mediaObject?.id, temporaryMediaId ] ); function clearImageAttributes() { setAttributes( { @@ -198,6 +205,10 @@ export function ImageEdit( { setTemporaryURL(); + // Keep a record of the media object id in state + // until we can grab one from the store. + setTemporaryMediaId( media.id ); + let mediaAttributes = pickRelevantMediaFiles( media, imageDefaultSize ); // If a caption text was meanwhile written by the user, From f6516b39b0397970b4817d311202d71d452a7a48 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Thu, 2 Dec 2021 12:35:40 +1100 Subject: [PATCH 07/11] Winding this back to check if an image exists when the editor loads. This removes the `isSelected` check, but also removes a bunch of code as well. We could run the same check on `isSelected`... TBC --- packages/block-library/src/image/edit.js | 53 +++++++++++------------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/packages/block-library/src/image/edit.js b/packages/block-library/src/image/edit.js index 1894bb04ca6251..8fd9d6fcc9b2f2 100644 --- a/packages/block-library/src/image/edit.js +++ b/packages/block-library/src/image/edit.js @@ -21,7 +21,6 @@ import { import { useEffect, useRef, useState } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { image as icon } from '@wordpress/icons'; -import { store as coreStore } from '@wordpress/core-data'; /* global wp */ @@ -102,6 +101,25 @@ function isMediaDestroyed( id ) { return attachment.destroyed; } +/** + * 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 {string} imageURL An image url for Image.src. + * @param {Function} onError A callback function if image load errors. + */ +function checkImageStatus( imageURL, onError = () => {} ) { + if ( ! imageURL ) { + return; + } + const newImage = new window.Image(); + newImage.src = imageURL; + newImage.addEventListener( 'error', function () { + onError(); + } ); +} + export function ImageEdit( { attributes, setAttributes, @@ -124,8 +142,8 @@ export function ImageEdit( { height, sizeSlug, } = attributes; + const [ temporaryURL, setTemporaryURL ] = useState(); - const [ temporaryMediaId, setTemporaryMediaId ] = useState(); const altRef = useRef(); useEffect( () => { @@ -143,31 +161,12 @@ export function ImageEdit( { return pick( getSettings(), [ 'imageDefaultSize', 'mediaUpload' ] ); }, [] ); - const mediaObject = useSelect( - ( select ) => { - return select( coreStore ).getMedia( id ); - }, - [ id ] - ); - - // Also check for destroyed media when the image is selected, - // If the image is used elsewhere in a post, - // the onCloseModal callback won't fire. - // Also check on media object itself in case - // the page had reloaded and the media attachment collection state no longer exists. + // Check onload if the image exists. useEffect( () => { - // If we can find the media in the store, - // remove the temporary one in state. - if ( temporaryMediaId && temporaryMediaId === mediaObject?.id ) { - setTemporaryMediaId( undefined ); - return; + if ( url ) { + checkImageStatus( url, clearImageAttributes ); } - if ( isSelected && !! id && ! temporaryMediaId ) { - if ( isMediaDestroyed( id ) || ! mediaObject?.id ) { - clearImageAttributes(); - } - } - }, [ isSelected, id, mediaObject?.id, temporaryMediaId ] ); + }, [] ); function clearImageAttributes() { setAttributes( { @@ -205,10 +204,6 @@ export function ImageEdit( { setTemporaryURL(); - // Keep a record of the media object id in state - // until we can grab one from the store. - setTemporaryMediaId( media.id ); - let mediaAttributes = pickRelevantMediaFiles( media, imageDefaultSize ); // If a caption text was meanwhile written by the user, From 8dac4ae3df2a46ffb8bff353125c32163e7c5e6a Mon Sep 17 00:00:00 2001 From: ramonjd Date: Mon, 13 Dec 2021 11:28:13 +1100 Subject: [PATCH 08/11] Removed the onerror listener and relying on the error handler already added to the Image props --- packages/block-library/src/image/edit.js | 40 ++++++++--------------- packages/block-library/src/image/image.js | 12 +++++-- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/packages/block-library/src/image/edit.js b/packages/block-library/src/image/edit.js index 8fd9d6fcc9b2f2..32a74b912282d6 100644 --- a/packages/block-library/src/image/edit.js +++ b/packages/block-library/src/image/edit.js @@ -101,25 +101,6 @@ function isMediaDestroyed( id ) { return attachment.destroyed; } -/** - * 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 {string} imageURL An image url for Image.src. - * @param {Function} onError A callback function if image load errors. - */ -function checkImageStatus( imageURL, onError = () => {} ) { - if ( ! imageURL ) { - return; - } - const newImage = new window.Image(); - newImage.src = imageURL; - newImage.addEventListener( 'error', function () { - onError(); - } ); -} - export function ImageEdit( { attributes, setAttributes, @@ -161,13 +142,6 @@ export function ImageEdit( { return pick( getSettings(), [ 'imageDefaultSize', 'mediaUpload' ] ); }, [] ); - // Check onload if the image exists. - useEffect( () => { - if ( url ) { - checkImageStatus( url, clearImageAttributes ); - } - }, [] ); - function clearImageAttributes() { setAttributes( { url: undefined, @@ -186,6 +160,19 @@ export function ImageEdit( { } } + /* + Runs an error callback if the image does not load. + If the error callback is triggered, we infer that that image + has been deleted. + */ + function onImageError( { isReplaced } ) { + // If the image block was not replaced with an embed, + // clear the attributes and trigger the placeholder. + if ( ! isReplaced ) { + clearImageAttributes(); + } + } + function onUploadError( message ) { noticeOperations.removeAllNotices(); noticeOperations.createErrorNotice( message ); @@ -377,6 +364,7 @@ export function ImageEdit( { context={ context } clientId={ clientId } onCloseModal={ onCloseModal } + onImageLoadError={ onImageError } /> ) } { ! url && ( diff --git a/packages/block-library/src/image/image.js b/packages/block-library/src/image/image.js index b6aa45e5a2c944..da49774497976c 100644 --- a/packages/block-library/src/image/image.js +++ b/packages/block-library/src/image/image.js @@ -79,6 +79,7 @@ export default function Image( { containerRef, context, clientId, + onImageLoadError, } ) { const imageRef = useRef(); const captionRef = useRef(); @@ -214,11 +215,18 @@ export default function Image( { } function onImageError() { - // Check if there's an embed block that handles this URL. + // 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 } } ); - if ( undefined !== embedBlock ) { + const shouldReplace = undefined !== embedBlock; + + if ( shouldReplace ) { onReplace( embedBlock ); } + + if ( onImageLoadError ) { + onImageLoadError( { isReplaced: shouldReplace } ); + } } function onSetHref( props ) { From 5410b0b708bc7eb203cf45705ef71e5a9c4fa3b2 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Tue, 14 Dec 2021 09:00:22 +1100 Subject: [PATCH 09/11] Fixed grammatical mistake. --- packages/block-library/src/image/edit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/image/edit.js b/packages/block-library/src/image/edit.js index 32a74b912282d6..20a2a1590d3d41 100644 --- a/packages/block-library/src/image/edit.js +++ b/packages/block-library/src/image/edit.js @@ -90,7 +90,7 @@ function hasDefaultSize( image, defaultSize ) { /** * Checks if a media attachment object has been "destroyed", * that is, removed from the media library. The core Media Library - * add a `destroyed` property to a deleted attachment object in the media collection. + * adds a `destroyed` property to a deleted attachment object in the media collection. * * @param {number} id The attachment id. * From 6859bd6d4144ac505f8d2b60c71cd43ec115a069 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Tue, 14 Dec 2021 13:35:04 +1100 Subject: [PATCH 10/11] Check if the media has been destroyed when the image is selected --- packages/block-library/src/image/edit.js | 4 ++-- packages/block-library/src/image/image.js | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/block-library/src/image/edit.js b/packages/block-library/src/image/edit.js index 20a2a1590d3d41..387e43c84c121c 100644 --- a/packages/block-library/src/image/edit.js +++ b/packages/block-library/src/image/edit.js @@ -96,7 +96,7 @@ function hasDefaultSize( image, defaultSize ) { * * @return {boolean} Whether the image has been destroyed. */ -function isMediaDestroyed( id ) { +export function isMediaDestroyed( id ) { const attachment = wp?.media?.attachment( id ) || {}; return attachment.destroyed; } @@ -165,7 +165,7 @@ export function ImageEdit( { If the error callback is triggered, we infer that that image has been deleted. */ - function onImageError( { isReplaced } ) { + function onImageError( isReplaced = false ) { // If the image block was not replaced with an embed, // clear the attributes and trigger the placeholder. if ( ! isReplaced ) { diff --git a/packages/block-library/src/image/image.js b/packages/block-library/src/image/image.js index da49774497976c..455743c6ebee09 100644 --- a/packages/block-library/src/image/image.js +++ b/packages/block-library/src/image/image.js @@ -43,7 +43,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 @@ -224,9 +224,7 @@ export default function Image( { onReplace( embedBlock ); } - if ( onImageLoadError ) { - onImageLoadError( { isReplaced: shouldReplace } ); - } + onImageLoadError( shouldReplace ); } function onSetHref( props ) { @@ -298,6 +296,9 @@ export default function Image( { if ( ! isSelected ) { setIsEditingImage( false ); } + if ( isSelected && isMediaDestroyed( id ) ) { + onImageLoadError(); + } }, [ isSelected ] ); const canEditImage = id && naturalWidth && naturalHeight && imageEditing; From 0bfc431d68ab71008b35009cddf9873f2bb8887f Mon Sep 17 00:00:00 2001 From: ramonjd Date: Tue, 14 Dec 2021 15:10:34 +1100 Subject: [PATCH 11/11] Only delete image source URL and ID when detecting a destroyed image. That way the caption, title and alt values persist in case the user deletes, then replaces. --- packages/block-library/src/image/edit.js | 30 +++++++++++++----------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/block-library/src/image/edit.js b/packages/block-library/src/image/edit.js index 387e43c84c121c..bcef8caefe829e 100644 --- a/packages/block-library/src/image/edit.js +++ b/packages/block-library/src/image/edit.js @@ -123,7 +123,6 @@ export function ImageEdit( { height, sizeSlug, } = attributes; - const [ temporaryURL, setTemporaryURL ] = useState(); const altRef = useRef(); @@ -142,21 +141,14 @@ export function ImageEdit( { return pick( getSettings(), [ 'imageDefaultSize', 'mediaUpload' ] ); }, [] ); - function clearImageAttributes() { - setAttributes( { - url: undefined, - alt: undefined, - id: undefined, - title: undefined, - caption: undefined, - } ); - } - // A callback passed to MediaUpload, // fired when the media modal closes. function onCloseModal() { if ( isMediaDestroyed( attributes?.id ) ) { - clearImageAttributes(); + setAttributes( { + url: undefined, + id: undefined, + } ); } } @@ -169,7 +161,10 @@ export function ImageEdit( { // If the image block was not replaced with an embed, // clear the attributes and trigger the placeholder. if ( ! isReplaced ) { - clearImageAttributes(); + setAttributes( { + url: undefined, + id: undefined, + } ); } } @@ -180,7 +175,14 @@ export function ImageEdit( { function onSelectImage( media ) { if ( ! media || ! media.url ) { - clearImageAttributes(); + setAttributes( { + url: undefined, + alt: undefined, + id: undefined, + title: undefined, + caption: undefined, + } ); + return; }