This is a follow-up issue to #1842160: Consider appropriate usage of theme() from within Twig templates We now just need to go cleanup our template files that included others, and remove the @todos.

Usage of {% include %} or theme() in templates represents some sort of recursion or sub-structure that is part of a template. Instead of using include directives or theme(), we should attempt to do the following in these instances:

* (A) Address sub-structure in the parent template to avoid both includes and theme(). A good example of this is where we'd have separate theme functions for table cells vs tables. Table cells are never independent of tables, so they should be consolidated to one template (i.e. renderable concept).
* (B) Have sub-structure be renderable, that is, an {{ item }} knows how to render itself, analogous to how render($var); was intended to operate
* (C) Pre-render markup in preprocessors. If theme() must be called, have it run in a preprocessor. @todo Is this an anti-pattern? Probably. We should avoid using theme() since it can invoke the theme layer too early, and use renderable arrays instead.

Comments

c4rl’s picture

Title: Update all files that use {% include %} to use theme() now instead. » Revise usage of {% include %}

Given the change in direction of #1842160: Consider appropriate usage of theme() from within Twig templates, I'm updating the summary to reflect that we want to either (A) address sub-structure in the parent template to avoid both includes and theme(), (B) have sub-structure be renderable, or (C) pre-render markup in preprocessors.

c4rl’s picture

Issue summary: View changes

Updated issue summary to reflect new direction

joelpittet’s picture

(B) would be my preference, but how do we make those items renderable?

c4rl’s picture

We may have to resort to (A) and (C) for now until we can address render shortcomings elsewhere -- I've already peered into pandora's box here :) #1899454: [meta] Refactor Render API

steveoliver’s picture

At the moment we're doing (C) in #1778624: rework theme_filter_tips to use the new Attributes, and call theme('item_list) while we're at it.

If you wanted to theme filter guidelines (the list of tips below a textarea when there are text filter options), the Twig workflow would be:

filter-guildelines.html.twig:
- has attributes, titles and other vars preprocessed and ready for print.
- has a {{ tips }} variable which is a preprocessed <ul> item list which can be overridden in item-list--filter-tips-guidelines.html.twig

Not ideal, but I think it works for now.

c4rl’s picture

Assigned: Unassigned » c4rl
Priority: Normal » Major

I think for the sake of getting things done, we have to stick with (C) for now, then (in core) attempt to refactor as (A), and eventually figure out (B).

So, to this end, it seems all of these files listed below need to have the {% include %} directive removed. I will attempt to make progress here ASAP, committing to front-end branch. Let me know if there are any objections.


find . -name '*twig*' | grep stark | xargs grep include | grep '%' | cut -d : -f 1 | sort | uniq

./core/themes/stark/templates/comment/comment-block.html.twig
./core/themes/stark/templates/form.inc/button.html.twig
./core/themes/stark/templates/form.inc/checkbox.html.twig
./core/themes/stark/templates/form.inc/checkboxes.html.twig
./core/themes/stark/templates/form.inc/color.html.twig
./core/themes/stark/templates/form.inc/date.html.twig
./core/themes/stark/templates/form.inc/email.html.twig
./core/themes/stark/templates/form.inc/file.html.twig
./core/themes/stark/templates/form.inc/hidden.html.twig
./core/themes/stark/templates/form.inc/image-button.html.twig
./core/themes/stark/templates/form.inc/number.html.twig
./core/themes/stark/templates/form.inc/password.html.twig
./core/themes/stark/templates/form.inc/radio.html.twig
./core/themes/stark/templates/form.inc/radios.html.twig
./core/themes/stark/templates/form.inc/range.html.twig
./core/themes/stark/templates/form.inc/search.html.twig
./core/themes/stark/templates/form.inc/submit.html.twig
./core/themes/stark/templates/form.inc/tel.html.twig
./core/themes/stark/templates/form.inc/textfield.html.twig
./core/themes/stark/templates/form.inc/url.html.twig
./core/themes/stark/templates/image/image-formatter.html.twig
./core/themes/stark/templates/image/image-style-effects.html.twig
./core/themes/stark/templates/image/image-style-preview.html.twig
./core/themes/stark/templates/image/image-style.html.twig
./core/themes/stark/templates/pager.inc/pager.html.twig
./core/themes/stark/templates/system/exposed-filters.html.twig
./core/themes/stark/templates/taxonomy/taxonomy-overview-terms.html.twig
./core/themes/stark/templates/theme.inc/item-list.html.twig
./core/themes/stark/templates/theme.inc/links.html.twig
./core/themes/stark/templates/theme.inc/ol.html.twig
./core/themes/stark/templates/theme.inc/tablesort-indicator.html.twig
./core/themes/stark/templates/theme.inc/ul.html.twig
./core/themes/stark/templates/user/user-admin-permissions.html.twig
./core/themes/stark/templates/user/user-admin-roles.html.twig

c4rl’s picture

Since #1812724: Consolidate all form element templates and add theme_hook_suggestons is fixed, I've removed the following:


core/themes/stark/templates/form.inc/button.html.twig
core/themes/stark/templates/form.inc/checkbox.html.twig
core/themes/stark/templates/form.inc/color.html.twig
core/themes/stark/templates/form.inc/email.html.twig
core/themes/stark/templates/form.inc/file.html.twig
core/themes/stark/templates/form.inc/hidden.html.twig
core/themes/stark/templates/form.inc/image-button.html.twig
core/themes/stark/templates/form.inc/number.html.twig
core/themes/stark/templates/form.inc/password.html.twig
core/themes/stark/templates/form.inc/radio.html.twig
core/themes/stark/templates/form.inc/range.html.twig
core/themes/stark/templates/form.inc/search.html.twig
core/themes/stark/templates/form.inc/submit.html.twig
core/themes/stark/templates/form.inc/tel.html.twig
core/themes/stark/templates/form.inc/textfield.html.twig
core/themes/stark/templates/form.inc/url.html.twig

c4rl’s picture

I've made the following identical to core/themes/stark/templates/form.inc/container.html.twig until #1819284: [meta] Consolidate all form element container templates, and add theme_hook_suggestions is resolved.


core/themes/stark/templates/form.inc/checkboxes.html.twig
core/themes/stark/templates/form.inc/date.html.twig
core/themes/stark/templates/form.inc/radios.html.twig

c4rl’s picture

Status: Active » Fixed

Marking this as fixed now. All calls to include are now removed.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

formatting

Project: » Lost & found issues

This issue’s project has disappeared. Most likely, it was a sandbox project, which can be deleted by its maintainer. See the Lost & found issues project page for more details. (The missing project ID was 1750250)