Problem/Motivation

The documentation in field.html.twig is now incorrect/outdated with regard to template suggestions:

 * To override output, copy the "field.html.twig" from the templates directory
 * to your theme's directory and customize it, just like customizing other
 * Drupal templates such as page.html.twig or node.html.twig.
 *
 * For example, for a field named 'body' displayed on the 'article' content
 * type, any of the following templates will override this default
 * implementation. The first of these templates that exists is used:
 * - field--body--article.html.twig
 * - field--article.html.twig
 * - field--body.html.twig
 * - field.html.twig

Current template suggestions for the body field via twig_debug:

<!-- THEME DEBUG -->
<!-- THEME HOOK: 'field' -->
<!-- FILE NAME SUGGESTIONS:
   * field--node--body--article.html.twig
   * field--node--body.html.twig
   * field--node--article.html.twig
   * field--body.html.twig
   x field--text-with-summary.html.twig
   * field.html.twig
-->
<!-- BEGIN OUTPUT from 'core/modules/text/templates/field--text-with-summary.html.twig' -->

Proposed resolution

Either update the documentation, or remove it and/or point to better documentation instead. If we are updating the documentation the body field may not be the best example to use since it's now a special-cased template (field--text-with-summary.html.twig).

Remaining tasks

Discuss.
Patch.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Unfrozen changes Unfrozen because it only changes documentation.

User interface changes

n/a

API changes

n/a

Files: 
CommentFileSizeAuthor
#23 Fixed issue of Issue number 2396553-2396553-23.patch2.03 KBnripeshtrivedi
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 85,934 pass(es). View
#20 Fixed issue of Issue number 2396553-2396553-20.patch2.01 KBnripeshtrivedi
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,519 pass(es). View
#17 Fixed issue of Issue number 2396553-2396553-17.patch2.01 KBnripeshtrivedi
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,229 pass(es). View
#13 Fixed issue of Issue number 2396553-2396553-13.patch1.95 KBnripeshtrivedi
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,135 pass(es). View
#11 drupal-fix-template-suggestions-2396553-11.patch1.28 KBfullerja
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,986 pass(es). View
#7 8.0x-Fixed issue of Issue number 2396553-2396553-7.patch1.3 KBnripeshtrivedi
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,221 pass(es). View

Comments

dawehner’s picture

Just curious, how likely is it that the field template is the first template people would like to override?
Given that twig debug is a core feature I would assume that pretty much everyone will be aware of it already.
IMHO explaining the concept of overrides is certainly wrong, but the question is, whether it should explain where to find the documentation, or whether you think it would be more pointless. I'd argue that this depends on the first question.

Cottser’s picture

I don't think field template is the first one people will want to override. Probably a bigger template, something like node, page, html. So yeah, having said that maybe just removing this documentation makes the most sense here.

davidhernandez’s picture

I would think overriding field or a field is more likely than html. For someone starting out, it may be more likely than page. Node is probably the most common, but in D8, I don't know.

I took a quick glance, and I don't see these "for example" override instructions in any other kind of template. As Cottser pointed out in the summary, using body as an example has problems, but the other fields we have out of the box have issues too. Also, I don't think people first look in the actual template file to find instructions on how to properly name it. Considering all that, I'm fine with removing the text.

jhodgdon’s picture

It seems like we should have a section on the Theming topic that explains what overrides are in general, and then on the Field template instead of having the explanation, we could link to that and just list what the overrides are (or what their structure is).

Actually, there's already a section on https://api.drupal.org/api/drupal/core!modules!system!theme.api.php/grou... "Theme hook suggestions".

So all we probably need to do is say something like:

Instead of overriding the theming for all fields, you can also just override theming for a subset of fields using @link themeable Theme hook suggestions. @endlink For example, for a body field on a node:
...

davidhernandez’s picture

We could link to this page in the theming guide. https://www.drupal.org/node/2354645

jhodgdon’s picture

I would be in favor of linking the *topic* to that page, but not each individual twig template that wants to list its suggestions. IMO the individual twig templates should like to the @defgroup topic, and if that topic needs more information we can add it there, and also link that to the theming guide. (The reason for this is that as much as possible we want the essential information to be in the source code base and on api.drupal.org. Not everyone is connected to the Internet at all times to go read drupal.org.)

