From a05b2eb12abfb65a5863d1f5d1ef88828848db15 Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Thu, 25 Feb 2021 17:14:17 +1300 Subject: [PATCH 1/5] Move the setting of the default attributes on new blocks to a single useEffect to handle images added by MediaPlaceholder and drag and drop --- packages/block-library/src/gallery/edit.js | 72 ++++++++++++++++++++-- 1 file changed, 67 insertions(+), 5 deletions(-) diff --git a/packages/block-library/src/gallery/edit.js b/packages/block-library/src/gallery/edit.js index afe8af17a87432..b6e5709517b50a 100644 --- a/packages/block-library/src/gallery/edit.js +++ b/packages/block-library/src/gallery/edit.js @@ -2,7 +2,7 @@ * External dependencies */ import classnames from 'classnames'; -import { isEmpty, concat, differenceBy, some, find } from 'lodash'; +import { isEmpty, concat, some, find } from 'lodash'; /** * WordPress dependencies @@ -24,7 +24,7 @@ import { useBlockProps, } from '@wordpress/block-editor'; import { store as coreStore } from '@wordpress/core-data'; -import { Platform, useEffect, useMemo } from '@wordpress/element'; +import { Platform, useEffect, useMemo, useState } from '@wordpress/element'; import { __, sprintf } from '@wordpress/i18n'; import { useSelect, useDispatch } from '@wordpress/data'; import { withViewportMatch } from '@wordpress/viewport'; @@ -94,6 +94,8 @@ function GalleryEdit( props ) { sizeSlug, } = attributes; + const [ currentImages, setCurrentImages ] = useState( [] ); + const { __unstableMarkNextChangeAsNotPersistent, replaceInnerBlocks, @@ -122,9 +124,11 @@ function GalleryEdit( props ) { const images = useMemo( () => innerBlockImages?.map( ( block ) => ( { + clientId: block.clientId, id: block.attributes.id, url: block.attributes.url, attributes: block.attributes, + fromSavedContent: Boolean( block.originalContent ), } ) ), [ innerBlockImages ] ); @@ -152,6 +156,59 @@ function GalleryEdit( props ) { [ innerBlockImages ] ); + useEffect( () => { + let imagesUpdated = false; + + // First lets check if any images have been deleted. + const newCurrentImages = currentImages.filter( ( currentImg ) => + images.find( ( img ) => { + return currentImg.clientId === img.clientId; + } ) + ); + + if ( newCurrentImages.length < currentImages.length ) { + imagesUpdated = true; + } + + // Now lets see if we have any images hydrated from saved content and if so + // add then to currentImages state. + images.forEach( ( image ) => { + if ( + image.fromSavedContent && + ! newCurrentImages.find( + ( currentImage ) => currentImage.id === image.id + ) + ) { + imagesUpdated = true; + newCurrentImages.push( image ); + } + } ); + + // Now check for any new images that have been added InnerBlocks and for which + // we have the imageData we need for setting default block attributes. + const incomingImages = images.filter( + ( image ) => + ! newCurrentImages.find( + ( currentImage ) => image.id && currentImage.id === image.id + ) && + imageData?.find( ( img ) => img.id === image.id ) && + ! image.fromSavedConent + ); + + incomingImages.forEach( ( newImage ) => { + imagesUpdated = true; + updateBlockAttributes( newImage.clientId, { + ...buildImageAttributes( false, newImage ), + id: newImage.id, + } ); + newCurrentImages.push( newImage ); + } ); + + if ( imagesUpdated ) { + setCurrentImages( newCurrentImages ); + } + }, [ images, imageData, currentImages ] ); + const shortCodeImages = useShortCodeTransform( shortCodeTransforms ); useEffect( () => { @@ -241,17 +298,22 @@ function GalleryEdit( props ) { const existingImageBlocks = ! newFileUploads ? innerBlockImages.filter( ( block ) => processedImages.find( - ( img ) => img.url === block.attributes.url + ( img ) => img.id === block.attributes.id ) ) : innerBlockImages; - const newImages = differenceBy( processedImages, images, 'url' ); + const newImages = processedImages.filter( + ( img ) => + ! existingImageBlocks.find( + ( existingImg ) => img.id === existingImg.attributes.id + ) + ); const newBlocks = newImages.map( ( image ) => { return createBlock( 'core/image', { - ...buildImageAttributes( false, image ), id: image.id, + url: image.url, } ); } ); From 47f9d628bdb5fd72b83bd37fc364d0b4e4c08c3b Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Fri, 26 Feb 2021 09:06:34 +1300 Subject: [PATCH 2/5] Move calculation of new images to a custom hook --- packages/block-library/src/gallery/edit.js | 64 ++++--------------- .../src/gallery/use-get-new-images.js | 56 ++++++++++++++++ 2 files changed, 68 insertions(+), 52 deletions(-) create mode 100644 packages/block-library/src/gallery/use-get-new-images.js diff --git a/packages/block-library/src/gallery/edit.js b/packages/block-library/src/gallery/edit.js index b6e5709517b50a..e798426b576aa8 100644 --- a/packages/block-library/src/gallery/edit.js +++ b/packages/block-library/src/gallery/edit.js @@ -24,7 +24,7 @@ import { useBlockProps, } from '@wordpress/block-editor'; import { store as coreStore } from '@wordpress/core-data'; -import { Platform, useEffect, useMemo, useState } from '@wordpress/element'; +import { Platform, useEffect, useMemo } from '@wordpress/element'; import { __, sprintf } from '@wordpress/i18n'; import { useSelect, useDispatch } from '@wordpress/data'; import { withViewportMatch } from '@wordpress/viewport'; @@ -51,6 +51,7 @@ import { } from './constants'; import useImageSizes from './use-image-sizes'; import useShortCodeTransform from './use-short-code-transform'; +import useGetNewImages from './use-get-new-images'; const MAX_COLUMNS = 8; const linkOptions = [ @@ -94,8 +95,6 @@ function GalleryEdit( props ) { sizeSlug, } = attributes; - const [ currentImages, setCurrentImages ] = useState( [] ); - const { __unstableMarkNextChangeAsNotPersistent, replaceInnerBlocks, @@ -151,63 +150,24 @@ function GalleryEdit( props ) { const getMediaItems = select( coreStore ).getMediaItems; - return getMediaItems( { include: imageIds } ); + return getMediaItems( { + include: imageIds, + per_page: imageIds.length, + } ); }, [ innerBlockImages ] ); - useEffect( () => { - let imagesUpdated = false; - - // First lets check if any images have been deleted. - const newCurrentImages = currentImages.filter( ( currentImg ) => - images.find( ( img ) => { - return currentImg.clientId === img.clientId; - } ) - ); - - if ( newCurrentImages.length < currentImages.length ) { - imagesUpdated = true; - } - - // Now lets see if we have any images hydrated from saved content and if so - // add then to currentImages state. - images.forEach( ( image ) => { - if ( - image.fromSavedContent && - ! newCurrentImages.find( - ( currentImage ) => currentImage.id === image.id - ) - ) { - imagesUpdated = true; - newCurrentImages.push( image ); - } - } ); - - // Now check for any new images that have been added InnerBlocks and for which - // we have the imageData we need for setting default block attributes. - const incomingImages = images.filter( - ( image ) => - ! newCurrentImages.find( - ( currentImage ) => image.id && currentImage.id === image.id - ) && - imageData?.find( ( img ) => img.id === image.id ) && - ! image.fromSavedConent - ); + const newImages = useGetNewImages( images, imageData ); - incomingImages.forEach( ( newImage ) => { - imagesUpdated = true; + useEffect( () => { + newImages?.forEach( ( newImage ) => { updateBlockAttributes( newImage.clientId, { ...buildImageAttributes( false, newImage ), id: newImage.id, } ); - newCurrentImages.push( newImage ); } ); - - if ( imagesUpdated ) { - setCurrentImages( newCurrentImages ); - } - }, [ images, imageData, currentImages ] ); + }, [ newImages ] ); const shortCodeImages = useShortCodeTransform( shortCodeTransforms ); @@ -303,14 +263,14 @@ function GalleryEdit( props ) { ) : innerBlockImages; - const newImages = processedImages.filter( + const newImageList = processedImages.filter( ( img ) => ! existingImageBlocks.find( ( existingImg ) => img.id === existingImg.attributes.id ) ); - const newBlocks = newImages.map( ( image ) => { + const newBlocks = newImageList.map( ( image ) => { return createBlock( 'core/image', { id: image.id, url: image.url, diff --git a/packages/block-library/src/gallery/use-get-new-images.js b/packages/block-library/src/gallery/use-get-new-images.js new file mode 100644 index 00000000000000..1b14854856e8af --- /dev/null +++ b/packages/block-library/src/gallery/use-get-new-images.js @@ -0,0 +1,56 @@ +/** + * WordPress dependencies + */ +import { useMemo, useState } from '@wordpress/element'; + +export default function useGetNewImages( images, imageData ) { + const [ currentImages, setCurrentImages ] = useState( [] ); + + return useMemo( () => getNewImages(), [ images, imageData ] ); + + function getNewImages() { + let imagesUpdated = false; + + // First lets check if any images have been deleted. + const newCurrentImages = currentImages.filter( ( currentImg ) => + images.find( ( img ) => { + return currentImg.clientId === img.clientId; + } ) + ); + + if ( newCurrentImages.length < currentImages.length ) { + imagesUpdated = true; + } + + // Now lets see if we have any images hydrated from saved content and if so + // add then to currentImages state. + images.forEach( ( image ) => { + if ( + image.fromSavedContent && + ! newCurrentImages.find( + ( currentImage ) => currentImage.id === image.id + ) + ) { + imagesUpdated = true; + newCurrentImages.push( image ); + } + } ); + + // Now check for any new images that have been added InnerBlocks and for which + // we have the imageData we need for setting default block attributes. + const newImages = images.filter( + ( image ) => + ! newCurrentImages.find( + ( currentImage ) => image.id && currentImage.id === image.id + ) && + imageData?.find( ( img ) => img.id === image.id ) && + ! image.fromSavedConent + ); + + if ( imagesUpdated || newImages?.length > 0 ) { + setCurrentImages( [ ...newCurrentImages, ...newImages ] ); + } + + return newImages.length > 0 ? newImages : null; + } +} From 6282a15e3fb36149f73afe6d68a84d3b93543b9f Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Fri, 26 Feb 2021 11:22:11 +1300 Subject: [PATCH 3/5] Move getMediaItems to a hook and optimise to only get the media for items we dont have yet --- packages/block-library/src/gallery/edit.js | 30 +--------- .../src/gallery/use-get-media.js | 59 +++++++++++++++++++ .../src/gallery/use-get-new-images.js | 4 +- 3 files changed, 64 insertions(+), 29 deletions(-) create mode 100644 packages/block-library/src/gallery/use-get-media.js diff --git a/packages/block-library/src/gallery/edit.js b/packages/block-library/src/gallery/edit.js index e798426b576aa8..3100a3293e96c4 100644 --- a/packages/block-library/src/gallery/edit.js +++ b/packages/block-library/src/gallery/edit.js @@ -2,7 +2,7 @@ * External dependencies */ import classnames from 'classnames'; -import { isEmpty, concat, some, find } from 'lodash'; +import { isEmpty, concat, find } from 'lodash'; /** * WordPress dependencies @@ -23,7 +23,6 @@ import { InspectorControls, useBlockProps, } from '@wordpress/block-editor'; -import { store as coreStore } from '@wordpress/core-data'; import { Platform, useEffect, useMemo } from '@wordpress/element'; import { __, sprintf } from '@wordpress/i18n'; import { useSelect, useDispatch } from '@wordpress/data'; @@ -52,6 +51,7 @@ import { import useImageSizes from './use-image-sizes'; import useShortCodeTransform from './use-short-code-transform'; import useGetNewImages from './use-get-new-images'; +import useGetMedia from './use-get-media'; const MAX_COLUMNS = 8; const linkOptions = [ @@ -132,31 +132,7 @@ function GalleryEdit( props ) { [ innerBlockImages ] ); - const imageData = useSelect( - ( select ) => { - if ( - ! innerBlockImages?.length || - some( - innerBlockImages, - ( imageBlock ) => ! imageBlock.attributes.id - ) - ) { - return imageData; - } - - const imageIds = innerBlockImages.map( - ( imageBlock ) => imageBlock.attributes.id - ); - - const getMediaItems = select( coreStore ).getMediaItems; - - return getMediaItems( { - include: imageIds, - per_page: imageIds.length, - } ); - }, - [ innerBlockImages ] - ); + const imageData = useGetMedia( innerBlockImages ); const newImages = useGetNewImages( images, imageData ); diff --git a/packages/block-library/src/gallery/use-get-media.js b/packages/block-library/src/gallery/use-get-media.js new file mode 100644 index 00000000000000..966da0426d660d --- /dev/null +++ b/packages/block-library/src/gallery/use-get-media.js @@ -0,0 +1,59 @@ +/** + * External dependencies + */ +import { some } from 'lodash'; + +/** + * WordPress dependencies + */ +import { useState } from '@wordpress/element'; +import { useSelect } from '@wordpress/data'; +import { store as coreStore } from '@wordpress/core-data'; + +export default function useGetMedia( innerBlockImages ) { + const [ currentImageMedia, setCurrentImageMedia ] = useState( [] ); + + const imageMedia = useSelect( + ( select ) => { + if ( + ! innerBlockImages?.length || + some( + innerBlockImages, + ( imageBlock ) => ! imageBlock.attributes.id + ) + ) { + return currentImageMedia; + } + + const imageIds = innerBlockImages + .map( ( imageBlock ) => imageBlock.attributes.id ) + .filter( + ( id ) => + ! currentImageMedia.find( ( img ) => id === img.id ) + ); + if ( imageIds.length === 0 ) { + return currentImageMedia; + } + const getMediaItems = select( coreStore ).getMediaItems; + return getMediaItems( { + include: imageIds, + per_page: imageIds.length, + } ); + }, + [ innerBlockImages ] + ); + + const newData = imageMedia?.filter( + ( img ) => + ! currentImageMedia.find( + ( currentImg ) => currentImg.id === img.id + ) + ); + + if ( newData?.length > 0 ) { + const newCurrentMedia = [ ...currentImageMedia, ...newData ]; + setCurrentImageMedia( newCurrentMedia ); + return newCurrentMedia; + } + return currentImageMedia; +} diff --git a/packages/block-library/src/gallery/use-get-new-images.js b/packages/block-library/src/gallery/use-get-new-images.js index 1b14854856e8af..25ac5a15560ee3 100644 --- a/packages/block-library/src/gallery/use-get-new-images.js +++ b/packages/block-library/src/gallery/use-get-new-images.js @@ -23,7 +23,7 @@ export default function useGetNewImages( images, imageData ) { } // Now lets see if we have any images hydrated from saved content and if so - // add then to currentImages state. + // add them to currentImages state. images.forEach( ( image ) => { if ( image.fromSavedContent && @@ -36,7 +36,7 @@ export default function useGetNewImages( images, imageData ) { } } ); - // Now check for any new images that have been added InnerBlocks and for which + // Now check for any new images that have been added to InnerBlocks and for which // we have the imageData we need for setting default block attributes. const newImages = images.filter( ( image ) => From f08d790e58bd466db00b7b725759be87db5dac72 Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Fri, 26 Feb 2021 12:21:23 +1300 Subject: [PATCH 4/5] Revert back to get media call as can't see a way to page large result sets with getMediaItems call --- .../block-library/src/gallery/use-get-media.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/block-library/src/gallery/use-get-media.js b/packages/block-library/src/gallery/use-get-media.js index 966da0426d660d..16bd1591d7188a 100644 --- a/packages/block-library/src/gallery/use-get-media.js +++ b/packages/block-library/src/gallery/use-get-media.js @@ -34,11 +34,16 @@ export default function useGetMedia( innerBlockImages ) { if ( imageIds.length === 0 ) { return currentImageMedia; } - const getMediaItems = select( coreStore ).getMediaItems; - return getMediaItems( { - include: imageIds, - per_page: imageIds.length, + + const getMedia = select( coreStore ).getMedia; + const newImageMedia = imageIds.map( ( img ) => { + return getMedia( img ); } ); + + if ( newImageMedia.some( ( img ) => ! img ) ) { + return currentImageMedia; + } + return newImageMedia; }, [ innerBlockImages ] ); @@ -55,5 +60,6 @@ export default function useGetMedia( innerBlockImages ) { setCurrentImageMedia( newCurrentMedia ); return newCurrentMedia; } + return currentImageMedia; } From 2788320bb27910a1031204087001a4f2751c4e1d Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Fri, 26 Feb 2021 15:29:23 +1300 Subject: [PATCH 5/5] Revert to just getting all imagedata each time as code much simpler that way --- .../src/gallery/use-get-media.js | 26 ++++++------------- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/packages/block-library/src/gallery/use-get-media.js b/packages/block-library/src/gallery/use-get-media.js index 16bd1591d7188a..fc04e070f96178 100644 --- a/packages/block-library/src/gallery/use-get-media.js +++ b/packages/block-library/src/gallery/use-get-media.js @@ -25,16 +25,13 @@ export default function useGetMedia( innerBlockImages ) { return currentImageMedia; } - const imageIds = innerBlockImages - .map( ( imageBlock ) => imageBlock.attributes.id ) - .filter( - ( id ) => - ! currentImageMedia.find( ( img ) => id === img.id ) - ); + const imageIds = innerBlockImages.map( + ( imageBlock ) => imageBlock.attributes.id + ); + if ( imageIds.length === 0 ) { return currentImageMedia; } - const getMedia = select( coreStore ).getMedia; const newImageMedia = imageIds.map( ( img ) => { return getMedia( img ); @@ -43,22 +40,15 @@ export default function useGetMedia( innerBlockImages ) { if ( newImageMedia.some( ( img ) => ! img ) ) { return currentImageMedia; } + return newImageMedia; }, [ innerBlockImages ] ); - const newData = imageMedia?.filter( - ( img ) => - ! currentImageMedia.find( - ( currentImg ) => currentImg.id === img.id - ) - ); - - if ( newData?.length > 0 ) { - const newCurrentMedia = [ ...currentImageMedia, ...newData ]; - setCurrentImageMedia( newCurrentMedia ); - return newCurrentMedia; + if ( imageMedia?.length !== currentImageMedia.length ) { + setCurrentImageMedia( imageMedia ); + return imageMedia; } return currentImageMedia;