Problem/Motivation

These variables are set in user_template_preprocess_default_variables_alter() and don't need to be duplicated in the node.html.twig documentation, which is approximately a metre long.

Steps to reproduce

Proposed resolution

Remaining tasks

Remove the same documentation lines from duplicates of node.html.twig which is included in various core themes.

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3458782

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

catch created an issue. See original summary.

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs work
Issue tags: +Novice

Made a start - but the change needs to be made in every copy of node.html.twig in core, of which there are a lot.

catch opened merge request !8648

catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

I am certain readmore can go as well, this was dropped way way back in #372743: Body and teaser as fields - before 7.0 was released! - but lingers on in the documentation, even though it doesn't work.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Title: Remove documentation for logged_in and is_admin from node.html.twig » Remove documentation for readmore, logged_in and is_admin from node.html.twig
catch’s picture

catch
he/him
Primary language English
commented

Yes let's do that one here too! Unbelievable levels of cruft.

ankitv18 made their first commit to this issue’s fork.

ankitv18’s picture

ankitv18 commented

Hi @catch and @longwave
I've removed the mentioned doc variables from node--*.html.twig and node.html.twig from the respective profile, themes and modules

MR!8648 still in draft state @catch could you please move MR into review.

ankitv18’s picture

ankitv18 commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs change record
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review
Issue tags: -Needs change record

In my opinion no change record is required, this is a documentation improvement/fix only.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Reviewed & tested by the community

No further instances remain, to me this is RTBC. As a docs only fix I think this can be backported to 10.3.x if we want to.

smustgrave’s picture

smustgrave commented

But there are tons of node.html.twig templates in themes out there. If you use the CR system to figure out when to update your theme templates this will be missed.

catch’s picture

catch
he/him
Primary language English
commented

@smustgrave but nothing in the actual twig template content needs to change, if people copy the entire docblock from core that is up to them. We don't add CRs for any other documentation changes, but people do copy and paste those around too.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Fair enough, we don't often write change notices just for themers, but here's an attempt for this one: https://www.drupal.org/node/3460562

smustgrave’s picture

smustgrave commented

I know for a theme I have I started with the templates from the starterkit theme using the generator.

Thanks @longwave!

nod_ closed merge request !8648

  • nod_ committed 873b1e7b on 10.3.x 
    Issue #3458782 by catch, ankitv18, longwave, smustgrave: Remove...

  • nod_ committed 1f4aa5cf on 10.4.x 
    Issue #3458782 by catch, ankitv18, longwave, smustgrave: Remove...

  • nod_ committed 98f85092 on 11.0.x 
    Issue #3458782 by catch, ankitv18, longwave, smustgrave: Remove...

  • nod_ committed 86138d4c on 11.x 
    Issue #3458782 by catch, ankitv18, longwave, smustgrave: Remove...
nod_’s picture

nod_
Primary language French
Location Lille
commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 86138d4c11 to 11.x and 98f8509200 to 11.0.x and 1f4aa5cf19 to 10.4.x and 873b1e7b07 to 10.3.x. Thanks!

ressa’s picture

ressa
he/him
commented

Thanks for the clean up everyone. Template variables that do nothing are among the thousand paper cuts, which seem innocent enough, but they all add up.

And thanks for suggesting and creating a Change Record @smustgrave and @longwave, which is how I heard about this improvement.

I manually open the Change Record page daily, to see if anything exciting has happened in Drupal. If only we could subscribe to Change Records ... maybe some day it will happen? #3094094: Allow email subscriptions to Change records

Status: Fixed » Closed (fixed)

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