Skip to content

Commit

Permalink
Merge pull request #14544 from ckeditor/ck/14542-images-wider-than-ed…
Browse files Browse the repository at this point in the history
…itor-are-shrunk-horizontally

Task (image): Images that are wider than the editor should be displayed in the correct proportions. Closes #14542.
  • Loading branch information
mmotyczynska authored Jul 11, 2023
2 parents c249d94 + afc03de commit aec7511
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 63 deletions.
13 changes: 0 additions & 13 deletions packages/ckeditor5-image/src/imageresize/imageresizehandles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,19 +142,6 @@ export default class ImageResizeHandles extends Plugin {
}
} );

resizer.on( 'begin', () => {
const img = imageUtils.findViewImgElement( imageView )!;
const aspectRatio = img.getStyle( 'aspect-ratio' );
const widthAttr = imageModel.getAttribute( 'width' );
const heightAttr = imageModel.getAttribute( 'height' );

if ( widthAttr && heightAttr && !aspectRatio ) {
editingView.change( writer => {
writer.setStyle( 'aspect-ratio', `${ widthAttr }/${ heightAttr }`, img );
} );
}
} );

resizer.bind( 'isEnabled' ).to( this );
} );
}
Expand Down
25 changes: 18 additions & 7 deletions packages/ckeditor5-image/src/imagesizeattributes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,19 @@ export default class ImageSizeAttributes extends Plugin {
}
} );

