Remove imagemargin and floating in Contao 5

[bezin]

Description

Many years ago we had margin before and margin after fields for content elements (not exactly sure about that), which we removed because that was theme-related. I think we should finally remove imagemargin and floating, too, as this is theme-related as well and should be solved otherwise.

The same goes for the layout options (rows and columns) in the gallery element actually, but this might be another topic.

[fritzmg]

Why do you want to remove floating? We use that all the time. Same with images per row for gallery elements.

[bezin]

Why do you want to remove floating? We use that all the time. Same with images per row for gallery elements.

We don't use either. But I can see, how you might use these — at least floating.

Let us see, how others use these settings.

[leofeyer]

I agree that we should probably keep the floating setting.

[fritzmg]

And regarding the row setting for gallery elements - that's how we use it:

.ce_gallery {
    ul {
        display: grid;
        gap: var(--grid-gap);
    }

    @for $i from 1 through 12 {
        ul.cols_#{$i} {
            grid-template-columns: repeat(#{$i}, 1fr);
        }
    }
}

plus a mechanism to automatically set the appropriate image size from contao.image.sizes.

[m-vo]

I agree that we should probably keep the floating setting.

Yes, in the new elements I would name it layout though, and make it a property of the content element instead of the figure/image.

[contaoacademy]

Yes, please keep the floating setting.

[bennyborn]

I'm also for keeping the floating but I'd also really like to see the imagemargin gone 😁

[Toflar]

Marking up for discussion because that's actually an easy one for anybody to contribute, we just need to decide what :)

[m-vo]

Here is what I'm currently doing in #4444:

contao/core-bundle/src/Controller/ContentElement/TextController.php

Lines 44 to 50 in 704940d

	$layoutAttributes = new HtmlAttributes();
	if ($model->addImage && $position = $model->floating) {
	$layoutAttributes->addClass("image--$position");
	}
	$template->set('layout_attributes', $layoutAttributes);

contao/core-bundle/src/Resources/contao/templates/_new/content_element/text.html.twig

Lines 17 to 38 in 704940d

	{% extends "@Contao/content_element/_base.html.twig" %}
	{% block content %}
	<div{{ layout_attributes }}>
	{% block content_inner %}
	{# RTE text #}
	{% block text %}
	<div class="rte">{{ text|insert_tag|raw }}</div>
	{% endblock %}
	{# Image #}
	{% block image %}
	{% if image %}
	{% include "@Contao/component/_figure.html.twig" with {
	figure: image,
	figure_attributes: attrs(figure_attributes|default).addClass('image_container')
	} %}
	{% endif %}
	{% endblock %}
	{% endblock %}
	</div>
	{% endblock %}

[fritzmg]

Here is what I'm currently doing in #4444:

That would introduce additional wrappers which I wouldn't like 🙃. I'd prefer the class being added on the main wrapper.

[m-vo]

Well, that's debatable. 🙂 IMHO it's better this way as it semantically separates outer and inner layout.

But, having a separate set of attributes makes it easy to adjust:

{% extends "@Contao/content_element/text.html.twig" %} 

{% set attributes = attributes.mergeWith(layout_attributes) %}

{% block content %}
  {{ block('content_inner') }}
{% endblock %}

[leofeyer]

As discussed in the Contao call, we want to keep the floating field and only remove the imagemargin field.