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.

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

Add image resizedHeight model attribute (convert from style height) #14520

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
82975fe
Add converter for image resizedHeight.
mmotyczynska Jun 7, 2023
65065ab
Remove resizedHeight on image resize.
mmotyczynska Jun 7, 2023
c80b762
Set inline aspect-ratio on image in editing downcast.
mmotyczynska Jun 12, 2023
c2b8cff
Update resizedHeight on image resize (when resize unit is px).
mmotyczynska Jun 13, 2023
8345334
Add PFO to manual test with image size attributes.
mmotyczynska Jun 13, 2023
d3a2752
Add manual test for image size attributes with ghs.
mmotyczynska Jun 13, 2023
226d1cd
Revert "Update resizedHeight on image resize (when resize unit is px)."
mmotyczynska Jun 16, 2023
6ba54a6
Reverse changes in image.css.
mmotyczynska Jun 21, 2023
603e961
Add height: auto for resized images (downcast).
mmotyczynska Jun 21, 2023
007ddec
Change upcast and downcast of image attributes.
mmotyczynska Jun 21, 2023
7d429e3
Set aspect-ratio for image view when resizing begins.
mmotyczynska Jun 21, 2023
1f1be62
Manual test with many image resize attributes test cases.
mmotyczynska Jun 21, 2023
fc13ab7
Upcast height style to height attribute if no other styles/attributes…
mmotyczynska Jun 23, 2023
e29e786
Convert resizedHeight to resizedWidth if there are attributes.
mmotyczynska Jun 23, 2023
02578a8
Correct manual test for image resize in px style only.
mmotyczynska Jun 26, 2023
470be3f
Upcast image aspect-ratio.
mmotyczynska Jun 26, 2023
02b1fb0
Revert "Upcast image aspect-ratio."
mmotyczynska Jun 26, 2023
941e9d1
Revert "Convert resizedHeight to resizedWidth if there are attributes."
mmotyczynska Jun 26, 2023
9ccaf9e
Revert "Upcast height style to height attribute if no other styles/at…
mmotyczynska Jun 26, 2023
bbb9f4a
Do not remove image_resized class when removing style height but styl…
mmotyczynska Jun 26, 2023
58ec57d
Remove unused styles.
mmotyczynska Jun 26, 2023
aafb59e
For image inline in editing set height style on img instead of span.
mmotyczynska Jun 26, 2023
e4faaa8
Remove style height from img after starting resizing.
mmotyczynska Jun 28, 2023
5a851e5
Remove style height after starting resizing - fix for block images.
mmotyczynska Jun 28, 2023
d8cea9a
Tests: image resize schema allowed attributes.
mmotyczynska Jun 30, 2023
7b3e614
Refactor editing downcast for resizedHeight.
mmotyczynska Jun 30, 2023
ecbd6e9
Tests: image block resizedHeight downcast.
mmotyczynska Jun 30, 2023
4c71086
Tests: image inline resizedHeight downcast.
mmotyczynska Jun 30, 2023
eb3bf54
Tests: image resizedWidth & resizedHeight upcast.
mmotyczynska Jun 30, 2023
93d0efe
Tests: image upcast from styles to attributes.
mmotyczynska Jun 30, 2023
452bcf3
Tests: image downcast aspect-ration inline style.
mmotyczynska Jul 1, 2023
5280b85
Tests: style height and aspect-ratio after starting resizing.
mmotyczynska Jul 3, 2023
86789e1
Add description for new ck-content resized image css rules.
mmotyczynska Jul 3, 2023
c301b21
Tests: correction for image size attributes.
mmotyczynska Jul 3, 2023
a49fee6
Tests: image resize command should remove resizedHeight.
mmotyczynska Jul 3, 2023
d7ea66a
Tests: improve manual test for image attributes & styles with full gh…
mmotyczynska Jul 3, 2023
412de18
Tests: add description to manual test for image attributes.
mmotyczynska Jul 3, 2023
fc0300d
Tests: manual test for image attributes - typo.
mmotyczynska Jul 3, 2023
3210440
Enforce importing ImageUtils.
mmotyczynska Jul 5, 2023
1985558
Refactor.
mmotyczynska Jul 5, 2023
4decf19
Comment upcasting image styles to width/height instead of resizedWidt…
mmotyczynska Jul 5, 2023
9d7fda9
Tests: update comment.
mmotyczynska Jul 5, 2023
662472b
Enforce importing ImageUtils.
mmotyczynska Jul 5, 2023
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
2 changes: 2 additions & 0 deletions packages/ckeditor5-image/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,13 @@
"@ckeditor/ckeditor5-essentials": "^38.0.1",
"@ckeditor/ckeditor5-heading": "^38.0.1",
"@ckeditor/ckeditor5-html-embed": "^38.0.1",
"@ckeditor/ckeditor5-html-support": "^38.0.1",
"@ckeditor/ckeditor5-indent": "^38.0.1",
"@ckeditor/ckeditor5-link": "^38.0.1",
"@ckeditor/ckeditor5-list": "^38.0.1",
"@ckeditor/ckeditor5-media-embed": "^38.0.1",
"@ckeditor/ckeditor5-paragraph": "^38.0.1",
"@ckeditor/ckeditor5-paste-from-office": "^38.0.1",
"@ckeditor/ckeditor5-table": "^38.0.1",
"@ckeditor/ckeditor5-theme-lark": "^38.0.1",
"@ckeditor/ckeditor5-typing": "^38.0.1",
Expand Down
74 changes: 71 additions & 3 deletions packages/ckeditor5-image/src/imageresize/imageresizeediting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,11 @@ export default class ImageResizeEditing extends Plugin {

private _registerSchema(): void {
if ( this.editor.plugins.has( 'ImageBlockEditing' ) ) {
this.editor.model.schema.extend( 'imageBlock', { allowAttributes: 'resizedWidth' } );
this.editor.model.schema.extend( 'imageBlock', { allowAttributes: [ 'resizedWidth', 'resizedHeight' ] } );
}

if ( this.editor.plugins.has( 'ImageInlineEditing' ) ) {
this.editor.model.schema.extend( 'imageInline', { allowAttributes: 'resizedWidth' } );
this.editor.model.schema.extend( 'imageInline', { allowAttributes: [ 'resizedWidth', 'resizedHeight' ] } );
}
}

Expand All @@ -97,6 +97,7 @@ export default class ImageResizeEditing extends Plugin {
*/
private _registerConverters( imageType: 'imageBlock' | 'imageInline' ) {
const editor = this.editor;
const imageUtils: ImageUtils = editor.plugins.get( 'ImageUtils' );

// Dedicated converter to propagate image's attribute to the img tag.
editor.conversion.for( 'downcast' ).add( dispatcher =>
Expand All @@ -118,6 +119,35 @@ export default class ImageResizeEditing extends Plugin {
} )
);

editor.conversion.for( 'dataDowncast' ).add( dispatcher =>
dispatcher.on( `attribute:resizedHeight:${ imageType }`, ( evt, data, conversionApi ) => {
if ( !conversionApi.consumable.consume( data.item, evt.name ) ) {
return;
}

const viewWriter = conversionApi.writer;
const viewElement = conversionApi.mapper.toViewElement( data.item );

viewWriter.setStyle( 'height', data.attributeNewValue, viewElement );
} )
);

editor.conversion.for( 'editingDowncast' ).add( dispatcher =>
dispatcher.on( `attribute:resizedHeight:${ imageType }`, ( evt, data, conversionApi ) => {
if ( !conversionApi.consumable.consume( data.item, evt.name ) ) {
return;
}

if ( data.attributeNewValue !== null ) {
const viewWriter = conversionApi.writer;
const figure = conversionApi.mapper.toViewElement( data.item );
const target = imageType === 'imageInline' ? imageUtils.findViewImgElement( figure ) : figure;

viewWriter.setStyle( 'height', data.attributeNewValue, target );
arkflpc marked this conversation as resolved.
Show resolved Hide resolved
}
} )
);

editor.conversion.for( 'upcast' )
.attributeToAttribute( {
view: {
Expand All @@ -128,7 +158,45 @@ export default class ImageResizeEditing extends Plugin {
},
model: {
key: 'resizedWidth',
value: ( viewElement: ViewElement ) => viewElement.getStyle( 'width' )
value: ( viewElement: ViewElement ) => {
const widthStyle = imageUtils.getSizeInPx( viewElement.getStyle( 'width' ) );
const heightStyle = imageUtils.getSizeInPx( viewElement.getStyle( 'height' ) );

// If both image styles: width & height are set, they will override the image width & height attributes in the
// browser. In this case, the image looks the same as if these styles were applied to attributes instead of styles.
// That's why we can upcast these styles to width & height attributes instead of resizedWidth and resizedHeight.
if ( widthStyle && heightStyle ) {
arkflpc marked this conversation as resolved.
Show resolved Hide resolved
return null;
}

return viewElement.getStyle( 'width' );
}
}
} );

editor.conversion.for( 'upcast' )
.attributeToAttribute( {
view: {
name: imageType === 'imageBlock' ? 'figure' : 'img',
styles: {
height: /.+/
}
},
model: {
key: 'resizedHeight',
value: ( viewElement: ViewElement ) => {
const widthStyle = imageUtils.getSizeInPx( viewElement.getStyle( 'width' ) );
const heightStyle = imageUtils.getSizeInPx( viewElement.getStyle( 'height' ) );

// If both image styles: width & height are set, they will override the image width & height attributes in the
// browser. In this case, the image looks the same as if these styles were applied to attributes instead of styles.
// That's why we can upcast these styles to width & height attributes instead of resizedWidth and resizedHeight.
if ( widthStyle && heightStyle ) {
return null;
}

return viewElement.getStyle( 'height' );
}
}
} );
}
Expand Down
25 changes: 24 additions & 1 deletion packages/ckeditor5-image/src/imageresize/imageresizehandles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import type { Element, ViewContainerElement, ViewElement } from 'ckeditor5/src/engine';
import { Plugin } from 'ckeditor5/src/core';
import { WidgetResize } from 'ckeditor5/src/widget';
import ImageUtils from '../imageutils';

import ImageLoadObserver, { type ImageLoadedEvent } from '../image/imageloadobserver';
import type ResizeImageCommand from './resizeimagecommand';
Expand Down Expand Up @@ -37,7 +38,7 @@ export default class ImageResizeHandles extends Plugin {
* @inheritDoc
*/
public static get requires() {
return [ WidgetResize ] as const;
return [ WidgetResize, ImageUtils ] as const;
}

/**
Expand All @@ -63,6 +64,7 @@ export default class ImageResizeHandles extends Plugin {
private _setupResizerCreator(): void {
const editor = this.editor;
const editingView = editor.editing.view;
const imageUtils: ImageUtils = editor.plugins.get( 'ImageUtils' );

editingView.addObserver( ImageLoadObserver );

Expand Down Expand Up @@ -130,6 +132,27 @@ export default class ImageResizeHandles extends Plugin {
writer.addClass( RESIZED_IMAGE_CLASS, widgetView );
} );
}

const target = imageModel.name === 'imageInline' ? imageView : widgetView;

if ( target.getStyle( 'height' ) ) {
editingView.change( writer => {
writer.removeStyle( 'height', target );
} );
}
} );

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
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export default class ResizeImageCommand extends Command {
if ( imageElement ) {
model.change( writer => {
writer.setAttribute( 'resizedWidth', options.width, imageElement );
writer.removeAttribute( 'resizedHeight', imageElement );
} );
}
}
Expand Down
56 changes: 55 additions & 1 deletion packages/ckeditor5-image/src/imagesizeattributes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,27 @@ export default class ImageSizeAttributes extends Plugin {
const viewElementName = imageType === 'imageBlock' ? 'figure' : 'img';

editor.conversion.for( 'upcast' )
.attributeToAttribute( {
view: {
name: imageType === 'imageBlock' ? 'figure' : 'img',
styles: {
width: /.+/
}
},
model: {
key: 'width',
value: ( viewElement: ViewElement ) => {
const widthStyle = imageUtils.getSizeInPx( viewElement.getStyle( 'width' ) );
const heightStyle = imageUtils.getSizeInPx( viewElement.getStyle( 'height' ) );

if ( widthStyle && heightStyle ) {
return widthStyle;
}

return null;
}
}
} )
.attributeToAttribute( {
view: {
name: viewElementName,
Expand All @@ -72,6 +93,27 @@ export default class ImageSizeAttributes extends Plugin {
value: ( viewElement: ViewElement ) => viewElement.getAttribute( 'width' )
}
} )
.attributeToAttribute( {
view: {
name: imageType === 'imageBlock' ? 'figure' : 'img',
styles: {
height: /.+/
}
},
model: {
key: 'height',
value: ( viewElement: ViewElement ) => {
const widthStyle = imageUtils.getSizeInPx( viewElement.getStyle( 'width' ) );
const heightStyle = imageUtils.getSizeInPx( viewElement.getStyle( 'height' ) );

if ( widthStyle && heightStyle ) {
return heightStyle;
}

return null;
}
}
} )
.attributeToAttribute( {
view: {
name: viewElementName,
Expand All @@ -91,7 +133,9 @@ export default class ImageSizeAttributes extends Plugin {
attachDowncastConverter( dispatcher, 'height', 'height' );
} );

function attachDowncastConverter( dispatcher: DowncastDispatcher, modelAttributeName: string, viewAttributeName: string ) {
function attachDowncastConverter(
dispatcher: DowncastDispatcher, modelAttributeName: string, viewAttributeName: string
) {
dispatcher.on<DowncastAttributeEvent>( `attribute:${ modelAttributeName }:${ imageType }`, ( evt, data, conversionApi ) => {
if ( !conversionApi.consumable.consume( data.item, evt.name ) ) {
return;
Expand All @@ -106,7 +150,17 @@ export default class ImageSizeAttributes extends Plugin {
} else {
viewWriter.removeAttribute( viewAttributeName, img );
}

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 ) {
viewWriter.setStyle( 'aspect-ratio', `${ width }/${ height }`, img );
}
} );
}
}
}

11 changes: 11 additions & 0 deletions packages/ckeditor5-image/src/imageutils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,17 @@ export default class ImageUtils extends Plugin {

return super.destroy();
}

/**
* Returns parsed value of the size, but only if it contains unit: px.
*/
public getSizeInPx( size: string | undefined ): number | null {
if ( size && size.endsWith( 'px' ) ) {
return parseInt( size );
}

return null;
}
}

/**
Expand Down
Loading