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 a bug that caused the display ratio of images to be corrupted starting from 6.3.( fix #53555 ) #53598

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

Fix a bug that caused the display ratio of images to be corrupted starting from 6.3.( fix #53555 ) #53598

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions packages/block-library/src/image/deprecated.js
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

If migrate needs to be added to all these deprecations, that seems to me like a bug in applyBlockDeprecatedVersions, not the image block deprecations. The documentation for block deprecations doesn't mention this, and it seems rather awkward and error-prone to have to add them everywhere.

Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,13 @@ const v4 = {
supports: {
anchor: true,
},
migrate( { width, height, ...attributes } ) {
return {
...attributes,
width: `${ width }px`,
height: `${ height }px`,
};
},
save( { attributes } ) {
const {
url,
Expand Down Expand Up @@ -480,6 +487,13 @@ const v5 = {
},
},
},
migrate( { width, height, ...attributes } ) {
return {
...attributes,
width: `${ width }px`,
height: `${ height }px`,
};
},
save( { attributes } ) {
const {
url,
Expand Down Expand Up @@ -654,6 +668,13 @@ const v6 = {
},
},
},
migrate( { width, height, ...attributes } ) {
return {
...attributes,
width: `${ width }px`,
height: `${ height }px`,
};
},
save( { attributes } ) {
const {
url,
Expand Down
21 changes: 17 additions & 4 deletions packages/block-library/src/image/save.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,30 @@ export default function save( { attributes } ) {
[ `wp-image-${ id }` ]: !! id,
} );

const imgStyle = { aspectRatio, objectFit: scale };

// Branch by whether width and height both have size.
// If the image block is very old and has not been migrated to a string type, the size will be int.
// Note that only width may be "auto".
if ( ! aspectRatio && height && width && 'auto' !== width ) {
Comment on lines +56 to +57
Copy link
Contributor

Choose a reason for hiding this comment

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

height can be set to auto in the editor with a fixed width, so we can't make this assumption here. We want to support layouts where that is required.

const w = parseInt( width );
const h = parseInt( height );
Comment on lines +58 to +59
Copy link
Contributor

Choose a reason for hiding this comment

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

The idea for the image block moving forward is to support units other than px, so we don't want to parseInt without confirming that they are pixel values first.

if ( ! isNaN( w ) && ! isNaN( h ) ) {
imgStyle.aspectRatio = `${ w }/${ h }`;
}
} else {
if ( 'string' === typeof width ) imgStyle.width = width;
if ( 'string' === typeof height ) imgStyle.height = height;
}
Comment on lines +57 to +66
Copy link
Contributor

Choose a reason for hiding this comment

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

The logic here should be moved to edit.js to allow for more flexibility of styling.


const image = (
<img
src={ url }
alt={ alt }
className={ imageClasses || undefined }
style={ {
...borderProps.style,
aspectRatio,
objectFit: scale,
width,
height,
...imgStyle,
} }
title={ title }
/>
Expand Down
Loading