Block Library - Gallery: Set 'addToGallery' prop to false when images don't have IDs

[Mamaduka]

Description

In the "add to gallery" state, passing an array of undefined ID values to the MediaUpload component causes adding all images to the gallery.

I'm trying to resolve this by setting the addToGallery prop to false and the MediaPlaceholder value to an empty object when images don't have IDs. With both props set to "falsy" values, the gallery media frame is initialized in the "Create Gallery" state. This lets users replace placeholder images from patterns.

Fixes #29834.

How has this been tested?

1. Have multiple items already added to the media library.
2. Create a new post
3. Drag the Gallery pattern (Two images side by side)
4. Click media library at the bottom
5. Select an image
6. Click on "Create Gallery" and then "Insert Gallery"
7. Only the selected image should replace one image from the pattern

Types of changes

Bugfix

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• [N/A] My code follows the accessibility standards.
• [N/A] I've tested my changes with keyboard and screen readers.
• [N/A] My code has proper inline documentation.
• I've included developer documentation if appropriate.
• [N/A] I've updated all React Native files affected by any refactorings/renamings in this PR.

[gwwar]

Thanks for taking this one! We can also filter out any unexpected data in the array. This can help guard against malformed data (potentially from bugs/accidental user-edits).

const imageIds = images.filter( ( id ) => ( Number.isInteger( id ) && id > 0 ) ); // for example [null,1,null,-3,44] will return [1, 44]
const hasImages = !! images.length;

//...
addToGallery={ hasImages }
value={ imageIds }

@glendaviesnz since you've been 👀 the Gallery block refactor, is it expected that the pattern in two images side-by-side has serialized null ids? If not, do you have a recommendation for a better spot to validate values here?

Twenty Twenty-One, Gallery Two Images Side by Side

<!-- wp:gallery {"ids":[null,null],"linkTo":"none","align":"wide"} -->
<figure class="wp-block-gallery alignwide columns-2 is-cropped"><ul class="blocks-gallery-grid"><li class="blocks-gallery-item"><figure><img src="https://s.w.org/images/core/5.5/don-quixote-05.jpg" alt="An old pencil drawing of Don Quixote and Sancho Panza sitting on their horses, by Wilhelm Marstrand."/></figure></li><li class="blocks-gallery-item"><figure><img src="https://s.w.org/images/core/5.5/don-quixote-01.jpg" alt="An old pencil drawing of Don Quixote and Sancho Panza sitting on their horses, by Wilhelm Marstrand."/></figure></li></ul></figure>
<!-- /wp:gallery -->

[Mamaduka]

Thanks, @gwwar, for the review.

We can also filter out any unexpected data in the array.

It makes sense.

I think we should keep image IDs check separated. While patterns might not have IDs, technically, images are there, and we want to have an edit component in that state.

[glendaviesnz]

is it expected that the pattern in two images side-by-side has serialized null ids?

Yes, this is the way the gallery handles external images, eg. if you add a stand alone image block and choose the option to add it from url, and then transform the image block to a gallery block it will add a null entry to the ids array.

do you have a recommendation for a better spot to validate values here?

I had a quick look at it appears like the ideal fix for this will need to go in the core media browser, as it needs to know that if it is passed an addToGallery=true, but an empty value, to assume that a gallery with external images is being updated, and in which case it would need to display those images in the gallery edit screen, even though they are not saved in the media library.

Currently @Mamaduka your fix prevents all of the images being added by default, but the media library defaults to 'Create gallery' which means that the existing images are overwritten:

This is ok with the patterns use case, as we can probably assume the user wants to replace the placeholder images, but as mentioned above it is possible for a user to create their own gallery with null image ids, in which case their images will be lost if they try to add additional images from the media library.

I had a quick look and couldn't see a way around this from the block side of things. I will have a look at the core side and see what options there might be, but not familiar with that code so not sure how far I will get.

[glendaviesnz]

in which case their images will be lost if they try to add additional images from the media library.

this is the current behaviour anyway I just realised, ie. if a user edits a gallery with null id images their existing images are lost, so we wouldn't make things any worse by at least not adding all images by default.

[gziolo]

Let's get it into the WP 5.7.1 release.

[gwwar]

Excellent, thanks @Mamaduka for the contribution! 💖 to @glendaviesnz and @gziolo for reviewing too!