Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Too many theme functions and theme templates that duplicate the same resulting markup structure.
Proposed resolution
- Consolidate theme functions and templates into single implementations
- Leverage theme hook suggestions to allow targeted theme/template overrides and variable preprocessing.
Procedure
Go through the list and identify similar things and group them together.See Twig conversion/refactoring/clean-up issues- 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
- List of all theme functions and templates
"unusable" means the function is located in a file that is not registered to hook_theme; "unclear" means that it is not clear where the theme function/template is located. (see #3.1) - Related: Twig conversion/refactoring/clean-up issues
- Related: all theme functions and their status of conversion into Twig
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
- #1595614: [meta] Remove all the theme functions and templates in core that simply output a link. Replace with #type 'link' render arrays
- #1833932: Convert theme_system_compact_link() into a #type link
- #2031301: Replace theme_more_link() and replace with #type 'more_link'
- #2036195: Remove views-more.html.twig and replace with #type link render arrays
tables
- #1812684: [meta] Consolidate all table templates and add theme_hook_suggestions
- #1751194: Introduce hook_theme_suggestions[_HOOK]() and hook_theme_suggestions[_HOOK]_alter()
- #2035897: remove theme_image_style_list() and call theme('table__image_styles__list') instead
- #2035903: Remove theme_image_style_effects() and call theme('table__image_styles__effects') instead.
- #2035905: Remove theme_book_admin_table and call theme('table__book_admin') instead
- #2092343: Consolidate forum.module and remove call to _theme_table_cell() within template_preprocess_forum_topic_list()
item lists
- #1813426: [meta] Consolidate all item list templates and add theme_hook_suggestions
- #311011: Replace links.html.twig with item-list--links.html.twig
- #1222254: Remove theme_task_list() and call theme('item_list__tasks') instead.
- #1777332: Replace theme_menu_link() and menu-tree.html.twig with a single Twig template
containers
- #1819284: [meta] Consolidate all form element container templates, and add theme_hook_suggestions
- #2041825: Remove theme_radios() and call theme('container') instead
- #2041845: Remove theme_checkboxes() and call theme('container') instead
Issues Pt 2 (other theme functions that need attention)
- #2032695: Use item-list.html.twig instead of views_view_summary.html.twig
- #54898: Add a description-list.html.twig template (ex. definition list)
- #1598886: Clean up pager theme functions
- #1777324: Remove theme_user_list() from core
- #1973418: Remove tablesort-indicator.html.twig, use CSS instead
- #1842232: Consolidate use of admin-page.html.twig and system-admin-index.html.twig
- #1812724: Consolidate all form element templates and add theme_hook_suggestons
- #2076301: Remove views-mini-pager.html.twig, use a pager theme suggestion instead
- #1926344: Consolidate search-result.html.twig and search-results.html.twig
- #2231853: Rename maintenance-page template into page--maintenance + install-page into page--maintenance--install
User interface changes
None.
API changes
None.
Related Issues
- #1781372: Add an API for listing (configuration) entities will replace administrative listing functions for Configurables.
- #1833920: [META] Markup Utility Functions
- #939462: Specific preprocess functions for theme hook suggestions are not invoked
- #1751194: Introduce hook_theme_suggestions[_HOOK]() and hook_theme_suggestions[_HOOK]_alter()
- #1804488: [meta] Introduce a Theme Component Library
Comment | File | Size | Author |
---|---|---|---|
#3 | 1804614-hook-theme-info-csv.diff.txt | 2.36 KB | jwilson3 |
Comments
Comment #1
sunComment #2
chx CreditAttribution: chx commentedNote that we agreed that the Twig conversion will do this so I am not 100% this needs to be a separate issue.
Comment #3
jwilson3I'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:
Comment #3.0
jwilson3Distinguish between unclear and unusable
Comment #4
jwilson3A link to the spreadsheet would help:
https://docs.google.com/spreadsheet/ccc?hl=en&key=0AoA0oZOfKNYUdDI2SWJ0S...
Comment #5
jwilson3The 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:
Comment #6
jwilson3In analyzing the data in the spreadsheet I also stumbled across and filed this gem: #1805342: Fix inconsistent use of underscores in template file names
Comment #7
webchickWow, that's some impressive analysis work, James! Nice! :D
Comment #8
larowlanRelated #1805678: Expand theme_pager_link() so it can accept html input or specify invisible elements
Comment #9
catchI 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.
Comment #10
brunodboAforementioned spreadsheet: https://docs.google.com/a/agentic.ca/spreadsheet/ccc?key=0ArHiL7dEQTuKdG...
Comment #10.0
brunodboLink to a google spreadsheet listing all theme functions.
Comment #10.1
jwilson3Added reference to the other google doc spreadsheet.
Comment #11
jwilson3I've added spreadsheet in #10 to issue summary.
Comment #11.0
jwilson3Put the spreadsheet links in list format.
Comment #11.1
jwilson3Expand upon what "unclear" means.
Comment #12
RobW CreditAttribution: RobW commentedKeep an eye on #1804488: [meta] Introduce a Theme Component Library too, to make sure we don't duplicate/invalidate work.
Comment #13
BerdirLooks like #1484720: [Meta] Reduce the number of theme functions/templates is a duplicate meta issue, no need to have both?
Comment #14
jwilson3It 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.
Comment #14.0
jwilson3Added link to theme_filter_tips refactoring issue
Comment #14.1
jwilson3Added reference to #1813426: [meta] Consolidate all item list templates and add theme_hook_suggestions
Comment #15
cweagansThis is the newer issue. Can we move important information from here to the other issue?
Comment #16
sunI'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
Comment #16.0
sunAdded link to #54898
Comment #16.1
jwilson3Added Sun's comment about sub-patterns #939462: Specific preprocess functions for theme hook suggestions are not invoked to issue summary.
Comment #16.2
jwilson3Added description on how to create a list of themables using the patch in comment #3.
Comment #16.3
jwilson3Minor update to explain where my google doc came from.
Comment #17
sunI'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.
Comment #18
chx CreditAttribution: chx commented#2 said all I had on this, I won't repeat myself but I am unfollowing, right now.
Comment #19
jwilson3Oops, 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.
Comment #19.0
jwilson3Revamped issue summary. [sun]
Comment #20
jwilson3I've finished categorizing all the Drupal 8 theme functions and have sorted the spreadsheet so they are ordered by similarity. Couple observations:
Comment #20.0
jwilson3It's hook_theme
Comment #21
jwilson3There 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.
Comment #22
RobW CreditAttribution: RobW commentedVery 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.Comment #23
sunNot 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.)
Comment #24
catch@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.
Comment #25
Fabianx CreditAttribution: Fabianx commentedWould like to put:
http://drupal.org/project/fences
into the mix.
Comment #26
RobW CreditAttribution: RobW commentedLooks 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.)
Comment #26.0
RobW CreditAttribution: RobW commentedAdded various issues to list.
Comment #26.1
jenlamptonadd other Twig google doc
Comment #26.2
jenlamptonupdate
Comment #26.3
jenlamptonremove conversion to l()
Comment #26.4
jenlamptonadded the one for form elements
Comment #26.5
jenlamptonadded the one for containers
Comment #26.6
jenlamptonadd the one for tables
Comment #26.7
jenlamptonmove l to top section
Comment #26.8
jenlamptonadded the component thingy
Comment #26.9
jenlamptonadd item list list
Comment #26.10
jenlamptonremove dupe
Comment #26.11
jenlamptonmore
Comment #26.12
jenlamptondivide
Comment #26.13
jenlamptonmove again
Comment #26.14
jenlamptonlinks first
Comment #26.15
jenlamptonlinks at top
Comment #26.16
jenlamptonmore
Comment #26.17
jenlamptonnode-add-list
Comment #26.18
jenlamptonrenderable table doesn't really belong
Comment #27
jenlamptonI 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.
Comment #28
steveoliver CreditAttribution: steveoliver commentedJen, 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?
Comment #29
catchSorry 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.
Comment #30
steveoliver CreditAttribution: steveoliver commentedI 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.
Comment #31
mbrett5062 CreditAttribution: mbrett5062 commented@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.
Comment #32
catchI 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.
Comment #32.0
catchremove issues that are linked from other meta issues
Comment #33
jenlamptonThe 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.
Comment #33.0
jenlamptonother issues
Comment #34
jenlamptontagging
Comment #34.0
jenlamptonissue summary cleanup
Comment #34.1
jenlamptonreorg
Comment #34.2
jenlamptonlists
Comment #34.3
jenlamptoncontainers
Comment #34.4
jenlamptoncleanup
Comment #34.5
star-szrAdd issue to remove views_mini_pager theme hook
Comment #34.6
star-szrUpdate nid for views-mini-pager consolidation
Comment #34.7
joelpittetUpdated issue summary.
Comment #34.8
joelpittetUpdated issue summary.
Comment #35
star-szrThank you @mitokens for adding all the remaining child issues as true child issues at DrupalCon LA :)
Comment #36
star-szrAlso I think anything here would be RC deadline.
Comment #37
Antti J. Salminen CreditAttribution: Antti J. Salminen as a volunteer commentedAnother issue to the summary.
Comment #38
joelpittetPossibly 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.
Comment #39
joelpittetComment #40
andypost8.1 is active
Comment #41
andypostPostponed all issues about to replace templates that are part of stable theme till 9.x
Comment #45
xjmComment #52
andypostLooks it mostly done
Comment #56
catch