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

CKEditor: Extra image options when inserting MediaItem from MediaLibrary in editor #2411

Open
bramds opened this Issue Jan 26, 2018 · 25 comments

Comments

Projects
None yet
6 participants
@bramds
Contributor

bramds commented Jan 26, 2018

Type

  • Feature

Problem description

Since we have dropped ckfinder, we now include images in the editor through the media libray. Now we can set the path and the alt text of the image. In some cases it could be usefull that you re able to also set:

  • image height and width ( recalculated like in ckfinder )
  • class for the image

This way we can control the image more in the frontend. ( setting width and height with the css class alone wouldnt work, because the dimension would be hardcoded in css and may distort some images )

@jeroendesloovere jeroendesloovere changed the title from Extra options image insert with editor ( with meda library ) to CKEditor: Extra image options when inserting MediaItem from MediaLibrary in editor Jan 26, 2018

@ohvitorino

This comment has been minimized.

Contributor

ohvitorino commented Jan 29, 2018

I'm not sure I agree with the distorted images comment. If in css you define only the width or the height (not both) you won't get any distortion or I'm I wrong?

@bramds

This comment has been minimized.

Contributor

bramds commented Jan 30, 2018

It is true that this will work with only height or width. Downside of only using the css-classes is also that you re limited to the classes you have made with width and height. End-user is not free to deterime his own dimensions.

@ohvitorino

This comment has been minimized.

Contributor

ohvitorino commented Jan 30, 2018

@bramds and in that case aren't we allowing the user to break the design in case he uploads an image that is too big, too small, too wide or too high?

@bramds

This comment has been minimized.

Contributor

bramds commented Jan 30, 2018

@ohvitorino maybe, but it gives more flexibility to create matching lay-outs too. With a bit of guidance you could make your end-users aware of how to use it? However I do think we agree, there needs to be some way to controle the dimensions of the image.

@carakas

This comment has been minimized.

Member

carakas commented Jan 30, 2018

@bramds maybe only allow the width to be set and keep the height on auto?

@carakas

This comment has been minimized.

Member

carakas commented Jan 30, 2018

you can do the cropping in the media library

@jeroendesloovere

This comment has been minimized.

Member

jeroendesloovere commented Jan 30, 2018

How it now works

  1. You upload an image in the editor (image gets uploaded to media library).
  2. The website uses the "source"... Too big, heavy image, ...

How it should work in the future

  1. Uploading is the same...
  2. We use variables [MediaLibraryItem:1], then the frontend can use another image filter set (LiipImagineBundle) instead of "source", a smaller one, f.e.: maximum 1000px x 1000px dimensions... Which we could overwrite [MediaLibraryItem:1:custom_thumbnail_filter_set].

Conclusion
I would not spend time trying to add "width/height" to the image, but rather integrate "ckeditor variables"... like [MediaLibraryItem:x], [PageUrl:10], [BlogArticleDetailUrl:10], ...

@carakas

This comment has been minimized.

Member

carakas commented Jan 30, 2018

I disagree @jeroendesloovere since that would require the dev to add every possible filter that the customer want, not scalable enough

@StijnVrolijk

This comment has been minimized.

Contributor

StijnVrolijk commented Jan 30, 2018

imagine has dynamic size filters, for example |imagine_filter('my_thumbnail_filter', { thumbnail: { size: [64, 64] }}).

I'll try and see if I can get CKEditor to make this renderable

@jeroendesloovere

This comment has been minimized.

Member

jeroendesloovere commented Jan 30, 2018

Like @StijnVrolijk says, if we program a variable that behaves like [MediaLibraryItem:10:500x100], the 500x100 can be used dynamically.
So, if that is programmed, we can look to create a "popup" with width/height text fields.

@StijnVrolijk

This comment has been minimized.

Contributor

StijnVrolijk commented Jan 30, 2018

@jeroendesloovere I tested that. Problem with that is everything in CKEditor gets parsed as a HTML file. If you put {{ asset('myimage.jpg')|imagine_filter('my_thumbnail_filter', { thumbnail: { size: [64, 64] }}) }} in CKEditor it will not get parsed.

If you somehow resize the image on the fly the URL you receive is a cached URL so if that expires you're out of luck again.

I've discussed some possible solutions with @carakas and @ohvitorino but we've come up empty. Some possible solutions were:

  • Somehow put the contents of an editor in a template file and include that template file in the original template (waaayyy too complex)
  • Serve the image on the fly so something like /imagesize?image=myimage&width=200&height=200 but that causes some serious security issues with DDoS'ing
  • Just use CSS and get on with our lives (currently the most plausible)
@bramds

This comment has been minimized.

Contributor

bramds commented Feb 1, 2018

Just to keep this updated: couldnt we just implement the css class + height and width? At the end it is the end-user who might need it, an extra feature is always good to have and with some plain css we can prevent it from breaking the layout...

@ohvitorino

This comment has been minimized.

Contributor

ohvitorino commented Feb 1, 2018

