ckeditor5 icon indicating copy to clipboard operation
ckeditor5 copied to clipboard

Setting image height and width

Open olup opened this issue 6 years ago β€’ 18 comments

Is this a bug report or feature request? (choose one)

πŸ†• Feature request

πŸ“ƒ Other details that might be useful

Hi ! Thanks for the very well designed ckeditor 5 and its image plugin. I have two / three questions that can be interpreted as feature requests.

Many front end, including mine, actually uses images parameters sizes to build progressive loading, placeholders, etc... We love our non-jumping image process, and for that it is critical that height and width are hardcoded in the img tag (indeed, only to compute an aspect ratio). From what I can understand in the image plugin code only url and alt and styles are setable on the image bloc object. Would it be possible to consider adding those parameters, or is it something i should extend myself (I am not talking PR but adding a plugin on top of this one) ?

Finally, something that don't really belong here, but i cannot find any equivalent plugin to manage embed (like a youtube video) in the editor. Is there work allready in this direction or is it uncharted territory ?

Cheers.

olup avatar Jun 27 '18 08:06 olup

From what I can understand in the image plugin code only url and alt and styles are setable on the image bloc object.

It also can also set the width, sizes and srcset. E.g. if I drop an image on https://ckeditor5.github.io that's the resulting data:

<figure class="image">
    <img src="https://33333.cdn.cke-cs.com/rc1DFuFpHqcR3Mah6y0e/images/fc0afb2a3bcfcd2b82139fdbc72eb063f4a8cc8d632a833a_sample.jpg" srcset="https://33333.cdn.cke-cs.com/rc1DFuFpHqcR3Mah6y0e/images/fc0afb2a3bcfcd2b82139fdbc72eb063f4a8cc8d632a833a_sample.jpg/w_80 80w, https://33333.cdn.cke-cs.com/rc1DFuFpHqcR3Mah6y0e/images/fc0afb2a3bcfcd2b82139fdbc72eb063f4a8cc8d632a833a_sample.jpg/w_160 160w, https://33333.cdn.cke-cs.com/rc1DFuFpHqcR3Mah6y0e/images/fc0afb2a3bcfcd2b82139fdbc72eb063f4a8cc8d632a833a_sample.jpg/w_240 240w, https://33333.cdn.cke-cs.com/rc1DFuFpHqcR3Mah6y0e/images/fc0afb2a3bcfcd2b82139fdbc72eb063f4a8cc8d632a833a_sample.jpg/w_320 320w, https://33333.cdn.cke-cs.com/rc1DFuFpHqcR3Mah6y0e/images/fc0afb2a3bcfcd2b82139fdbc72eb063f4a8cc8d632a833a_sample.jpg/w_400 400w, https://33333.cdn.cke-cs.com/rc1DFuFpHqcR3Mah6y0e/images/fc0afb2a3bcfcd2b82139fdbc72eb063f4a8cc8d632a833a_sample.jpg/w_480 480w, https://33333.cdn.cke-cs.com/rc1DFuFpHqcR3Mah6y0e/images/fc0afb2a3bcfcd2b82139fdbc72eb063f4a8cc8d632a833a_sample.jpg/w_560 560w, https://33333.cdn.cke-cs.com/rc1DFuFpHqcR3Mah6y0e/images/fc0afb2a3bcfcd2b82139fdbc72eb063f4a8cc8d632a833a_sample.jpg/w_640 640w, https://33333.cdn.cke-cs.com/rc1DFuFpHqcR3Mah6y0e/images/fc0afb2a3bcfcd2b82139fdbc72eb063f4a8cc8d632a833a_sample.jpg/w_720 720w, https://33333.cdn.cke-cs.com/rc1DFuFpHqcR3Mah6y0e/images/fc0afb2a3bcfcd2b82139fdbc72eb063f4a8cc8d632a833a_sample.jpg/w_800 800w" sizes="100vw" width="800">
</figure>

However, I don't understand why we don't set the height too... I think that we needed width so it works well with sizes and srcset. But, for some reason, we omit height.

@pjasiun @oleq, do you remember any reasons for not setting the height?

