Problem/Motivation

The name 'page' suggests what is being rendered is the entire HTML page. But that's not the case. All that's being rendered are the contents of the <body> tag.

For all intents and purposes, page.html.twig is "the body", and {{ page_top }} and {{ page_bottom }} are a <body> prefix and suffix, respectively.

Then why do we still call it "the page template"? It actually is "the body template". Once you label it this way, all of the surrounding language starts to make much more sense.

Proposed resolution

  1. Rename #type => 'page' to #type => 'body'
  2. Rename page.html.twig to body.html.twig

Remaining tasks

TBD.

User interface changes

None.

API changes

  1. Renamed #type => 'page' to #type => 'body'
  2. Renamed page.html.twig to body.html.twig
Files: 
CommentFileSizeAuthor
#8 2372581-page_body-8.patch9.79 KBandypost
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 99,822 pass(es). View

Comments

andypost’s picture

Status: Active » Needs review
FileSize
5.33 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 58,961 pass(es), 27,195 fail(s), and 1,289 exception(s). View

Initial stub on rename - renamed element. let's see test coverage

Suppose we can easily rename element in beta because this code introduced in 8.x
Renames of templates is tricky, maybe we can provide BC shim for old ones and "assert" some message to user that used theme has "deprecated templates" somehow

Status: Needs review » Needs work

The last submitted patch, 1: 2372581-page_body-1.patch, failed testing.

steamx’s picture

Hm i guess there is a need for renaming all the theme_preprocess_page hooks as well. Actually the body content is the page content. You won't find anything really necessary in that can be rendered in your browser.

For history reasons i'd not suggest to change that.

andypost’s picture

Status: Needs work » Needs review
FileSize
859 bytes
5.89 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 99,804 pass(es), 15 fail(s), and 0 exception(s). View

Should fix a lot of tests

cilefen’s picture

Issue tags: +Needs beta evaluation

This needs a beta evaluation to get in now. If it does not, this is such a BC break that it will have to wait for Drupal 9.

cilefen’s picture

Unless there is a backwards-compatible way to do this.

Status: Needs review » Needs work

The last submitted patch, 4: 2372581-page_body-3.patch, failed testing.

andypost’s picture

Status: Needs work » Needs review
FileSize
5.9 KB
9.79 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 99,822 pass(es). View

Fix more tests and clean-up comments

About BC - I'm pretty sure that element type should be "body" so maybe better to split the issue into rename type and rename templates (suppose here a lot of bikeshed)

Wim Leers’s picture

So, I think #8 is proposing to rename it internally, but still keep page.html.twig as the template name for now?

andypost’s picture

@Wim yep, that's why I said about split

Wim Leers’s picture

I wonder how themers feel about that. AFAIK that'd be the first case where the #type does not match the *.html.twig template name.

andypost’s picture

@Wim I see no BC for #3 otoh that's not a hook so looks fine. so no idea how to fit in beta and 8.x

lauriii’s picture

Templates are not frozen so I think this should be acceptable. And changing element might not have to be considered as a API change.

+1 to this because of the page.html.twig has been very confusing always.

lauriii’s picture

Issue tags: +Needs change record
lauriii’s picture

Even though I'm +1 to this, I talked quickly with Lewis and few ideas/problems bumped in:

  • Body might be confusing between body field
  • Calling #type => 'body' might be a little confusing, how ever its being called in very rare circumstances
  • Might be worth investigating what other CMSs and frameworks are doing if there is something which makes more sense

Anyway this makes sense since html.html.twig is creating the html element and body.html.twig would be creating the body element.

davidhernandez’s picture

I do not agree. We are way too late in the process to rename the page template. That is a huge change.

It is a drupalism we've been stuck with for a long time, and I'd like to change it as much as anyone else, but not weeks before RC.

What advantage would there be to renaming it internally, if the template isn't changed. That sounds like an inconsistency we can do without.

Wim Leers’s picture

Would html_body instead of body alleviate the concerns in #15?

lauriii’s picture

I prefer body over html_body. I think body is still something we can use if we don't come up with a better name.

@davidhernandez: it should be easy to adopt for the new name of the template. It means in most of the cases only renaming the template.

davidhernandez’s picture

There is more to this than simply renaming a template. It is an entire conceptual change. People have become accustom to the existence of the page template. The documentation, tutorials, books, etc, all reference it. Changing it pre-beta or early beta when some features were still being worked out would have been fine, but doing it now without a lot of discussion or fanfare could be very confusing.

Wim Leers’s picture

#19: But, as the IS says:

The name 'page' suggests what is being rendered is the entire HTML page. But that's not the case. All that's being rendered are the contents of the <body> tag.

It is the current name that is confusing, that is a Drupalism. Yes, changing it will mean many old tutorials and books will be outdated. But those tutorials and books are for D7 anyway.

davidhernandez’s picture

It depends on your point of view. For all intents and purposes the body tag is the whole page, for someone concerned with content or theming. There have been many discussions in different issues about how the whole html/page combo needs to be completely redone, but we've been too far into development to do it.

My concern is simply disruption versus benefit. This will be disruptive, so the benefits need to be clear. Are we gaining more than just renaming something? Does this resolve any internal conflict, or benefit a particular use case?

Wim Leers’s picture

My concern is simply disruption versus benefit. This will be disruptive, so the benefits need to be clear. Are we gaining more than just renaming something? Does this resolve any internal conflict, or benefit a particular use case?