nripeshtrivedi’s picture

FileSize
1.3 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,221 pass(es). View

Hi,

Based on the comments, I am submmiting a patch for this issue. Please review it and let me know if any changes are required

jhodgdon’s picture

Status: Active » Needs work

Thanks for the patch! A few notes:

a) When you upload a patch, set the status of the issue to "needs review" to alert both human reviewers and the test bot that a patch has been provided.

b) No documentation line should be longer than 80 characters.

c) When you make a list in documentation, use this syntax:
https://www.drupal.org/node/1354#lists

d) Based on the information above, I do not think the information in the patch is accurate. For instance, the first example in the issue summary is field--node--body--article.html.twig, which is not the same as field--field-name--content-type.html.twig. What it really is: field--entity-type--field-name--bundle-name.html.twig.

e) As discussed above, we need a link to the topic that tells about theme hook suggestions.

nripeshtrivedi’s picture

Hi,

Thanks for the valuable comments. I came acroos this page which reflects on theme hook suggestions in drupal 8- https://www.drupal.org/node/2186401. If you could let me know it suffices, I would be glad to submit a patch for it

jhodgdon’s picture

I am not sure what you are suggesting... What we want is:
- The html.twig file should link to the topic (see #4)
- The topic (which is in the theme.api.php file in core/modules/system and is called @defgroup themeable) should link to that drupal.org page that you found.

fullerja’s picture

Status: Needs work » Needs review
FileSize
1.28 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,986 pass(es). View

Added the template suggestions from the description and the link from #4.

fullerja’s picture

Issue tags: +SprintWeekend2015
nripeshtrivedi’s picture

FileSize
1.95 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,135 pass(es). View

Hi,

Based on the comments, I am submitting a patch for this issue. Any comments or suggestions are really appreciated

jhodgdon’s picture

Status: Needs review » Needs work

Thanks for the patches! Regarding the patch in #13:

a)

+ * theming for a subset of fields using @link themeable Theme hook suggestions.
+ * @endlink For example, for a body field on a node:

@link ... @endlink needs to be all on one line. So you probably have to have a newline before the @link to fit it all in one line.

b) same code block... I think maybe the second sentence here should say "For example, here are some theme hook suggestions that can be used for a body field on an article node type:" ?

c)

+ * 
+ * For further information on overriding theme hooks in drupal 8 see
+ * https://www.drupal.org/node/2186401

- Please do not say "Drupal 8". This documentation is likely to be moved into Drupal 9 with no change. Actually, just take out the "in drupal 8" completely. We don't need it. And by the way if you do need to say "drupal" in documentation it should be "Drupal".

Thanks!

fullerja’s picture

I rolled a patch in #11 that doesn't have the mentioned issues. Would you consider reviewing that one, or would it be better for me to roll a new patch/interdiff from #13?

jhodgdon’s picture

The patch in #11 is not complete. See #10.

nripeshtrivedi’s picture

FileSize
2.01 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,229 pass(es). View

Thanks for reviewing the patch. I am submitting a modified patch now as per the instructions. Please review it.

nripeshtrivedi’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Status: Needs review » Needs work

Great, now we're getting closer!

A few things to fix in this latest patch:

a) There are spaces at the ends of many of the lines. Set your programming text editor to either remove or highlight end-of-line spaces.

b) The sentence before "For example" in the .twig file does not end in a . -- put this inside the @link ... @endlink, so it will say "... suggestions. @endlink For example, ..."

c) I just read through the issue summary again and it looks like maybe the body field isn't the best example. So let's change "body" to "field_foo" (note that in Twig file names, the _ becomes - but in the sentence before you should use _ for the field name).

nripeshtrivedi’s picture

FileSize
2.01 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,519 pass(es). View

Thanks again for reviewing my patch and for the suggestions. I have corrected the the things as mentioned this time. Please let me know if it is fine.

nripeshtrivedi’s picture

Status: Needs work » Needs review
Cottser’s picture

Status: Needs review » Needs work