Finally, something that don't really belong here, but i cannot find any equivalent plugin to manage embed (like a youtube video) in the editor. Is there work allready in this direction or is it uncharted territory ?

Please report it in a separate ticket. Video embedding (from YT, Vimeo, etc) is high on our priority list, but it wasn't reported yet.

Reinmar avatar Jun 27 '18 09:06 Reinmar

@pjasiun @oleq, do you remember any reasons for not setting the height?

I believe we wanted to set as few attributes as possible, and we did not see any reason to set height. However, your argument, @olup, is very good and I agree that we should set it.

pjasiun avatar Jun 27 '18 09:06 pjasiun

Is there a way to parse existing width attributes from HTML when using editor.setData? I tried to extend image support (perhaps not quite correct):

editor.model.schema.extend('image', {
  allowAttributes: 'imageWidth'
});

editor.conversion.for('downcast').add(downcastAttributeToAttribute({
  model: 'imageWidth',
  view: 'width'
}));

editor.conversion.for('upcast').add(upcastAttributeToAttribute({
  model: 'imageWidth',
  view: 'width'
}));

But achieved only this after editor.getData():

<figure class="image" width="100"><img src="..."></figure>

We need to set width on images because we use CKEditor for creating emails, and we can't use stylesheets to apply styling to content.

the-owl avatar Oct 02 '18 14:10 the-owl

Related: https://github.com/ckeditor/ckeditor5-table/issues/122#issuecomment-424333590. Probably the same will happen with other widgets and/or elements that are not mapped 1:1.

jodator avatar Oct 02 '18 15:10 jodator

@the-owl : please take a look at a similar solution for tables: https://github.com/ckeditor/ckeditor5-table/issues/122#issuecomment-426982275

I've re-phrased the downcast writer to match your code - but I didn't test it ;)

editor.conversion.for( 'downcast' ).add( dispatcher => {
	dispatcher.on( 'attribute:imageWidth:image', ( evt, data, conversionApi ) => {
		const image = data.item;

		// The table from the model is mapped to the widget element: <figure>.
		const viewFigure = conversionApi.mapper.toViewElement( image );

		// A <image> is direct child of a <figure> but there might be other view
		// (including UI) elments inside <figure>.
		const viewImage = [ ...viewFigure.getChildren() ]
			.find( element => element.name === 'img' );

		// User view writer to change style of a view table.
		if ( data.attributeNewValue ) {
			conversionApi.writer.setStyle( 'width', data.attributeNewValue, viewImage );
		} else {
			conversionApi.writer.removeStyle( 'width', viewImage );
		}
	} );
} );

jodator avatar Oct 04 '18 11:10 jodator

Hi, Should I Need to write plugin for this by myself and build the ckeditor ?

100cm avatar Oct 31 '18 03:10 100cm

Will the solution implemented as part of this also include resizing media embeds? Should I open a separate issue in https://github.com/ckeditor/ckeditor5-media-embed ?

mkopinsky avatar Feb 19 '19 20:02 mkopinsky

My team needs this also πŸ‘

tbredin avatar Aug 26 '19 03:08 tbredin

We've just released image resizing. You can see it live in nightly docs https://ckeditor5.github.io/docs/nightly/ckeditor5/latest/features/image.html#resizing-images. Official docs will be updated later on today.

Reinmar avatar Aug 26 '19 11:08 Reinmar

I also need this feature.

Was it implemented? Some of the comments above are talking about image resizing etc. but I don't think that was what the OP was asking about.

Happy to write my own plugin... but it'd be amazing if someone has already done this ;)!?

-- EDIT

I thought I'd go ahead and do this, however I'm slightly confused as the viewToDom function won't return me a DOM node for the image. Since they stored in a WeakMap theres no way for me to iterate and get the correct image.

If I try and create a new image here, that'll obviously happen async and writer seems to fail after.

Is this the correct approach or is there a better way?

