Skip to content

Commit

Permalink
Fix gallery to image transform and inconsistent types used in gallery…
Browse files Browse the repository at this point in the history
… block (#20084)

* Fix type inconsistencies in gallery block

* Use integer form for image id when converting a gallery to an image

* Ensure correct types are used for other transforms
  • Loading branch information
talldan committed Feb 13, 2020
1 parent cf8fa0a commit 526b7d7
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 12 deletions.
5 changes: 5 additions & 0 deletions packages/block-library/src/gallery/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,32 @@
"selector": ".blocks-gallery-item",
"query": {
"url": {
"type": "string",
"source": "attribute",
"selector": "img",
"attribute": "src"
},
"fullUrl": {
"type": "string",
"source": "attribute",
"selector": "img",
"attribute": "data-full-url"
},
"link": {
"type": "string",
"source": "attribute",
"selector": "img",
"attribute": "data-link"
},
"alt": {
"type": "string",
"source": "attribute",
"selector": "img",
"attribute": "alt",
"default": ""
},
"id": {
"type": "string",
"source": "attribute",
"selector": "img",
"attribute": "data-id"
Expand Down
23 changes: 19 additions & 4 deletions packages/block-library/src/gallery/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
map,
reduce,
some,
toString,
} from 'lodash';

/**
Expand Down Expand Up @@ -102,7 +103,9 @@ class GalleryEdit extends Component {
if ( attributes.images ) {
attributes = {
...attributes,
ids: map( attributes.images, 'id' ),
// Unlike images[ n ].id which is a string, always ensure the
// ids array contains numbers as per its attribute type.
ids: map( attributes.images, ( { id } ) => parseInt( id, 10 ) ),
};
}

Expand Down Expand Up @@ -161,7 +164,11 @@ class GalleryEdit extends Component {
}

selectCaption( newImage, images, attachmentCaptions ) {
const currentImage = find( images, { id: newImage.id } );
// The image id in both the images and attachmentCaptions arrays is a
// string, so ensure comparison works correctly by converting the
// newImage.id to a string.
const newImageId = toString( newImage.id );
const currentImage = find( images, { id: newImageId } );

const currentImageCaption = currentImage
? currentImage.caption
Expand All @@ -171,7 +178,9 @@ class GalleryEdit extends Component {
return currentImageCaption;
}

const attachment = find( attachmentCaptions, { id: newImage.id } );
const attachment = find( attachmentCaptions, {
id: newImageId,
} );

// if the attachment caption is updated
if ( attachment && attachment.caption !== newImage.caption ) {
Expand All @@ -186,7 +195,9 @@ class GalleryEdit extends Component {
const { attachmentCaptions } = this.state;
this.setState( {
attachmentCaptions: newImages.map( ( newImage ) => ( {
id: newImage.id,
// Store the attachmentCaption id as a string for consistency
// with the type of the id in the images attribute.
id: toString( newImage.id ),
caption: newImage.caption,
} ) ),
} );
Expand All @@ -198,6 +209,10 @@ class GalleryEdit extends Component {
images,
attachmentCaptions
),
// The id value is stored in a data attribute, so when the
// block is parsed it's converted to a string. Converting
// to a string here ensures it's type is consistent.
id: toString( newImage.id ),
} ) ),
columns: columns ? Math.min( newImages.length, columns ) : columns,
} );
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/gallery/gallery-image.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ export default compose( [
const { id } = ownProps;

return {
image: id ? getMedia( id ) : null,
image: id ? getMedia( parseInt( id, 10 ) ) : null,
};
} ),
withDispatch( ( dispatch ) => {
Expand Down
14 changes: 7 additions & 7 deletions packages/block-library/src/gallery/transforms.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { filter, every } from 'lodash';
import { filter, every, toString } from 'lodash';

/**
* WordPress dependencies
Expand Down Expand Up @@ -44,13 +44,13 @@ const transforms = {
return createBlock( 'core/gallery', {
images: validImages.map(
( { id, url, alt, caption } ) => ( {
id,
id: toString( id ),
url,
alt,
caption,
} )
),
ids: validImages.map( ( { id } ) => id ),
ids: validImages.map( ( { id } ) => parseInt( id, 10 ) ),
align,
sizeSlug,
} );
Expand All @@ -64,7 +64,7 @@ const transforms = {
type: 'array',
shortcode: ( { named: { ids } } ) => {
return parseShortcodeIds( ids ).map( ( id ) => ( {
id,
id: toString( id ),
} ) );
},
},
Expand Down Expand Up @@ -116,11 +116,11 @@ const transforms = {
{
type: 'block',
blocks: [ 'core/image' ],
transform: ( { images, align, sizeSlug } ) => {
transform: ( { images, align, sizeSlug, ids } ) => {
if ( images.length > 0 ) {
return images.map( ( { id, url, alt, caption } ) =>
return images.map( ( { url, alt, caption }, index ) =>
createBlock( 'core/image', {
id,
id: ids[ index ],
url,
alt,
caption,
Expand Down

0 comments on commit 526b7d7

Please sign in to comment.