Thanks @nripeshtrivedi, some items I can see:

  1. Regarding #19.b, I think the change was not to make the link text longer but simply to include a period inside the @link...@endlink.

    Before - Code from #17 is closer, so referring to it instead:

    +++ b/core/modules/system/templates/field.html.twig
    @@ -7,12 +7,16 @@
    + * Instead of overriding the theming for all fields, you can also just override ¶
    + * theming for a subset of fields using ¶
    + * @link themeable Theme hook suggestions @endlink For example, ¶
    + * here are some theme hook suggestions that can be used for a body field ¶
    + * on an article node type:
    

    After - Should (I think) look more like:

    +++ b/core/modules/system/templates/field.html.twig
    @@ -7,12 +7,16 @@
    + * Instead of overriding the theming for all fields, you can also just override
    + * theming for a subset of fields using
    + * @link themeable Theme hook suggestions. @endlink For example, here
    + * are some theme hook suggestions that can be used for a field_foo field on an
    + * article node type:
    

    Changes made:

    • Period added inside @link...@endlink
    • "body" changed to "field_foo"
    • Lines re-wrapped at 80 characters
    • Trailing whitespace removed
  2. +++ b/core/modules/system/templates/field.html.twig
    @@ -7,12 +7,15 @@
    + * suggestions @endlink For example,here are some theme hook suggestions
    

    Missing space after comma here.

  3. +++ b/core/modules/system/templates/field.html.twig
    @@ -7,12 +7,15 @@
    + * - field--node--field_foo--article.html.twig
    + * - field--node--field_foo.html.twig
    + * - field--node--article.html.twig
    + * - field--field_foo.html.twig
    + * - field--text-with-summary.html.twig
    

    In all these lines, as @jhodgdon said in #19.c, it needs to be field-foo not field_foo here.

nripeshtrivedi’s picture

FileSize
2.03 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 85,934 pass(es). View

I think I misinterpreted the information. I have corrected the pointed out mistakes. I hope it is up to the mark. Please review it

nripeshtrivedi’s picture

Status: Needs work » Needs review
jhodgdon’s picture

PLEASE make an interdiff file when you upload a new patch on an existing issue!

Anyway, that aside, I think the latest patch looks good. Can someone else verify that the suggestions are all valid?

joelpittet’s picture

FYI, if you haven't ever made an interdiff before or need a refresher see the documentation https://drupal.org/documentation/git/interdiff
;)

nripeshtrivedi’s picture

thanks for the comments. I will keep the suggestions in mind. If you could please verify the latest patch and let me know if it is alright

jhodgdon’s picture

Right, I would really appreciate it if one of our theming experts could take a final look at the patch and make sure the suggestions are accurate. Thanks!

davidhernandez’s picture

Status: Needs review » Reviewed & tested by the community

The template suggestions are correct.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 23: Fixed issue of Issue number 2396553-2396553-23.patch, failed testing.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 23: Fixed issue of Issue number 2396553-2396553-23.patch, failed testing.

davidhernandez’s picture

Yeah, something is up with testbot. The patch is fine.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 23: Fixed issue of Issue number 2396553-2396553-23.patch, failed testing.

Status: Needs work » Needs review
nripeshtrivedi’s picture

Based on #25, #29 , #33 and re testing it again, I am marking it as reviewed and tested by community

nripeshtrivedi’s picture

Status: Needs review » Reviewed & tested by the community
davidhernandez’s picture

We had broken HEAD last night. That is why the test was failing. It should be good now.

nripeshtrivedi’s picture

yeah, the last test went alright

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Documentation fixes are not blocked in beta. Committed 58bd271 and pushed to 8.0.x. Thanks!

  • alexpott committed 58bd271 on 8.0.x
    Issue #2396553 by nripeshtrivedi, fullerja: field.html.twig...
nripeshtrivedi’s picture

Thanks for the commit. I am a little new to community, just curious to know, When I was working on a flag patch and after it was committed, it showed in my profile, does it work a little differently with drupal core ?

davidhernandez’s picture

@nripeshtrivedi, commit credits for Drupal core do not currently show up on your user profile. For now you can see your progress on places like drupalcores.com. And I can see you there at the bottom with your first commit mention. Congratulations!

nripeshtrivedi’s picture

Thanks, it was a pleasure to contribute :)

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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