+1

I'd like to get feedback from more front-end people :)

mortendk’s picture

if we wanna do anything on the page issue -then it should not be to rename page.html.twig to body.html.twig but instead merge page.html.twig & html.html.twig to remove that exatra template that dont gives us any benefits (and those can be fixed with a twig block)

I have a hard time seeing a win by renaming page to body at this point, yes its a drupalism but calling it body would also suggest that its where the body tag gets rendered ... and then html.html.twig makes even less sense.

davidhernandez’s picture

calling it body would also suggest that its where the body tag gets rendered

Yeah, that's a good point regarding more confusion. The tag it seems to reference isn't even in there.

mherchel’s picture

I think body sounds fine. That being said, I think most Drupal themers will get it. The real question is if it's going to make sense to non-drupal frontenders

andypost’s picture

page exists just because we use 4 hooks but all of them are for HTML template (build and rendered here)
https://api.drupal.org/api/drupal/8/search/hook_page_
hook_page_top()
hook_page_bottom()

hook_page_attachments() and hook_page_attachments_alter()

Also maintenance and install pages re-use this but there's separate issue #2231853: Rename maintenance-page template into page--maintenance + install-page into page--maintenance--install

andypost’s picture

Another suggestion is "layout" for element name - looks the exactly what it is

LewisNyman’s picture

I think the HTML tag is sticking as close to frontender knowledge as you can get. Having said that, I see layout a lot in static site generators like Jekyll, Middleman and other tools like Squarespace

andypost’s picture

@LewisNyman the last 2 examples use actually the same naming - layout that contains regions

Also "render element" for our page contains of "page" variable that contains regions, that's why I suggest to use layout

  foreach (\Drupal::theme()->getActiveTheme()->getRegions() as $region) {
    if (!isset($variables['page'][$region])) {
      $variables['page'][$region] = array();
    }
  }
RavindraSingh’s picture

Assigned: Unassigned » RavindraSingh
RavindraSingh’s picture

Assigned: RavindraSingh » Unassigned
Wim Leers’s picture

I think merging the concepts of html and page makes a ton of sense. The arguments in #23 + #26.

To me, that's frankly always been one of the main WTFs, that was also confusing to explain to front-end people that aren't familiar with Drupal. The html template without the page template is almost meaningless!

Can we still make that change?

davidhernandez’s picture

Can we still make that change?

I would also question the lateness of such a change; however, it is something many of us have wanted to do. Whenever it was previous brought up, I believe the reason not to do had to do with caching, but I don't remember the specifics.

Wim Leers’s picture

I think it should be possible now. :)

davidhernandez’s picture

I'd honestly prefer effort put into that than arguing over renaming page. The split is the bigger WTF. For example, all of the page level CSS classes get printed in html.html.twig because that is where the body tag is. We are also doing a lot of work right now to remove most of the variables in the page templates, so the code there will get a lot smaller. Merging them wouldn't make the result unreadable at all.

Wim Leers’s picture

The split is the bigger WTF.

+1

Let's do it!

andypost’s picture

Assigned: Unassigned » andypost

Will try to attack that, I see the best to merge hook__preprocess_page() into hook__preprocess_html()

Most of them are slim so disruption is not big

davidhernandez’s picture

I think it would be a good idea to get sign off from both theme system maintainers and core committers before putting too much effort into it. I can easily see the committers saying, "This isn't critical, so why are we doing this now?" And if they don't think it is a good idea at this stage, it has to be pushed back to 9.x. We can't touch the templates in a point release.

Is there justification besides TX/DX? Can we get a performance boost merging them?

Wim Leers’s picture

It's one less template to render, so one less file system hit. So, yes, there should be a performance improvement, but I expect it to be very modest.

I think improved TX/DX, and one less Drupalism are the best reasons.

Wim Leers’s picture

#40: I understand why the first two issues are kinda blockers for this. But the third one I don't understand. Status messages are already a block; they've been moved out of the page template a long time ago.

Also, I think this issue can continue despite those soft blockers? I'm happy to help resolve conflicts on any of those issues, or this issue, if that helps.

andypost’s picture

Looks the issue is a duplicate of #2090967: Extend page.html.twig with html.html.twig
The issue above pushed to 9.x, but now most of reasons are resolved so maybe better to proceed there?

Wim Leers’s picture

Let's try.

Wim Leers’s picture

Version: 8.0.x-dev » 9.x-dev
catch’s picture

Version: 9.x-dev » 8.1.x-dev
Status: Needs review » Postponed

Worth trying a bc layer + deprecation.

Jeff Burnz’s picture

Not really sure about this merge or rename.

How will this work with layouts? ATM {{ page }} is dead easy to take over and control, not so easy if this is all one big template, not a DRY approach repeating head etc for the sake of a layout.

Actually renaming this to layout is not a bad idea also, but personally I have not noticed a whole lot of confusion with regards to page.html.twig naming. None actually.

The effort I would like to see, personally, is to get rid of page.html.twig entirely and just have a notion of layout plugins, like Panels in core sort of thing.

Cottser’s picture

@Jeff Burnz don't worry the merging thing was hashed out in #2090967: Extend page.html.twig with html.html.twig, this is only about renaming (only about what's in the issue summary).

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

andypost’s picture

Status: Postponed » Needs work

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

joelpittet’s picture

Issue tags: +Twig

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.