Problem/Motivation
Feedback was brought up in #1898416-118: filter.module - Convert theme_ functions to Twig that some of the markup and logic in the filter.module theme functions/templates could be improved. The purpose of that issue was just to convert the existing markup and logic to Twig templates, this issue is to discuss changing it.
(The numbers do not match up with the original review as some of the points have been addressed or were not relevant to this follow-up issue.)
+++ b/core/modules/filter/templates/filter-tips.html.twig @@ -0,0 +1,52 @@ + {% if multiple %} + <div class="compose-tips"> + {% endif %} ... + {% if multiple %} + </div> + {% endif %} ... + {% if multiple %} + </div> + {% endif %}
As a themer, the conditional output of the wrapper elements (and lack thereof) always resulted in a unexpected bad surprise when testing the design as anonymous user.
I wonder whether we want to remove these 'multiple' conditionals as part of this conversion to (1) simplify the template and (2) prevent nasty surprises?
+++ b/core/modules/filter/templates/filter-tips.html.twig @@ -0,0 +1,52 @@ + {% if multiple %} + <div{{ tip.attributes }}> + <h3>{{ tip.name }}</h3> + {% endif %}
The 'multiple' condition here looks wrong to me -- the wrapping container + heading is only used for the long filter tips; i.e., the condition should be 'long' instead.
That said, it would be great if we could split these two completely different representations (short/list vs. long/page) into two separate templates. Having them in a single never really made sense to me.
Proposed resolution
Discuss the points from the review here and make updates as necessary.
Remaining tasks
Discuss
Patch
User interface changes
TBD
API changes
TBD
Comments
Comment #1
star-szrComment #2
star-szrRemoving point about the paragraph tags, they are gone from #1898416: filter.module - Convert theme_ functions to Twig now, and removing the item_list consolidation so that #1778624: rework theme_filter_tips to use the new Attributes, and call theme('item_list) while we're at it can handle it.
Comment #3
star-szrFixing code tags.