Skip to content

Commit

Permalink
Merge pull request #14763 from ckeditor/ck/epic/14146-support-image-h…
Browse files Browse the repository at this point in the history
…eight-attribute

Feature (image): The image `width` and `height` attributes are preserved while loading editor content. Closes #14146.

Feature (image): Images without size specified will automatically gain natural image size on any interaction with the image. See #14146.

MAJOR BREAKING CHANGE (image): The model attribute name of the resized image is now changed to `resizedWidth`. The `width` and `height` attributes are now used to preserve the image's natural width and height.

MAJOR BREAKING CHANGE (image): The `srcset` model attribute has been simplified. It is no longer an object `{ data: "...", width: "..." }`, but a value previously stored in the `data` part.
  • Loading branch information
niegowski authored Sep 14, 2023
2 parents 8ae1200 + 84d77c6 commit 58e9c88
Show file tree
Hide file tree
Showing 62 changed files with 2,336 additions and 550 deletions.
2 changes: 1 addition & 1 deletion packages/ckeditor5-engine/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export type { Marker } from './model/markercollection';
export type { default as Batch } from './model/batch';
export type { default as Differ, DiffItem, DiffItemAttribute, DiffItemInsert, DiffItemRemove } from './model/differ';
export type { default as Item } from './model/item';
export type { default as Node } from './model/node';
export type { default as Node, NodeAttributes } from './model/node';
export type { default as RootElement } from './model/rootelement';
export type {
default as Schema,
Expand Down
4 changes: 4 additions & 0 deletions packages/ckeditor5-html-support/tests/integrations/image.js
Original file line number Diff line number Diff line change
Expand Up @@ -2475,6 +2475,8 @@ describe( 'ImageElementSupport', () => {
'src',
'srcset',
'linkHref',
'width',
'height',
'htmlImgAttributes',
'htmlFigureAttributes',
'htmlLinkAttributes'
Expand Down Expand Up @@ -2509,6 +2511,8 @@ describe( 'ImageElementSupport', () => {
'alt',
'src',
'srcset',
'width',
'height',
'htmlA',
'htmlImgAttributes'
] );
Expand Down
2 changes: 2 additions & 0 deletions packages/ckeditor5-image/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
"@ckeditor/ckeditor5-cloud-services": "39.0.2",
"@ckeditor/ckeditor5-core": "39.0.2",
"@ckeditor/ckeditor5-dev-utils": "^38.0.0",
"@ckeditor/ckeditor5-html-support": "39.0.2",
"@ckeditor/ckeditor5-paste-from-office": "39.0.2",
"@ckeditor/ckeditor5-easy-image": "39.0.2",
"@ckeditor/ckeditor5-editor-classic": "39.0.2",
"@ckeditor/ckeditor5-engine": "39.0.2",
Expand Down
2 changes: 2 additions & 0 deletions packages/ckeditor5-image/src/augmentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import type {
ImageCaptionUtils,
ImageInsertUI,
ImageResizeEditing,
ImageSizeAttributes,
ImageStyleEditing,
ImageStyleUI,
ImageTextAlternativeEditing,
Expand Down Expand Up @@ -76,6 +77,7 @@ declare module '@ckeditor/ckeditor5-core' {
[ ImageCaptionUtils.pluginName ]: ImageCaptionUtils;
[ ImageInsertUI.pluginName ]: ImageInsertUI;
[ ImageResizeEditing.pluginName ]: ImageResizeEditing;
[ ImageSizeAttributes.pluginName ]: ImageSizeAttributes;
[ ImageStyleEditing.pluginName ]: ImageStyleEditing;
[ ImageStyleUI.pluginName ]: ImageStyleUI;
[ ImageTextAlternativeEditing.pluginName ]: ImageTextAlternativeEditing;
Expand Down
26 changes: 5 additions & 21 deletions packages/ckeditor5-image/src/image/converters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ import type {
import { first, type GetCallback } from 'ckeditor5/src/utils';
import type ImageUtils from '../imageutils';

type SrcsetAttributeType = null | { data: unknown; width: unknown };

/**
* Returns a function that converts the image view representation:
*
Expand Down Expand Up @@ -178,7 +176,7 @@ export function upcastPicture( imageUtils: ImageUtils ): ( dispatcher: UpcastDis
}

/**
* Converter used to convert the `srcset` model image attribute to the `srcset`, `sizes` and `width` attributes in the view.
* Converter used to convert the `srcset` model image attribute to the `srcset` and `sizes` attributes in the view.
*
* @internal
* @param imageType The type of the image.
Expand All @@ -197,27 +195,13 @@ export function downcastSrcsetAttribute(
const img = imageUtils.findViewImgElement( element )!;

if ( data.attributeNewValue === null ) {
const srcset = data.attributeOldValue as SrcsetAttributeType;

if ( srcset && srcset.data ) {
writer.removeAttribute( 'srcset', img );
writer.removeAttribute( 'sizes', img );

if ( srcset.width ) {
writer.removeAttribute( 'width', img );
}
}
writer.removeAttribute( 'srcset', img );
writer.removeAttribute( 'sizes', img );
} else {
const srcset = data.attributeNewValue as SrcsetAttributeType;

if ( srcset && srcset.data ) {
writer.setAttribute( 'srcset', srcset.data, img );
if ( data.attributeNewValue ) {
writer.setAttribute( 'srcset', data.attributeNewValue, img );
// Always outputting `100vw`. See https://github.com/ckeditor/ckeditor5-image/issues/2.
writer.setAttribute( 'sizes', '100vw', img );

if ( srcset.width ) {
writer.setAttribute( 'width', srcset.width, img );
}
}
}
};
Expand Down
3 changes: 2 additions & 1 deletion packages/ckeditor5-image/src/image/imageblockediting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
} from './converters';

import ImageEditing from './imageediting';
import ImageSizeAttributes from '../imagesizeattributes';
import ImageTypeCommand from './imagetypecommand';
import ImageUtils from '../imageutils';
import {
Expand All @@ -41,7 +42,7 @@ export default class ImageBlockEditing extends Plugin {
* @inheritDoc
*/
public static get requires() {
return [ ImageEditing, ImageUtils, ClipboardPipeline ] as const;
return [ ImageEditing, ImageSizeAttributes, ImageUtils, ClipboardPipeline ] as const;
}

/**
Expand Down
15 changes: 1 addition & 14 deletions packages/ckeditor5-image/src/image/imageediting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,20 +59,7 @@ export default class ImageEditing extends Plugin {
name: 'img',
key: 'srcset'
},
model: {
key: 'srcset',
value: ( viewImage: ViewElement ) => {
const value: Record<string, string> = {
data: viewImage.getAttribute( 'srcset' )!
};

if ( viewImage.hasAttribute( 'width' ) ) {
value.width = viewImage.getAttribute( 'width' )!;
}

return value;
}
}
model: 'srcset'
} );

const insertImageCommand = new InsertImageCommand( editor );
Expand Down
3 changes: 2 additions & 1 deletion packages/ckeditor5-image/src/image/imageinlineediting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
} from './converters';

import ImageEditing from './imageediting';
import ImageSizeAttributes from '../imagesizeattributes';
import ImageTypeCommand from './imagetypecommand';
import ImageUtils from '../imageutils';
import {
Expand All @@ -40,7 +41,7 @@ export default class ImageInlineEditing extends Plugin {
* @inheritDoc
*/
public static get requires() {
return [ ImageEditing, ImageUtils, ClipboardPipeline ] as const;
return [ ImageEditing, ImageSizeAttributes, ImageUtils, ClipboardPipeline ] as const;
}

/**
Expand Down
25 changes: 25 additions & 0 deletions packages/ckeditor5-image/src/image/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,3 +135,28 @@ export function determineImageTypeForInsertionAtSelection(
// Otherwise insert an inline image.
return 'imageInline';
}

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

return null;
}

/**
* Returns true if both styles (width and height) are set.
*
* 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.
*/
export function widthAndHeightStylesAreBothSet( viewElement: ViewElement ): boolean {
const widthStyle = getSizeValueIfInPx( viewElement.getStyle( 'width' ) );
const heightStyle = getSizeValueIfInPx( viewElement.getStyle( 'height' ) );

return !!( widthStyle && heightStyle );
}
79 changes: 69 additions & 10 deletions packages/ckeditor5-image/src/imageresize/imageresizeediting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { ViewElement } from 'ckeditor5/src/engine';
import { type Editor, Plugin } from 'ckeditor5/src/core';
import ImageUtils from '../imageutils';
import ResizeImageCommand from './resizeimagecommand';
import { widthAndHeightStylesAreBothSet } from '../image/utils';

/**
* The image resize editing feature.
Expand Down Expand Up @@ -82,11 +83,11 @@ export default class ImageResizeEditing extends Plugin {

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

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

Expand All @@ -97,23 +98,55 @@ 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 =>
dispatcher.on( `attribute:width:${ imageType }`, ( evt, data, conversionApi ) => {
dispatcher.on( `attribute:resizedWidth:${ imageType }`, ( evt, data, conversionApi ) => {
if ( !conversionApi.consumable.consume( data.item, evt.name ) ) {
return;
}

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

if ( data.attributeNewValue !== null ) {
viewWriter.setStyle( 'width', data.attributeNewValue, figure );
viewWriter.addClass( 'image_resized', figure );
viewWriter.setStyle( 'width', data.attributeNewValue, viewImg );
viewWriter.addClass( 'image_resized', viewImg );
} else {
viewWriter.removeStyle( 'width', figure );
viewWriter.removeClass( 'image_resized', figure );
viewWriter.removeStyle( 'width', viewImg );
viewWriter.removeClass( 'image_resized', viewImg );
}
} )
);

editor.conversion.for( 'dataDowncast' ).attributeToAttribute( {
model: {
name: imageType,
key: 'resizedHeight'
},
view: modelAttributeValue => ( {
key: 'style',
value: {
'height': modelAttributeValue
}
} )
} );

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

const viewWriter = conversionApi.writer;
const viewImg = conversionApi.mapper.toViewElement( data.item );
const target = imageType === 'imageInline' ? imageUtils.findViewImgElement( viewImg ) : viewImg;

if ( data.attributeNewValue !== null ) {
viewWriter.setStyle( 'height', data.attributeNewValue, target );
} else {
viewWriter.removeStyle( 'height', target );
}
} )
);
Expand All @@ -127,8 +160,34 @@ export default class ImageResizeEditing extends Plugin {
}
},
model: {
key: 'width',
value: ( viewElement: ViewElement ) => viewElement.getStyle( 'width' )
key: 'resizedWidth',
value: ( viewElement: ViewElement ) => {
if ( widthAndHeightStylesAreBothSet( viewElement ) ) {
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 ) => {
if ( widthAndHeightStylesAreBothSet( viewElement ) ) {
return null;
}

return viewElement.getStyle( 'height' );
}
}
} );
}
Expand Down
18 changes: 13 additions & 5 deletions packages/ckeditor5-image/src/imageresize/imageresizehandles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@
* @module image/imageresize/imageresizehandles
*/

import type { Element, ViewContainerElement, ViewElement } from 'ckeditor5/src/engine';
import type { Element, 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 All @@ -22,8 +23,6 @@ const RESIZABLE_IMAGES_CSS_SELECTOR =
'span.image-inline.ck-widget > img,' +
'span.image-inline.ck-widget > picture > img';

const IMAGE_WIDGETS_CLASSES_MATCH_REGEXP = /(image|image-inline)/;

const RESIZED_IMAGE_CLASS = 'image_resized';

/**
Expand All @@ -37,7 +36,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 +62,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 All @@ -74,7 +74,7 @@ export default class ImageResizeHandles extends Plugin {

const domConverter = editor.editing.view.domConverter;
const imageView = domConverter.domToView( domEvent.target as HTMLElement ) as ViewElement;
const widgetView = imageView.findAncestor( { classes: IMAGE_WIDGETS_CLASSES_MATCH_REGEXP } ) as ViewContainerElement;
const widgetView = imageUtils.getImageWidgetFromImageView( imageView )!;
let resizer = this.editor.plugins.get( WidgetResize ).getResizerByViewElement( widgetView );

if ( resizer ) {
Expand Down Expand Up @@ -130,6 +130,14 @@ 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.bind( 'isEnabled' ).to( this );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ export default class ResizeImageCommand extends Command {

this.isEnabled = !!element;

if ( !element || !element.hasAttribute( 'width' ) ) {
if ( !element || !element.hasAttribute( 'resizedWidth' ) ) {
this.value = null;
} else {
this.value = {
width: element.getAttribute( 'width' ) as string,
width: element.getAttribute( 'resizedWidth' ) as string,
height: null
};
}
Expand Down Expand Up @@ -71,7 +71,9 @@ export default class ResizeImageCommand extends Command {

if ( imageElement ) {
model.change( writer => {
writer.setAttribute( 'width', options.width, imageElement );
writer.setAttribute( 'resizedWidth', options.width, imageElement );
writer.removeAttribute( 'resizedHeight', imageElement );
imageUtils.setImageNaturalSizeAttributes( imageElement );
} );
}
}
Expand Down
Loading

0 comments on commit 58e9c88

Please sign in to comment.