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.

Sign up for GitHub

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

Jump to bottom

Fix: Gallery does not link to full size images #16011

Merged
merged 1 commit into from Aug 5, 2019
Merged

Fix: Gallery does not link to full size images #16011

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/block-library/src/gallery/block.json
View file
Open in desktop
Expand Up @@ -13,6 +13,11 @@
"selector": "img",
"attribute": "src"
},
"fullUrl": {
"source": "attribute",
"selector": "img",
"attribute": "data-full-url"
},
"link": {
"source": "attribute",
"selector": "img",
Expand Down
13 changes: 11 additions & 2 deletions packages/block-library/src/gallery/save.js
View file
Open in desktop
Expand Up @@ -17,14 +17,23 @@ export default function save( { attributes } ) {

switch ( linkTo ) {
case 'media':
href = image.url;
href = image.fullUrl || image.url;
Copy link
Member

Choose a reason for hiding this comment

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

In what cases is there no full size?

Copy link
Member Author

Choose a reason for hiding this comment

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

Current galleries don't have the fullUrl attribute set, this is a simple way to be back-compatible in these cases. Another even more important case where this may happen is when the gallery links to external images e.g: a set of image blocks linking to external URL's is transformed into a gallery.

break;
case 'attachment':
href = image.link;
break;
}

const img = <img src={ image.url } alt={ image.alt } data-id={ image.id } data-link={ image.link } className={ image.id ? `wp-image-${ image.id }` : null } />;
const img = (
<img
src={ image.url }
alt={ image.alt }
data-id={ image.id }
data-full-url={ image.fullUrl }
Copy link
Member

Choose a reason for hiding this comment

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

Why should this be saved in the HTML?

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 replicated exactly the same mechanism used in Attachment page links (data-link attribute), even if the user sets the link to none we save data-link attribute. This happens because the user may create a gallery set the link to none publish, and later come back to the editor and set the link to "Attachment page" or "Media File". When that happens, we need to have the full image URL, and the attachment page URL available so we can link to these destinations. It may also be useful for third-party plugins having these attributes available.

Copy link
Member

Choose a reason for hiding this comment

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

Why not set it as a block level attribute?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @ellatrix the only way to set this attribute as a block-level attribute would be by using an attribute that is an array of strings containing the full URL of each image. But, then if the user reordered the images in code (which is something users may expect to work), it will break the block. We do something similar for image id's so the server parser can have access to attributes, but, even for this case we save a data-id attribute in HTML, and we have an attribute sync mechanisms to make sure the reordering works. Given that the server has access to image id's, it can quickly get the full URL, so there is no need to perform this mechanism another time.

I guess the ideal solution would be using nested image blocks so these attributes can be normal block attributes of the nested images. But, until we can implement that, I guess this solution is fine as it just replicates what we were doing for another attribute and solves a big problem affecting our users.

data-link={ image.link }
className={ image.id ? `wp-image-${ image.id }` : null }
/>
);

return (
<li key={ image.id || image.url } className="blocks-gallery-item">
Expand Down
4 changes: 4 additions & 0 deletions packages/block-library/src/gallery/shared.js
View file
Open in desktop
Expand Up @@ -10,5 +10,9 @@ export function defaultColumnsNumber( attributes ) {
export const pickRelevantMediaFiles = ( image ) => {
const imageProps = pick( image, [ 'alt', 'id', 'link', 'caption' ] );
imageProps.url = get( image, [ 'sizes', 'large', 'url' ] ) || get( image, [ 'media_details', 'sizes', 'large', 'source_url' ] ) || image.url;
const fullUrl = get( image, [ 'sizes', 'full', 'url' ] ) || get( image, [ 'media_details', 'sizes', 'full', 'source_url' ] );
if ( fullUrl ) {
imageProps.fullUrl = fullUrl;
}
return imageProps;
};