Postponed on #3159050: Allow deprecating theme suggestions
Problem/Motivation
Currently, core has 4 theme hook suggestions for the "field" theme hook:
field--[field-type].tpl.php
field--[field-name].tpl.php
field--[bundle].tpl.php
field--[field-name]--[bundle].tpl.php
Note: that the field name is prefixed with the name of the module providing the field, which usally means “field-” is prefixed to the field name. Also, note that the "bundle" is a sub-type of an entity, e.g. “content types” are bundles since an "article" is a bundle of the node entity.
The theme hook suggestions are supposed to be in order of specificity. With least specific suggestions appearing first in the list. Suggestions at the end of the list override suggestions earlier in the list.
Unfortunately, the field--[field-name].tpl.php
and field--[bundle].tpl.php
suggestions are in the wrong order. A specific field should take precedence over "any field attached to this bundle". A fix for this should be backported to D7.
We also have a namespacing issue. If the same name is used for the bundle name or the field name or the field type, it can cause unintended issues. For example, if a user creates an "image" content type there will be a namespace collision with any fields that are the "image" field type; field--[field-type]
and field--[bundle]
would both be field--image
. If a themer wanted to use a theme hook suggestion for all image-type fields (perfectly reasonable!), they would also accidentally trigger the bundle-based suggestion on all fields attached to the image content type (perfectly horrible!)
Proposed resolution
Fix the order of specificity for theme hook suggestions, namely:
field--[field-type].tpl.php
field--[bundle].tpl.php
field--[field-name].tpl.php
field--[field-name]--[bundle].tpl.php
Come up with a way to properly namespace theme hook suggestions so that bundle-based suggestions don't conflict with type-based suggestions. Note that field names are currently prefixed with the name of the module implementing the field, which almost universally means a field name is prefixed with "field-".
Remaining tasks
Reach consensus that this is the correct order of specificity.Done.- Reach consensus on a namespacing fix.
- Write a Drupal 8 patch.
- Backporting the namespacing fix would break D7 themes in the wild, so its not backportable. But, the order of specificity fix should be backported to D7.
Workaround
See here for a workaround that provides a solution for adding theme suggestions for fields based on display mode: https://drupal.stackexchange.com/questions/201210/field-template-for-dis...
Comment | File | Size | Author |
---|---|---|---|
#15 | 1367354-15-field-theme-hook-suggestion.patch | 1.53 KB | JohnAlbin |
Comments
Comment #0.0
RobW CreditAttribution: RobW commentedEdited for clarity.
Comment #1
RobW CreditAttribution: RobW commentedSome additional notes and usage example:
On a second look what I said before -- "If very specific code is needed for both bundle fields and specific fields, the same amount of corrective field--FIELD-NAME--BUNDLE.tpl's will be required no matter the override order" -- is true in many cases, in others FIELD-NAME taking priority over BUNDLE could result in easier to maintain, DRYer code.
Given the structure:
...let's say "field a" is a user reference that should display an image of the user along with their name, so some custom code is written in
field--field-a.tpl.php
. Let's also say that for whatever reason we need wrapper divs on all fields in node type two. With the current cascade, creating a general-casefield--two.tpl.php
, which containsfield.tpl.php
code with the addition of a wrapper div will absolutely requirefield--field-a--two.tpl.php
to re-establish the custom picture and name code.If we re-order the cascade as suggested here, a themer could decide if the custom field (field a) needs the wrapper div, or if maybe it already has markup that could function similarly, or if they should include a wrapper across the board in
field--field-a.tpl.php
, ensuring that there is only one theme file to edit for that particular field.The point is a general tpl referring to multiple tpls of different types in a bundle/node-type should not override a specific tpl referring to the smallest unit displayable, just that field. If the cascade were switched, options would exists for less repetitive, easier to manage code where currently there are none.
Comment #2
davidwatson CreditAttribution: davidwatson commentedAgree with the general concept here. The specificity outlined in #0 (asserting that bundles are less specific than fields) seems far more intuitive. Any other thoughts?
Comment #3
JohnAlbinHere's a patch for D7. Rolling one for D8 now.
Comment #4
JohnAlbinHere's the patch for D8.
Comment #5
RobW CreditAttribution: RobW commentedWooo! Thanks for giving this some attention.
At this point existing modules and themes are relying on the current cascade, so I don't think it's a good idea to change this in D7. As a default in D8 it's great. Thoughts?
Comment #6
RobW CreditAttribution: RobW commentedMinor change that's working fine for me, so RTBC.
Comment #7
catchIf this isn't backportable, do we want to look at namespacing these - so
field-bundle-$bundle.tpl.php
to avoid conflicts between field and bundle names?Comment #8
RobW CreditAttribution: RobW commentedCatch, are you suggesting that for D7 or D8?
Namespacing the template suggestions in field.module will fix conflicts where a field and bundle have the same name (like in field collection: #1137024-24: Field--your-field-collection.tpl.php cascades and overrides all collected field's tpls), but won't help with other module's template suggestions (like when entity view mode or DS adds a view mode template suggestion).
I was thinking about fixing this in field collection by namespacing the bundle itself; the field's machine name would stay
{field-name}
, but the bundle name would becomebundle-{field-name}
. We could open an issue to fix all conflicts across Drupal by namespacing the bundle and view mode machine names, or ask all modules and theme developers to follow a namespaced template pattern (field--{field name}--bundle-{bundle-name}--view-mode-{view mode name}.tpl
).[Edit] Replaced my pre-caffeinated thoughts with something more coherent.
Comment #9
JohnAlbinSo I looked at the theme hook suggestions created for Drupal's default image field:
field__image appears to be the name of the field. BUT it's actually the field type. Prefixing it so it's "field__type_image" makes a lot of sense to avoid this confusion.
field__article is the bundle the field is attached to. We should prefix it to avoid namespace conflicts. But using "field__bundle_article" exposes the confusing "bundle" word to themers. Not good. However, we can use the entity_type to prefix the bundle name so it makes better sense. e.g. "field__node_article". (Field collection's bundle suggestions would then look like "field__field_collection_field_name", I believe.)
Hmm…Actually, I can't think of an actual use-case to have a bundle-specific theme hook suggestion. “I want to override the markup for all fields in the exact same way on the article content type.” Seriously. Why? How about we remove that entire suggestion and if a themer wants such an obscure edge case, they can just add one in the usual way.
So this patch would make the example above be:
Comment #10
RobW CreditAttribution: RobW commentedWriting a full treatment of this issue has been on my to do list for months but I keep putting it off. Having bundle specific field templates becomes a little more reasonable when you are fielding non-node entities, e.g., simplifying HTML structure or adding a class on fields in a file entity or field collection that are themselves going to be wrapped in normal field markup at the node level.
Namespace collisions are even more likey if view modes or displays (or any additional user named variable) get added to the template suggestions. I can imagine someone (who should probably plan a little better) having a slideshow view mode, a slideshow node type, and a slideshow file type. We should probably rename this issue to prevent template name variable namespace collisions (please someone reword that so it's somewhat readable by humans) or open a new issue to cover it.
And I think prefixing is the best fix, even if it has the potential to create ridiculously long file names. I suppose we could introduce a shorthand like f-fieldname--t-entity-type--v-viewmode.tpl.
Comment #12
davidwatson CreditAttribution: davidwatson commented@10 - It sounds like there are in fact two issues - one for the specificity (this issue) and another to handle namespacing. I wonder if conflating the two will cause more problems down the road, but I do see the value in tackling the latter, even if we don't change the title of this issue.
Comment #13
JohnAlbinYes, we are conflating the two issues. But I think its easy to address them both in this issue. And Catch asked about it. And my general rule is “KEEP THE DRUPAL 8 CORE COMMITTER HAPPY.” :-D Also, while it would technically be faster to separate the two, the bug in Drupal 7 is not urgent and the bug fix patch wouldn't even fix the main issue people are experiencing with the Field Collections module (which is how I got here.)
The fix for the specificity is straight-forward. That part of this issue is totally back-portable. In fact, I'll upload the D7 patch for this right now so people can see it.
The namespacing issue requires that we change the names of the theme hook suggestions. That is totally not backportable as it would break the theme hook suggestions that people are using now.
“more reasonable” isn't a specific use-case. :-D Maybe I need to ask a more specific question: What specific use case can you describe where you want the exact same wrapping markup around each field on a file entity or on a field collection?
I've worked with the field collection module A LOT and I love it, but I would never want the same wrapping markup around each field in a collection since they each usually have their own semantics. And given how much I've used this module and how I've never needed such functionality, I would argue that its an uncommon edge case so we shouldn't be providing it by default. Let the user add that suggestion if they need it. After all, we don't provide suggestions for display modes.
Or worse: an image field type called "image" in an "image" field collection on an "image" content type with an "image" display mode on image.com.
I would prefer to limit the number of suggestions to those most likely (the 3 that I mention in comment #9).
field__type_TYPE
field__[field_name]
(Note that the field name is prefixed with the module providing the field, which is usually the "field" module.)field__[field_name]__[bundle]
(For field's attached to nodes, this means the bundle is the node type.)And I think we should just leave the rest for a themer to implement. Rather then add display modes in the form of
field__[field_name]__display_[display_mode]
to the mix too.Note: for the third theme hook suggestion, we could namespace the bundle by prefixing with the name of the entity. So
__node_article
, but I think that would give us__user_user
since user entity's don't have bundles. And I feel STRONGLY that we shouldn't use the word "bundle" anywhere in our theme hook suggestions.Done! :-)
Comment #14
davidwatson CreditAttribution: davidwatson commentedIf the consensus here is to tackle it in the same issue, I'm willing to run with it. :]
Playing the devil's advocate here (assuming I've parsed this out correctly), but consider the case of minimizing markup for performance reasons. It wouldn't be terribly hard to imagine a scenario in which two or three fields belonging to a simple collection happens to have the same lightweight wrapping markup, if you don't want or need any measure of control after that.
Agreed, and that list looks sane. Themers who need the extra control can still alter template suggestions as needed. Not sure yet how I feel about the actual conventions, have to mull it over a bit.
Comment #14.0
davidwatson CreditAttribution: davidwatson commentedrefined issue explanation.
Comment #15
JohnAlbinWhile everyone is mulling over the list I outlined in comment #13, here's a re-rolled patch that implements that list.
Note that I had to update Bartik to use the updated theme hook suggestion name for taxonomy terms; otherwise, tests broke. :-)
Comment #15.0
JohnAlbinInclude namespacing issue in summary
Comment #18
andypostPatch outdated, seems duplicated issue #2229355: Field template suggestions are colliding
Comment #21
catchComment #25
joelpittetLooks like different approaches tackled this.
Comment #26
HongPong CreditAttribution: HongPong at kor group commentedI found a solution for adding theme suggestions for fields based on display mode here: https://drupal.stackexchange.com/questions/201210/field-template-for-dis... ... adding this here since this is a prominent search result.
Comment #27
catchThe precedence order is still an issue, bundle name comes before field name in https://api.drupal.org/api/drupal/core!modules!system!system.module/func...
This might be a duplicate of #2831144: Bundle / field name / view mode theme suggestions can conflict, but re-opening for now.
Comment #29
darvanenTriaging a collection of related issues as part of the Bug Smash Initiative, I'm going to favour this issue as it is the oldest and has some attempts at work to fix.
Closing as duplicate of this issue in order of creation:
Postponing on #3159050: Allow deprecating theme suggestions as it will not be possible to deploy any work done here without a way to deprecate old template suggestions.