this.editor.conversion.for( 'downcast' ).add( dispatcher => {
  dispatcher.on( 'insert:image', ( evt, data, conversionApi ) => {
    const modelElement = data.item;
    const writer = conversionApi.writer;
    const viewFigure = conversionApi.mapper.toViewElement( modelElement );
    if ( !viewFigure || !writer ) {
      return;
    }
    const viewElement = findViewChild( viewFigure, 'img', conversionApi )
    const height = modelElement.getAttribute( 'height' ) || 0;
   if ( height > 0 ) {
      writer.setAttribute( 'height', height, viewElement );
      return;
    }
    const img = this.editor.editing.view.domConverter.viewToDom( viewElement );
     // img is undefined
  } );
}, { priority: 'low' } );

rhysstubbs avatar Oct 13 '20 09:10 rhysstubbs

We love our non-jumping image process, and for that it is critical that height and width are hardcoded in the img tag

Agreed. Otherwise we see jumps like this, which are very jarring if the image loads slowly:

neongreen avatar Nov 24 '20 18:11 neongreen

Related issue: #8663.

Reinmar avatar Jul 19 '21 10:07 Reinmar

I do not understand how this is related to #8663. Could you elaborate? 😊 πŸ™

wimleers avatar Jul 19 '21 10:07 wimleers

Currently, CKE5 does support width but AFAICS only if it's paired with sizes or something similar. E.g. the width attribute will be output when an image was pasted into the editor (from file). Additionally, with ImageResize present, the width inline style present on figure is also handled.

#8663 is about backward compat with CKE4 and we'll need to work in it on different notations of similar things. So if CKE4 could've output something, it will need to be handled in CKE5 as well.

Reinmar avatar Jul 19 '21 10:07 Reinmar

Hello, will the height setting be possible since that functionality has been implemented? This is important for SVG source of images. Currently, when you insert image and set SVG as its source, the height is capped at 150px, beyond which it is not possible to resize the image, making the scalability an issue. I spent already a week on this but cannot find a way out beyond manually setting the height and width of the image.

tomitrescak avatar Aug 23 '21 23:08 tomitrescak

This feature is needed by many people, especially for creating emails. Outlook and Windows 10 Mail rendering engine doesn’t understand the style attribute on <img> elements. So we need to define a width attribute for correct displaying. And it would be nice if ImageResize plugin allowed us to set image sizes using this attributes.

SazanYa avatar Jun 28 '22 09:06 SazanYa

pexels-cottonbro-4761788

suracekumar632 avatar Jul 12 '22 04:07 suracekumar632

Bump on this, img tags are getting width and height stripped from the rendered img tag. Any update on this?

jc-harbour avatar Aug 10 '22 16:08 jc-harbour

This has been reported as a bug against Drupal too now: https://www.drupal.org/project/drupal/issues/3336446#comment-14888366.

This bug has been known for a while now β€” is it really so complicated to fix? πŸ˜…

wimleers avatar Jan 26 '23 12:01 wimleers

It would be really nice to have this fixed, agree. We and our users love CKE5 on our websites, but the CLS score has been hit quite severely on some of our pages due to the arguments missing.

KlemenDEV avatar Jan 26 '23 18:01 KlemenDEV

I'm working on an image-dimensions plugin. Just have to hook up the observable properties.

image

but this would 100% be better as a first party plugin.

leevigraham avatar Feb 12 '23 10:02 leevigraham

@leevigraham Could you share the code? 😊 πŸ™

wimleers avatar May 04 '23 11:05 wimleers

@wimleers I didn't get much further than the screenshot above.

leevigraham avatar May 05 '23 04:05 leevigraham

We are taking a look at this, if anyone wants to check the status, observe the https://github.com/ckeditor/ckeditor5/issues/14146 and its subtasks.

Witoso avatar May 22 '23 11:05 Witoso

The implementation is close to completion, but we would love to hear some feedback. The details are gathered in the comment.

Witoso avatar Jul 14 '23 09:07 Witoso

:tada: This feature was merged to the master and is available on the nightly releases and nightly docs to test.

In a comment in another issue, you can find more details about the changes.

Gathering interest in the UI setup for those attributes in: #15044

Witoso avatar Sep 21 '23 15:09 Witoso