GetmeUK / ContentTools

A JS library for building WYSIWYG editors for HTML content.
http://getcontenttools.com
MIT License
3.95k stars 395 forks source link

Question: Responsive images on edit mode #472

Open Kunie-dev opened 6 years ago

Kunie-dev commented 6 years ago

My img elements have styles img.img-responsive { display: block; max-width: 100%; height: auto; } When I'm on edit mode, img elements are changed to div, and they are given background-image property. They are not displayed auto sizing... I hope that div contain origin img which set invisible (no doing none display) at least. Or is there another way? recommend please.

darksnake747 commented 6 years ago

First, to start, I want to point out that ContentTools states that the editor does not work well on mobile devices. More importantly, at least from my observations, it doesn't have native support for responsive displays. Nor does it have native support for Bootstrap, which I am assuming you are using based on that class name. That said, you can achieve this on your own; in fact, it is how I use it. I don't have mobile support for the edit mode completely perfect, but that is an issue of screen real estate and not something I care to fix.

Because of all that, when the content switches to edit mode and your img tags become divs, the Bootstrap CSS no longer applies - that is really why the responsive images aren't working. You should be able to add some CSS to the editor pages that appies the Bootstrap CSS from img.img-responsive, which is: { display: block; max-width: 100%; height: auto; } to the divs. Try this:

.ce-element--type-image.img-responsive { display: block; max-width: 100%; height: auto; }

You could be having another problem though. Does the div in edit mode have the img-responsive class on it? If not, the easiest way to handle that will be to add a supported style for it, like this:

ContentTools.StylePalette.add([
    new ContentTools.Style('Responsive', 'img-responsive', ['img']),
]);

Personally, I inject that class on my images when they get inserted. That's done like this:

dialog.addEventListener('imageuploader.save', function () {
    // ... do stuff here ...
    dialog.save(image.url, [width, height], {
        'class': 'img-responsive',
        //... other image settings ...
    });
});

Moving forward, if these things happen, just keep in mind that by wanting to use the ContentTools editor with Bootstrap and a responsive display, you will have to manually add support for it like this.

I hope this helps you. Let us know if you get the problem resolved.

cubiclesoft commented 6 years ago

First, to start, I want to point out that ContentTools states that the editor does not work well on mobile devices. More importantly, at least from my observations, it doesn't have native support for responsive displays.

Try the mobile_ready branch. It's a little bit behind master.

ghost commented 6 years ago

It would be great to officially add an option to handle images with a real tag and not with a div and a background. It's a recurring problem everytime I use content tools.

anthonyjb commented 6 years ago

Hi @Alex360hd, what problems are you running into? I may well consider changing the tag over (though it would be some time before I do (like v2 when we switch to es6 next year) unless someone else is able to implement it before then), but in the meantime if you let me know what you're running up against maybe I can help.

As you might expect we're using CT a lot and we do come up against things from time to time but so far (touch wood) we've always been able to find an approach that works (or we change CT). If you let me know what it is you're stuck on I might be able to provide an approach or if there's no work around then a short term fix.

ghost commented 6 years ago

There is problem with responsive design. A div with a background image don't behave like a responsive image.

For example, if I remember well, your div that handle the background image have inline width and height defined, which make it non responsive by nature.

In addition, your theme may have some rules applied to , rule that don't apply when you are in edit mode with div and background, so you have to add some custom rules to handle that divs to not break your theme when you click on the content tools edit button.

I understand that working with div and background image allows you to provide some options to place / crop the image, that's why, instead of replacing the widget, I suggest to add a new one.

Best regards,

anthonyjb commented 6 years ago

So we use background images I think because they support for pseudo elements and we show the dimensions of the image using these and data attributes. In hindsight it would have been better to just create additional JS to support this as it's not required for place and crop.

I get what you mean about the additional rules, we just have a boiler plate for these now that we apply at the start of the project, but yes it would be nice to remove this requirement moving forward.

You can however support responsive images with background images we do it all the time, for example:

There's different ways to do this, but here are a couple of approaches we've used (howver:

Not saying this is perfect (and you may well already be doing this) but on the off chance it helps you or others finding this thread :)

ghost commented 6 years ago

Thank you for the informations, but conceptually, I can't understand the choice to replace standard image tag with a div with background. To me, it can only bring problems.

anthonyjb commented 6 years ago

@Alex360hd I'm human, just not a great decision, as I stated in hindsight I wouldn't do it this way again.

ghost commented 6 years ago

No problem :) This plugin is still an excellent plugin and I thank you for giving it to us !

That's why I criticize some part of it, I'm french and we have a proverb that says :

Qui aime bien chatie bien

Which could be translated like that :

Who loves well chatises as well

That's what I'm doing with your plugin :)

ghost commented 6 years ago

At lease, if somebody push a PR to improve image editing ? would you be ok to merge it to officially bring a good support for images ?

anthonyjb commented 6 years ago

@Alex360hd yes absolutely.