[RFC] Minor comments about the Twig templates

[javiereguiluz]

I was looking at the source of your Twig templates and I have some comments about them. These are personal and unsolicited comments, so feel free to ignore them altogether.

---

{# existing code #}
{% if context.directories|length > 0 %}

{# proposal #}
{% if context.directories is not empty %}

The first one is easier to understand for developers, the second one is easier for designers.

---

{# existing code #}
<form action="{{ path('useraction', {'action': 'enable', 'id': user.id}) }}" method="post">

{# proposal #}
<form action="{{ path('useraction', { action: 'enable', id: user.id }) }}" method="post">

Is not required to wrap hash keys with quotes. Not doing that saves you some characters and can help to differentiate better between keys and values. Adding a white space between the opening { and the closing } increases readability too.

---

{# existing code #}
<li{{ group.is_active ? ' class="active"' : '' }} id="tabindicator-{{ group.id }}">

{# proposal #}
<li class="{{ group.is_active ? 'active' }}" id="tabindicator-{{ group.id }}">

Including the class HTML attribute as part of the Twig string messes things. Moreover, it's not a problem having an empty class attribute when the condition is not true. Lastly, it's OK to not provide the second expression of the ternary operator in cases like this. Another example:

{# existing code #}
<div class="tab-pane{{ group.is_active ? ' active' : '' }}" id="{{ group.id }}">

{# proposal #}
<div class="tab-pane {{ group.is_active ? 'active' }}" id="{{ group.id }}">

---

{# existing code #}
<option value="{{ ext }}">{{ ext|upper() }}</option>

{# proposal #}
<option value="{{ ext }}">{{ ext|upper }}</option>

It's better to not add an empty () when the filter doesn't define arguments, to not mislead users and make them think this is a function.

[GwendolenLynch]

DISCLAIMER: I am not a template person :wink:

Awesome suggestions @javiereguiluz, thank you very much :+1:

[bobdenotter]

@javiereguiluz Thanks for the feedback! These all seem like sensible changes to make.

Over time the codebase (including the twig templates) has grown, and a bunch of different people have worked on it. I think it's somebody went through them with a fine-toothed comb, and fixed the style issues you've listed! 👍

[GwendolenLynch]

Most of this should be taken care of now, and a massive refactor is underway to get our Twig templates compatible with Twig 2+.

If anyone has useful data to add, please feel most welcome but addressing SnR.