[Layout] Too many "forced" content for inner `.page-body`

[cavasinf]

Hey, We’re currently a bit stuck with how the layout-xxx.html.twig file define the base 'structure' of the page.

Objective

The goal is to create a dashboard page with multiple rows and columns of with the same height.

Current state

Currently, the base template forces this HTML structure:

<div class="row row-cards">          
   <section id="" class="content">
       [...]
   </section>
</div>

https://github.com/kevinpapst/TablerBundle/blob/39b5e2946c0d4fea5ad05be9b1cdad686d7035b3/templates/layout-vertical.html.twig#L120-L128

Render

Current render

Here's what we have currently:

Desired render:

To achieve this, we need to apply the .row-deck class directly after the top-level .container class.
But we want the .row-deck in the top div + remove the section part.

The whole HTML wanted:

Question/Solution

1. Is there specific reason for having the section block + default row row-cards classes?
2. It may be a relic of admin-lte bundle?

It might be more flexible if this part of the structure was left for the developers to define, rather than enforced by the top-level template.

References with differents classes just after the .container div: https://preview.tabler.io/ https://preview.tabler.io/activity.html https://preview.tabler.io/photogrid.html

[kevinpapst]

This was in your initial commit already: https://github.com/kevinpapst/TablerBundle/commit/2584dc1e58085067071c57d53bc4934f6f5c016a#diff-ad9fb94d48b0a156f12a7c3972ef9db886e2eb46f6626bafb146cd53d7d77c69R119

Who knows which feature accidentally work due to this class being existent right now. Changing would be quite a BC break.

Anyway, a dashboard with same height widgets works perfectly fine for me with that HTML:

You can simply add a second div with the wanted CSS classes and everything works fine.

[cavasinf]

This was in your initial commit already

Yes, I was looking for when/why we do that, it was there from the beginning: https://github.com/kevinpapst/TablerBundle/commit/657ea7142a71a4c8efa311e253a71c6cb15559e2#diff-77f98db4bbf74ae14cad716cd52d662106d4ea5a78039716a76307c9745b7be9R112-R114

Who knows which feature accidentally work due to this class being existent right now. Changing would be quite a BC break.

Yes, it could, testing in the table preview, there is no big impact:

1. The .content class does nothing
2. .row .row-cards add the same margin as .page-body.

• The only impact is if you have a js/css selector targeting this.
• Starting a page directly with some .col without .row (because we add it in the base template)

[kevinpapst]

Do you want to sent a PR with the changes? I will try then apply your changes locally and test for a while to see if anything weird happens.

[cavasinf]

Do you want to sent a PR with the changes?

Yes I can Do we keep the section.content element?

[kevinpapst]

I though you only replace <div class="row row-cards"> with <div class="row">.

This PR change will definitely blow up my app. I use both blocks / the section as selector in JS and CSS:

{% block page_content_class %}{{ parent() }} {% block page_class %}{% endblock %}{% endblock %}

And I am almost certain I will find more references if I search more detailed:

[cavasinf]

I though you only replace <div class="row row-cards"> with <div class="row">.

No 😢

This PR change will definitely blow up my app

We have a project where another developer also uses/overrides these blocks. The quick fix was to update the base.html.twig with:

{% block page_content_before %}
   <div class="row row-cards">
      <section id="{% block page_content_id %}{% endblock %}" class="{% block page_content_class %}content{% endblock %}">
{% endblock %}

{% block page_content_after %}
      </section>
   </div>
{% endblock %}

It is a temporary patch to avoid the BC break but will/should not be left that way.

---

The end goal it that every pages that uses {% block page_content %}{% endblock %} should look like this now:

✔	❌
{% block page_content %} <div class="row row-cards"> <div class="col-sm-6 col-lg-3"></div> <div class="col-sm-6 col-lg-3"></div> <div class="col-sm-6 col-lg-3"></div> <div class="col-sm-6 col-lg-3"></div> </div> {% endblock %}	{% block page_content %} <div class="col-sm-6 col-lg-3"></div> <div class="col-sm-6 col-lg-3"></div> <div class="col-sm-6 col-lg-3"></div> <div class="col-sm-6 col-lg-3"></div> {% endblock %}

OR

✔	❌
{% block page_content %} <div class="row justify-content-center"> <div class="col-8"> <div class="card"></div> </div> </div> {% endblock %}	{% block page_content %} <div class="justify-content-center"> <div class="col-8"> <div class="card"></div> </div> </div> {% endblock %}

[kevinpapst]

Do you want to start working on the next major version? This could include the open "BC Break" PRs and this change here as well.

[cavasinf]

We can, do we also include issue that will lead to BC break too? Like #164 or #120

[kevinpapst]

Yes, all planned BC breaks