Problem/Motivation
Currently theme suggestions for field templates are colliding - see system_theme_suggestions_field()
So field type and field name are used in one level of suggestions, for example:
field__changed
fires for "changed" field type and the same named field for the most of core's entities.
Proposed resolution
Find a proper way separate field name and its type.
Current suggestion #3 - move suggestion of field name to one level deeper.
So each field name would be prefixed with its type and suffixed by specific entity and bundle
$suggestions[] = 'field__' . $element['#field_type'];
$suggestions[] = 'field__' . $element['#field_type'] . '__' . $element['#field_name'];
$suggestions[] = 'field__' . $element['#field_type'] . '__' . $element['#field_name'] . '__' . $element['#entity_type'];
$suggestions[] = 'field__' . $element['#field_type'] . '__' . $element['#field_name'] . '__' . $element['#entity_type'] . '__' . $element['#bundle'];
Remaining tasks
discus, create a patch, commit
User interface changes
no
API changes
tbd
Original report by @andypost
Follow-up from #2226233-22: Redo changes from field.module to Twig conversion issue that were clobbered.
Do not mix field name and type on one level of template suggestions.
Faced with that in #1962846-8: Use field instance name for header of comment list, drop comment-wrapper template
That leads to field__comment
template could be used for for any field that named "comment" for example custom entity with base field named so
field "text" with a "field_name" attached to "entity_type" within "bundle"
Comment | File | Size | Author |
---|---|---|---|
#16 | field-suggestions-2229355-10264875--1.patch | 786 bytes | vprocessor |
#3 | 2229355-field-suggestions-3.patch | 1.1 KB | andypost |
2226233-22.patch | 724 bytes | andypost | |
Comments
Comment #1
andypostComment #2
markhalliwellNo. We shouldn't remove this, as stated in my comment: #1962846-16: Use field instance name for header of comment list, drop comment-wrapper template.
Comment #3
andypostThe proper template suggestion added
Comment #5
markhalliwellI'm pasting the conversation that @andypost and I had in IRC. I'm postponing this issue on the now parent issue. I'm also bumping this to major because this affects DX and FX. Theme suggestions for fields is a pretty big deal.
Comment #6
markhalliwellComment #7
markhalliwellComment #8
markhalliwellSorry for all the noise. I'm still getting used to parent/child/related issues.
Comment #9
andypostComment #10
alexpottWhy is this postponed on #939462: Specific preprocess functions for theme hook suggestions are not invoked?
Comment #11
andypostI think better to make this one active to get more people to discuss the problem space.
Comment #12
star-szrNothing here is actionable by a novice contributor yet, untagging.
Comment #13
star-szrAdding this related, older issue. Seems related to me, anyway.
Comment #14
catchWe might not be able to change this after RC/beta, so tagging.
Comment #15
vprocessor CreditAttribution: vprocessor at Skilld commentedTask>>Find a proper way separate field name and its type.
I think we can add field__name- to identify that we want to use first param as field name
then old code will work like usually but we will get new feature
Please, see patch
https://www.drupal.org/files/issues/field-suggestions-2229355-10264875.p...
Comment #16
vprocessor CreditAttribution: vprocessor commentedTask>>Find a proper way separate field name and its type.
I think we can add field__name- to identify that we want to use first param as field name
Please, see patch
https://www.drupal.org/files/issues/field-suggestions-2229355-10264875--...
Comment #17
andypostbetter to prefix "type-" as well
Comment #18
vprocessor CreditAttribution: vprocessor commentedAndy, patch with type
https://www.drupal.org/files/issues/field-suggestions-2229355-10265235.p...
Comment #19
lauriiiCould these possibly collide also?
Comment #20
alexpottComment #21
catchTrying to think through how this could be fixed if it didn't get in during RC.
Could we leave the existing suggestions in (with the collisions, deprecated), and add the new ones? That's messy but would at least mean this moves to 8.1.x instead of 9.x.
Comment #22
xjmTo have committers consider it for inclusion in RC, add a statement to the issue summary of why we need to make this change during RC, including what happens if we do not make the change and what any disruptions from it are. You can add a section
<h3>Why this should be an RC target</h3>
to the summary.Comment #23
xjmAlso, it's not really feasible to decide whether an issue is safe to do during RC when there's no patch yet (because we can't evaluate the disruption), so untagging for now.
Comment #24
catchI think we need to decide whether this is fixable in RC, or 8.x at all, before there's any point in working on a patch for it. Re-tagging for now.
Comment #25
xjmBased on #21, this either needs to be done in some messy BC way in a minor, or postponed until 9.x.
Comment #27
Fabianx CreditAttribution: Fabianx as a volunteer and at Tag1 Consulting commentedA few quick thoughts:
- Suggestions can be added easily, so more specific things can be targeted.
It is not necessary to remove the old suggestions to allow to prevent conflicts.
There will be confusion why there are two, so it would be nice to highlight deprecated suggestions in the twig output.
We might want to _always_ prefix suggestions and mark non-prefixed suggestions as deprecated.
Or we might want to use a new key: #deprecated_theme_suggestions that contain the same as theme suggestions to allow to differentiate.
Comment #28
lauriii+1 for the idea of just adding the new more specific suggestions.
Another idea would be to hide the deprecated theme suggestions from the theme debug list, but they would be still loaded normally if there is pre-existing template.
Comment #29
andypostNice idea! so the question how to document that?
Comment #30
alexpottDiscussed with @Cottser, @lauriii, @xjm, @joelpittet. We agreed that this is not a theme system issue so removing the Twig tag. Also removing the 'sprint' tag as this does not appear to be being sprinted on.
The discussion thought that we should be adding the new templates and deprecating the old template suggestions. We also thought that since we have a number of issues around template suggestions we should open an issue discussing how to make the system more robust.
Comment #31
catchMarking duplicate of #1367354: [PP-1] The list of theme hook suggestions for "field" is incorrectly ordered and has namespacing collisions.
Comment #32
xjm