Problem/Motivation

Too many theme functions and theme templates that duplicate the same resulting markup structure.

Proposed resolution

  1. Consolidate theme functions and templates into single implementations
  2. Leverage theme hook suggestions to allow targeted theme/template overrides and variable preprocessing.

Procedure

  1. Go through the list and identify similar things and group them together. See Twig conversion/refactoring/clean-up issues
  2. Find existing issues or create a new to consolidate groups of similar theme functions, using theme suggestions and preprocess to handle minor differences (or just skipping the differences if they're superfluous).

Details

Remaining tasks

Blockers

Not actually blockers for consolidating theme functions and templates, but without resolving these issues, modules and themes will not be able to easily target/override a specialized theme hook, which means a critical regression compared to D7:

Issues Pt 1 (theme functions that are grouped together)

links

tables

item lists

containers

Issues Pt 2 (other theme functions that need attention)

User interface changes

None.

API changes

None.

CommentFileSizeAuthor
#3 1804614-hook-theme-info-csv.diff.txt2.36 KBjwilson3
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

chx’s picture

Note that we agreed that the Twig conversion will do this so I am not 100% this needs to be a separate issue.

jwilson3’s picture

I've created a spreadsheet listing all of the D8 theme functions that I think will be useful to anyone working on this, or the Twig stuff.

Its based on a small script (attached) that converts all the information from module_invoke_all('theme') into a comma-separated list.

In doing this, I've found a couple interesting observations:

  1. The script used to generate the spreadsheet uses function_exists after a full core bootstrap via hook_init() on the Modules List page, to test if the theme function is available. There are a number of functions do not inform hook_theme which file needs to be loaded and are *not* present (via function_exists) at time of exectution of full drupal bootstrap. Thus, these are marked "unusable", if a template/function was found. However, there are other cases where neither a file, function, or template exists (marked "unclear" in the spreadsheet). This might be a red-herring; obviously, core works, and the proper files are loaded when they are called... the cause could be that these functions are only being called from files in special parts of the site where the file is guaranteed to be loaded.
  2. The "render element" from hook_theme() seems really inconsistently used. This column, in theory, could be used to identify similar functions, however in a lot of cases, there are theme functions that are simple wrappers for theme_table() but instead of labeling the render element a table, it's just labeled the more generic and less informative element. It is also possible I've missed the point entirely for this, because, the documentation about "render element" is totally unclear about what this is used for.
jwilson3’s picture

Issue summary: View changes

Distinguish between unclear and unusable

jwilson3’s picture

jwilson3’s picture

The functions that don't provide a "file" and are also not loaded after full Drupal bootstrap, creating conditions where trying to use the theme function without knowing which file its in and loading that file by hand, or through some other mechanism:

theme_comment_preview
theme_image_style_list
theme_image_style_preview
theme_image_resize_summary
theme_image_scale_summary
theme_image_crop_summary
theme_image_rotate_summary
theme_node_admin_overview
theme_help
theme_update_page
theme_install_page
theme_task_list
theme_authorize_message
theme_authorize_report
theme_update_report
theme_update_version
theme_update_status_label
jwilson3’s picture

In analyzing the data in the spreadsheet I also stumbled across and filed this gem: #1805342: Fix inconsistent use of underscores in template file names

webchick’s picture

Wow, that's some impressive analysis work, James! Nice! :D

larowlan’s picture

catch’s picture

I saw a spreadsheet somewhere which had a list of Twig conversions - this was in irc and I didn't save the link. If someone knows where that is (probably Jen Lampton or FabianX do), it'd be good to cross-reference against that one.

brunodbo’s picture

brunodbo’s picture

Issue summary: View changes

Link to a google spreadsheet listing all theme functions.

jwilson3’s picture

Issue summary: View changes

Added reference to the other google doc spreadsheet.

jwilson3’s picture

I've added spreadsheet in #10 to issue summary.

jwilson3’s picture

Issue summary: View changes

Put the spreadsheet links in list format.

jwilson3’s picture

Issue summary: View changes

Expand upon what "unclear" means.

RobW’s picture

Keep an eye on #1804488: [meta] Introduce a Theme Component Library too, to make sure we don't duplicate/invalidate work.

Berdir’s picture

Looks like #1484720: [Meta] Reduce the number of theme functions/templates is a duplicate meta issue, no need to have both?

jwilson3’s picture

Issue tags: +d8dtx, +Twig, +theme system cleanup

It does look like a dupe, but I'll let others decide which to close. Probably would be good to get them all under the same tags too.

jwilson3’s picture

Issue summary: View changes

Added link to theme_filter_tips refactoring issue

jwilson3’s picture

cweagans’s picture

Status: Active » Closed (duplicate)

This is the newer issue. Can we move important information from here to the other issue?

sun’s picture

Status: Closed (duplicate) » Active

I'm re-opening this issue, because the outlined summary is much more actionable than the discussion in #1804488: [meta] Introduce a Theme Component Library.

Marked #1484720: [Meta] Reduce the number of theme functions/templates as absolute duplicate, since it essentially asked for the identical actions to happen.

This issue is about doing something. You may follow #1804488: [meta] Introduce a Theme Component Library for discussion.

Speaking of, while most of the consolidation may happen already, doing so will leave us in a regression state, since "subpatternized" theme functions à la '[theme_]links__operations' cannot really be overridden by a theme yet, nor properly preprocessed by modules. This has to be fixed in:
#939462: Specific preprocess functions for theme hook suggestions are not invoked

sun’s picture

Issue summary: View changes

Added link to #54898

jwilson3’s picture

Issue summary: View changes

Added Sun's comment about sub-patterns #939462: Specific preprocess functions for theme hook suggestions are not invoked to issue summary.

jwilson3’s picture

Issue summary: View changes

Added description on how to create a list of themables using the patch in comment #3.

jwilson3’s picture

Issue summary: View changes

Minor update to explain where my google doc came from.

sun’s picture

I've revamped the issue summary. Unfortunately, there was a conflict while editing, but I think/hope I've re-incorporated the last edit(s) correctly.

chx’s picture

#2 said all I had on this, I won't repeat myself but I am unfollowing, right now.

jwilson3’s picture

Oops, sorry, guess we were editing it at the same time without knowing... I was just adding your feedback in #16, but thanks for the revamp.

I'd also like to point out, I've added a column to the spreadsheet called "What is it really?" which we could use for identifying what kind of html is produced. So far, I'm using things like:

* list
* list of links (ie a menu)
* list item (link)
* text wrapper (span)
* text wrapper (em)
* block-level wrapper
* block-level wrapper w/ title

chx's comment notwithstanding, AFAICT, Twig is only still in its infancy phase of one-to-one conversion of theme functions and tpl.php templates to html.twig templates and with preprocess functions. I think there is high-level work that can be done in parallel to think about how we can consolidate theme functions. I dont see this standing *outside* of twig, but working directly with it.

jwilson3’s picture

Issue summary: View changes

Revamped issue summary. [sun]

jwilson3’s picture

Issue tags: +d8dtx, +Twig, +theme system cleanup

I've finished categorizing all the Drupal 8 theme functions and have sorted the spreadsheet so they are ordered by similarity. Couple observations:

  • Not sure why my spreadsheet report 186 themables, while the spreadsheet being used by the twig conversion only has 135.
  • There is an extraordinary amount of themables that amount to nothing more than an html block-level wrapper element around a title element and a body element, and in a few casess some secondary content (eg, description text, or meta data, or footer section etc.). Would having one single themable function theme_container, that accepts options similar to item_list make sense? It would default to a div for the block wrapper, but could pass in any block level element in the variables. The title and wrapper element would also have sensible defaults but be customizable as well. We would then also have a couple preset alternate configurations using theme suggestions, for example: theme_container__aside (change the title wrapper to an h3), theme_container__article (change the wrapper to 'article', change the title wrapper to an h2). This would reduce the twig conversions from lots of very similar templates to very simple preprocess functions
  • there are two feed templates in the aggregator module that could really be generalized into something like theme_feed or theme_feed__rss theme_feed__opml so that contrib / services / etc could take advantage of them. We have theme_html, why no drop in templates for other standard output formats?
  • More to come...
jwilson3’s picture

Issue summary: View changes

It's hook_theme

jwilson3’s picture

There are 22 themables in core that each implement what amounts to a form table. There are some minor variations, eg, some have rows that are draggable, some have a column of operations, some Have a pager. It seems like there could be a way to pull out some of the common code into a new themable, and provide options for the variations above to be able to use one single theme template, and only implement preprocess functions.

RobW’s picture

Very supportive of a generic wrapper template. I suggested something similar in http://drupal.org/node/1804488#comment-6586708, a "wrapper--{the full file name of the template you want to wrap}.tpl, using either #theme_wrappers or a new function." Something standardized across all theme functions so that you don't need to fork a template just to add <div class="wrap"></div> for one context.

sun’s picture

Not sure on the block-level wrapper consolidation idea. Replacing some of them with article (e.g., article__comment) might make sense, because the semantics for articles are shared.

However, a generic "container" for all of these would be too much consolidation, IMHO. I don't think the goal is to reduce Drupal to 5 theme functions, right? ;)

What definitely makes more sense is to attempt to consolidate the text wrappers. We have a couple of issues for those already (see OP). For those that are wrapping the markup with a DIV, the existing #type/#theme container might be sufficient already. (Apparently, theme_container was misclassified as form wrapper -- but it's a generic wrapper.)

catch’s picture

@chx it's my understanding that the Twig sandbox is focusing on a straight conversion from .tpl.php to .twig. Only once that's done will the focus move to consolidating templates (someone can correct me if I'm wrong). Given that, it makes sense for those who care about consolidating templates to try to figure out some of that now where it's possible, since it's not duplicating any work happening elsewhere.

Fabianx’s picture

Would like to put:

http://drupal.org/project/fences

into the mix.

RobW’s picture

Looks like I didn't read closely enough when I posted my first comment. My thoughts on the wrappers was more to remove all nonsemantic wrapping elements (anything outside the primary container, e.g., elements with a "*-wrap" class) in core templates and then use a standardized separate template file to add them back in, whereas jwilson is calling the containing block level element the wrapper. I'm actually with sun on this -- sorry for the static.

Re: Fences, there was a large discussion moderated by Jacine earlier this year about adding markup control in the UI. TL;DR, most were against it as a core option, mostly because it adds another place markup can be set and would introduce more complexity/ pointy-clicky maintenance time, which front end devs hate. (FYI, I use fences every once in a while and love it, but I love it in contrib.)

RobW’s picture

Issue summary: View changes

Added various issues to list.

jenlampton’s picture

Issue summary: View changes

add other Twig google doc

jenlampton’s picture

Issue summary: View changes

update

jenlampton’s picture

Issue summary: View changes

remove conversion to l()

jenlampton’s picture

Issue summary: View changes

added the one for form elements

jenlampton’s picture

Issue summary: View changes

added the one for containers

jenlampton’s picture

Issue summary: View changes

add the one for tables

jenlampton’s picture

Issue summary: View changes

move l to top section

jenlampton’s picture

Issue summary: View changes

added the component thingy

jenlampton’s picture

Issue summary: View changes

add item list list

jenlampton’s picture

Issue summary: View changes

remove dupe

jenlampton’s picture

Issue summary: View changes

more

jenlampton’s picture

Issue summary: View changes

divide

jenlampton’s picture

Issue summary: View changes

move again

jenlampton’s picture

Issue summary: View changes

links first

jenlampton’s picture

Issue summary: View changes

links at top

jenlampton’s picture

Issue summary: View changes

more

jenlampton’s picture

Issue summary: View changes

node-add-list

jenlampton’s picture

Issue summary: View changes

renderable table doesn't really belong

jenlampton’s picture

I also had a bunch of separate meta issues for each component grouping we'd identified in the process of converting to Twig, but this one is much better. :) I've added them to the summary here too.

Just to update everyone outside Twigland about our involvement in the markup cleanup:

Our process is First to convert all existing template files and theme functions to twig. This gives us a chance to look at every single piece of markup that comes out of core. While doing this we're finding a lot of places where things can be consolidated or eliminated (and creating issues against core). After we get everything moved to Twig (mostly as-is) we'll begin refactoring over in the Twig sandbox.

Second, we'll begin working to create our Theme Component Library (see #1804488: [meta] Introduce a Theme Component Library). We'll do this by consulting Jacine's blog post about what a component library *should* look like, but start by taking the pieces of markup that Drupal uses all the time and making those into our first components. Our components will be twig templates.

Third, we'll introduce these compontents back into core, probably in the system module (?), but much like our current theme functions in theme.inc.

Fourth, We'll start refactoring all the markup generated by each module, to use the component library (and perhaps renderable objects, see #1843798: [meta] Refactor Render API to be OO). In an ideal world, modules will not need to provide their own markup anymore, because they can build their output using the components we've provided. (Drupal is not a perfect world, so we will of course have some new template files provided by modules.) We'd like to use the process of recreating markup for the modules using components to test the breadth of our component library :)

Fifth, we'll create patches against each module, replacing the theme functions and tpl.php files with a Twig alternative that uses the Component Library heavily, (if not entirely).

Since we're not really sure what's going to be happening first (Consolidation, Twigification, Markup improvements, etc) we've been creating issues for every consolidation and markup improvement suggestion against core instead of internally against our sandbox. Our hopes are that by merging with HEAD often we'll avoid duplicating work, and make the fastest progress :)

For the complete Twig roadmap, see: Our Sandbox page.

steveoliver’s picture

Status: Active » Postponed

Jen, re: your #27,

Based on our most recent discussion (2012-12-20 Twig Hangout), I see things a little different relative to Twig. Specifically: we should complete conversion before consolidation. This is where I see us (between 1.a and 1.b). (I'll post this to the Twig sandbox):

CONVERSION

Note: We've currently got lots of similar templates -- that's OK -- just wait... this is only *conversion*. And it needs to be done now!

1. Convert remaining theme functions and templates in the Twig sandbox.

1.a. Everyone/Anyone: Finish conversions.

1.b. Unofficial self-selected gate-keepers (jen, myself, joel?, c4rl?): Verify conversions pass all tests.

2. Work from #1757550: [Meta] Convert core theme functions to Twig templates.

CONSOLIDATION

Note: This is where we'll be getting rid of similar theme functions and templates from core.

3. Consolidate theme functions and templates. (This issue).

COMPONENTS

Note: Do this in core? Or first in a sandbox like we've discussed? I think we need a wider audience (Core).

4. Create theme components (special theme functions and/or renderable objects).

5. Implement theme components. File and fix 'theme system cleanup' issues.

Postpone?

catch’s picture

Status: Postponed » Active

Sorry why is this postponed on conversion? Can't people be working on it in parallel? There's definitely not time to individually convert every core template to twig then go back and do this afterwards. At this point I'm concerned whether either of them are going to happen at all since there's been no activity in the core queue on either front for nearly two months now.

steveoliver’s picture

I thought postpone and only do conversion now since its seems we're way closer to that. I'm just trying to get something towards closure, and it seems that would be conversion only. Looking at form.inc, though, it'd be pretty painful to NOT consolidate now. The next patch I submit will be for consolidation in form.inc in #1812724: Consolidate all form element templates and add theme_hook_suggestons.

mbrett5062’s picture

@steveoliver & @catch, as noted in the summary, #939462: Specific preprocess functions for theme hook suggestions are not invoked is a blocker for this. Myself and @tim.plunkett got that green 1 Month ago today. Could someone please take a look at it and get it pushed through to completion. @fabianx put it on his list todo, but obviously he is busy, as I am sure you both are. If we could just get someone to look it over, then I can try and address any issues found and move it forward.

catch’s picture

I don't think #939462: Specific preprocess functions for theme hook suggestions are not invoked blocks this at all. We already use preprocess functions in this way, the more we use them the more obvious that bug becomes but fixing that issue shouldn't stop any individual patch. If absolutely necessary we can make that issue critical though given a wider use of suggestions.

catch’s picture

Issue summary: View changes

remove issues that are linked from other meta issues

jenlampton’s picture

The blocker here is this: Before we can use suggestions to consolidate similar templates, we'll need to make sure that suggestions we need will work correctly. To answer #29 this issue was postponed because there's no point in updating calls to from one function that works to another one that doesn't.

I don't think all the sub-issues here will run into that problem, so making this issue active again is probably a good idea. But those working on it these issues should understand that we do still have critical bugs in core that may impede their progress :)

However, we do have a new proposal for a solution to that problem, if you are interested see #2004872: [meta] Theme system architecture changes.

jenlampton’s picture

Issue summary: View changes

other issues

jenlampton’s picture

Issue tags: +Template consolidation

tagging

jenlampton’s picture

Issue summary: View changes

issue summary cleanup

jenlampton’s picture

Issue summary: View changes

reorg

jenlampton’s picture

Issue summary: View changes

lists

jenlampton’s picture

Issue summary: View changes

containers

jenlampton’s picture

Issue summary: View changes

cleanup

star-szr’s picture

Issue summary: View changes

Add issue to remove views_mini_pager theme hook

star-szr’s picture

Issue summary: View changes

Update nid for views-mini-pager consolidation

joelpittet’s picture

Issue summary: View changes

Updated issue summary.

joelpittet’s picture

Issue summary: View changes

Updated issue summary.

star-szr’s picture

Thank you @mitokens for adding all the remaining child issues as true child issues at DrupalCon LA :)

star-szr’s picture

Issue tags: +rc deadline

Also I think anything here would be RC deadline.

Antti J. Salminen’s picture

Issue summary: View changes

Another issue to the summary.

joelpittet’s picture

Category: Task » Plan
Status: Active » Postponed

Possibly something we can consider for 8.1.x but postponing for now. Some consolidation has been completed which is great. Thanks for all your help with this meta.

joelpittet’s picture

Version: 8.0.x-dev » 8.1.x-dev
andypost’s picture

Status: Postponed » Active

8.1 is active

andypost’s picture

Postponed all issues about to replace templates that are part of stable theme till 9.x

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

xjm’s picture

Issue tags: -rc deadline

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

andypost’s picture

Looks it mostly done

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

catch’s picture

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.