// Dedicated converter to propagate attributes to the <img> element.
editor.conversion.for( 'downcast' ).add( dispatcher => {
attachDowncastConverter( dispatcher, 'width', 'width' );
attachDowncastConverter( dispatcher, 'height', 'height' );
// Dedicated converters to propagate attributes to the <img> element.
editor.conversion.for( 'editingDowncast' ).add( dispatcher => {
attachDowncastConverter( dispatcher, 'width', 'width', true );
attachDowncastConverter( dispatcher, 'height', 'height', true );
} );

editor.conversion.for( 'dataDowncast' ).add( dispatcher => {
attachDowncastConverter( dispatcher, 'width', 'width', false );
attachDowncastConverter( dispatcher, 'height', 'height', false );
} );

function attachDowncastConverter(
dispatcher: DowncastDispatcher, modelAttributeName: string, viewAttributeName: string
dispatcher: DowncastDispatcher, modelAttributeName: string, viewAttributeName: string, setRatioForInlineImage: boolean
) {
dispatcher.on<DowncastAttributeEvent>( `attribute:${ modelAttributeName }:${ imageType }`, ( evt, data, conversionApi ) => {
if ( !conversionApi.consumable.consume( data.item, evt.name ) ) {
Expand All @@ -151,12 +156,18 @@ export default class ImageSizeAttributes extends Plugin {
viewWriter.removeAttribute( viewAttributeName, img );
}

const isResized = data.item.hasAttribute( 'resizedWidth' );

// Do not set aspect ratio for inline images which are not resized (data pipeline).
if ( imageType === 'imageInline' && !isResized && !setRatioForInlineImage ) {
return;
}

const width = data.item.getAttribute( 'width' );
const height = data.item.getAttribute( 'height' );
const isResized = data.item.hasAttribute( 'resizedWidth' );
const aspectRatio = img.getStyle( 'aspect-ratio' );

if ( width && height && !aspectRatio && isResized ) {
if ( width && height && !aspectRatio ) {
viewWriter.setStyle( 'aspect-ratio', `${ width }/${ height }`, img );
}
} );
Expand Down
31 changes: 0 additions & 31 deletions packages/ckeditor5-image/tests/imageresize/imageresizehandles.js
Original file line number Diff line number Diff line change
Expand Up @@ -687,37 +687,6 @@ describe( 'ImageResizeHandles', () => {
} );
} );

describe( 'aspect-ratio style', () => {
beforeEach( async () => {
editor = await createEditor();

await setModelAndWaitForImages( editor,
'<paragraph>[' +
`<imageInline width="100" height="50" imageStyle="side" src="${ IMAGE_SRC_FIXTURE }"></imageInline>` +
']</paragraph>'
);

widget = viewDocument.getRoot().getChild( 0 ).getChild( 0 );
} );

it( 'is added after starting resizing, if width & height attributes are set', () => {
const resizerPosition = 'bottom-left';
const domParts = getWidgetDomParts( editor, widget, resizerPosition );
const initialPointerPosition = getHandleCenterPoint( domParts.widget, resizerPosition );
const viewImage = widget.getChild( 0 );

expect( viewImage.getStyle( 'aspec-ratio' ) ).to.be.undefined;

resizerMouseSimulator.down( editor, domParts.resizeHandle );

resizerMouseSimulator.move( editor, domParts.resizeHandle, null, initialPointerPosition );

expect( viewImage.getStyle( 'aspect-ratio' ) ).to.equal( '100/50' );

resizerMouseSimulator.up( editor );
} );
} );

describe( 'undo integration', () => {
beforeEach( async () => {
editor = await createEditor();
Expand Down
10 changes: 5 additions & 5 deletions packages/ckeditor5-image/tests/imagesizeattributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -288,14 +288,14 @@ describe( 'ImageSizeAttributes', () => {
);
} );

it( 'should not add aspect-ratio if attributes are set but image is not resized', () => {
it( 'should add aspect-ratio if attributes are set but image is not resized (but to editing view only)', () => {
editor.setData(
'<p><img class="image_resized" width="100" height="200" src="/assets/sample.png" "></p>'
);

expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
'<p><span class="ck-widget image-inline" contenteditable="false">' +
'<img height="200" src="/assets/sample.png" width="100"></img>' +
'<img height="200" src="/assets/sample.png" style="aspect-ratio:100/200" width="100"></img>' +
'</span></p>'
);

Expand Down Expand Up @@ -424,20 +424,20 @@ describe( 'ImageSizeAttributes', () => {
);
} );

it( 'should not add aspect-ratio if attributes are set but image is not resized', () => {
it( 'should add aspect-ratio if attributes are set but image is not resized', () => {
editor.setData(
'<figure class="image"><img width="100" height="200" src="/assets/sample.png"></figure>'
);

expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
'<figure class="ck-widget image" contenteditable="false">' +
'<img height="200" src="/assets/sample.png" width="100"></img>' +
'<img height="200" src="/assets/sample.png" style="aspect-ratio:100/200" width="100"></img>' +
'</figure>'
);

expect( editor.getData() ).to.equal(
'<figure class="image">' +
'<img src="/assets/sample.png" width="100" height="200">' +
'<img style="aspect-ratio:100/200;" src="/assets/sample.png" width="100" height="200">' +
'</figure>'
);
} );
Expand Down
10 changes: 10 additions & 0 deletions packages/ckeditor5-image/theme/image.css
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@

/* Make sure the image is never smaller than the parent container (See: https://github.com/ckeditor/ckeditor5/issues/9300). */
min-width: 100%;

/* Keep proportions of the block image if the height is set and the image is wider than the editor width.
See https://github.com/ckeditor/ckeditor5/issues/14542. */
height: auto;
}
}

Expand Down Expand Up @@ -105,6 +109,12 @@
}
}

/* Keep proportions of the inline image if the height is set and the image is wider than the editor width.
See https://github.com/ckeditor/ckeditor5/issues/14542. */
& .image-inline img {
height: auto;
}

/* The inline image nested in the table should have its original size if not resized.
See https://github.com/ckeditor/ckeditor5/issues/9117. */
& td,
Expand Down
7 changes: 0 additions & 7 deletions packages/ckeditor5-image/theme/imageresize.css
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
*/

/* Preserve aspect ratio of the resized image after introducing image height attribute. */
.ck-content figure.image_resized img,
.ck-content img.image_resized {
height: auto;
}
Expand All @@ -31,12 +30,6 @@
}

.ck.ck-editor__editable {
/* Preserve aspect ratio of the resized image after introducing image height attribute. */
& .image.image_resized img,
& .image-inline.image_resized img {
height: auto;
}

/* The resized inline image nested in the table should respect its parent size.
See https://github.com/ckeditor/ckeditor5/issues/9117. */
& td,
Expand Down

0 comments on commit aec7511

Please sign in to comment.