Well, it's not always a case that an extra feature is a good thing 🙂 But in this case it seems to me that maybe the user should have the possibility to control the image's size when inserting it the editor, either by using css or the dimensions. I'll make a PR when I have the time, and then we'll see if it gets merged.

@carakas carakas added this to the 5.3.0 milestone Feb 13, 2018

@jeroendesloovere

This comment has been minimized.

Member

jeroendesloovere commented Feb 19, 2018

@StijnVrolijk, you mention {{ asset('myimage.jpg')|imagine_filter('my_thumbnail_filter', { thumbnail: { size: [64, 64] }}) }}, but isn't that something that we need in the frontend.

In the database (using backend) this can be: [MediaItem:1:64x64].
In the frontend we can convert this code to a more advanced line of code... like you mentioned.

Not?

@StijnVrolijk

This comment has been minimized.

Contributor

StijnVrolijk commented Feb 19, 2018

@jeroendesloovere unfortunately not, parsing text in your template is apparently the last step in the twig process. So unless you specifically put {{ asset('myimage.jpg')|imagine_filter('my_thumbnail_filter', { thumbnail: { size: [64, 64] }}) }} in your template it will just get parsed as text.

Try the following:

$this->template->assign('foo', '{{ asset('myimage.jpg')|imagine_filter('my_thumbnail_filter', { thumbnail: { size: [64, 64] }}) }}');
{{ foo|raw }}

It won't work

@StijnVrolijk

This comment has been minimized.

Contributor

StijnVrolijk commented Feb 19, 2018

Maybe, we could do something like this:

MediaItems.html.twig

{% for mediaItem in mediaItems %}
    {% set processedImages = processedImages|merge(
        {
            mediaItem.uniqueId: asset(mediaItem.image)|imagine_filter(mediaItem.filter, { thumbnail: { size: [mediaItem.width, mediaItem.height] }})
        }
    ) %}
{% endfor %}

myTemplate.html.twig

{% set myText = "This is my text, this is an image: [MediaItemWithUniqueId]" %}

{% for image in myTemplateImages %}
  {{ myText|replace('someRegexToCaptureTheMediaItemString', attribute(_context, 'processedImages[' ~ theUniqueIdSomehow ~ ']') }}
{% endfor %}

{{ myText|raw }}

Something like that, it's purely theoretical and still has some things I'm not sure how to do... It's just a thought, maybe it can work

@jeroendesloovere

This comment has been minimized.

Member

jeroendesloovere commented Feb 19, 2018

What about the other way around

{{ convert_media_items_to_html_images(myText) }}

Then the PHP method can fetch [MediaItem:< ID >:<width/height>] structure and output anything we like in a HTML format <img src="xxx" />...

=> We just need HTML that works. Which means that we don't need to convert to {{ asset(...)|imagine_... }}

@jeroendesloovere

This comment has been minimized.

Member

jeroendesloovere commented Feb 19, 2018

In the future we could also introduce other shortcuts like f.e.: [Page:10:'url'].

{{ convert_fork_cms_shorttags(myText) }}

Or f.e.: forkshortcodes(myText)...

@jeroendesloovere

This comment has been minimized.

Member

jeroendesloovere commented Feb 19, 2018

I'm sure this will work, since the "Media widgets" work the same way.

{# Slider widget #}
{{ media_library_widget('Slider', blogArticle.mediaGroup.id) }}

Just outputs HTML

@StijnVrolijk

This comment has been minimized.

Contributor

StijnVrolijk commented Feb 19, 2018

=> We don't need to convert to liip imagine in the frontend template. We just need HTML that works.

How are you going to generate the custom sized image?

@jeroendesloovere

This comment has been minimized.

Member

jeroendesloovere commented Feb 19, 2018

https://github.com/liip/LiipImagineBundle#runtime-options

Using PHP

<?php
$runtimeConfig = array(
    "thumbnail" => array(
        "size" => array(50, 50)
    )
);
?>

<img src="<?php $this['imagine']->filter('/relative/path/to/image.jpg', 'my_thumb', $runtimeConfig) ?>" />
@jeroendesloovere

This comment has been minimized.

Member

jeroendesloovere commented Feb 19, 2018

@StijnVrolijk, ok, problem solved?!

@StijnVrolijk

This comment has been minimized.

Contributor

StijnVrolijk commented Feb 19, 2018

hm, perhaps. could you try to implement it?

@jeroendesloovere

This comment has been minimized.

Member

jeroendesloovere commented Feb 19, 2018

@StijnVrolijk We first need to find a solution in the backend for: "How are we going to show an image for [MediaItem:1]"? Any suggestions?

================
Does anybody actually uses custom image sizes in frontend?
I mean, are we not better of with just one "frontend_filter" setting (like we have one for backend: media_library_backend_thumbnail), which scales big images to a maximum of f.e. a width of 1800 pixels.

@carakas carakas modified the milestones: 5.3.1, 5.4.0 May 28, 2018

@carakas carakas modified the milestones: 5.4.0, 5.5.0 Aug 6, 2018

@nchamsi

This comment has been minimized.

nchamsi commented Aug 30, 2018

we can imagine somthing to select the image position
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment