search

yiisoft / yii2

Yii 2: The Fast, Secure and Professional PHP Framework
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
14.23k stars 6.91k forks source link

Remove beginPage() and endPage() in layouts #1507

Closed egorpromo closed 10 years ago

egorpromo commented 10 years ago

There is example of layout. Note the <?php $this->beginPage() ?> at the begining and <?php $this->endPage() ?> at the ending. I think these method calls would be in every layout. These are important calls because without them it is not possible to use $this->head(), $this->beginBody(), $this->endBody() anyway. It is possible to forget them. Another reason is complicated layout file. There are many method calls in layout. It is hard to remember all of them. I propose to remove them from layout file and place their functionality in other place. Then layout files will be a little simple. I have thoughts.

  1. if beginPage() and endPage() are needed in layouts only then it is possible to place their functionality in yii\web\Controller::render()
  2. If they are needed in all rendered files then it is possible to place their functionality in Response or other place.

What do you think? I think web-develpers don't want to place additional two lines of code every time the layout created.

samdark commented 10 years ago

The change is because in 1.1 begin and end page were marked with regexp and it was causing weird issues such as placing links before/after title depending on what markup was used.

Not sure if it can be simplified somehow w/o using content parsing.

egorpromo commented 10 years ago

I don't understand you. The goal is to place functionality of these to function outside layout. Why developers need them in layouts if their work could be done by framework in automatic?

klimov-paul commented 10 years ago

See: https://github.com/yiisoft/yii/pull/1866 https://github.com/yiisoft/yii/issues/1388

egorpromo commented 10 years ago

@klimov-paul Nothing interesting... Just take methods which are in EVERY layout and place their functionality outside user scope (in core yii scripts)

samdark commented 10 years ago

@egorpromo I think I'm lost. Can you explain your thoughts with code?

egorpromo commented 10 years ago 
<?php
use yii\helpers\Html;
?>
<!DOCTYPE html>
<html lang="<?= Yii::$app->language ?>">
<head>
    <meta charset="<?= Yii::$app->charset ?>"/>
    <title><?= Html::encode($this->title) ?></title>
    <?php $this->head() ?>
</head>
<body>
<?php $this->beginBody() ?>
    <div class="container">
        <?= $content ?>
    </div>
    <footer class="footer">© 2013 me :)</footer>
<?php $this->endBody() ?>
</body>
</html>

Look at the code above. There is no beginPage() and endPage calls. But we need to save their functionality in other place.

We can take endPage() from here and place its functionality in (for example) yii\base\Controller::render():

        public function render($view, $params = [])
        {
                $output = $this->getView()->render($view, $params, $this);
                $layoutFile = $this->findLayoutFile($this->getView());
                if ($layoutFile !== false) {
                        $output = $this->getView()->renderFile($layoutFile, ['content' => $output], $this);
                        //We have $output. We can do the functionality that is made by endPage().
                        $output = $this->endPage($ouput);
                        //Now we have modified $output.
                        return $output;
                } else {
                        return $output;
                }
        }

Of course some refactoring needed for realize my idea. But layouts will be more simple.

egorpromo commented 10 years ago

better:

$output = $this->getView()->endPage($ouput);
qiangxue commented 10 years ago

We have $output. We can do the functionality that is made by endPage().

How will you implement this?

egorpromo commented 10 years ago

I think there is no problem to implement this. We have yii\base\Controller::render() and $output. $output stores html markup of layout (without beginPage() and endPage() calls because I don't want it there). So it is possible modify html markup in yii\base\Controller::render():

        public function render($view, $params = [])
        {
//some code here
                $output  = $this->getView()->renderFile($layoutFile, ['content' => $output], $this); 
                $view=$this->getView();
                $view->trigger(self::EVENT_END_PAGE);
                foreach (array_keys($view->assetBundles) as $bundle) {
                        $view->registerAssetFiles($bundle);
                }
                $output = strtr($output, [
                        self::PH_HEAD => $view->renderHeadHtml(),
                        self::PH_BODY_BEGIN => $view->renderBodyBeginHtml(),
                        self::PH_BODY_END => $view->renderBodyEndHtml(),
                ]);

                unset(
                        $view->metaTags,
                        $view->linkTags,
                        $view->css,
                        $view->cssFiles,
                        $view->js,
                        $view->jsFiles
                );
//some code here
        }

It is not correct to bundle View and Controller but I have made this because I don't know your thoughts about for what are beginPage() and endPage() needed. If they are needed for layouts only then it is better to invent new method like yii\base\View::renderLayoutFile() for addition to yii\base\View::renderFile() and put the functionality in there.

qiangxue commented 10 years ago

How do you get the following code work?

$output = strtr($output, [
                        self::PH_HEAD => $view->renderHeadHtml(),
                        self::PH_BODY_BEGIN => $view->renderBodyBeginHtml(),
                        self::PH_BODY_END => $view->renderBodyEndHtml(),
                ]);
egorpromo commented 10 years ago

I think it is possible to remove yii\vase\View::beginPage(), yii\base\View::endPage() and yii\web\View::endPage(). Their functionality can be in renedering methods.

qiangxue commented 10 years ago

No need repeating your suggestion. Could you answer my last question? Assuming you get $output, how will you insert head, bodyBegin and bodyEnd into it?

egorpromo commented 10 years ago

@qiangxue Sorry, I am in hurry. We need the functionality in View class only. So we will have all constants of View.

qiangxue commented 10 years ago

? I'm not asking about syntaxes. I'm asking "Assuming you get $output, how will you insert head, bodyBegin and bodyEnd into appropriate places in the content?"

egorpromo commented 10 years ago

I think we must leave bodyBegin() and bodyEnd(). We need remove beginPage() and endPage() only without touching the rest methods.

qiangxue commented 10 years ago

If you already write beginBody, why would you complain about beginPage?

If we do what you suggested here, it would mean View needs to be aware of the "layout" concept and do special handling there. Then there will be issue about nested layouts, etc.

Ragazzo commented 10 years ago

Then there will be issue about nested layouts, etc.

sorry for interrupting, but will there be some nested layouts in Yii2? like blade in L4?

qiangxue commented 10 years ago

We already have it in 1.1 and 2.0.

Ragazzo commented 10 years ago

can you point to the docs maybe or give an example? i mean like this http://laravel.com/docs/templates

qiangxue commented 10 years ago

It's something like this in 1.1: https://github.com/yiisoft/yii/blob/master/demos/blog/protected/views/layouts/column2.php

egorpromo commented 10 years ago

View already aware of the "layout" concept because View have beginPage() and endPage() methods. And in any html markup needed only one beginBody(), endBody() etc. I think there will no be problem with nested layouts because they dont use beginBody(), endBody() etc.

qiangxue commented 10 years ago

beginPage doesn't mean layout. It's perfectly fine you put beginPage in a view that doesn't use any layout.

egorpromo commented 10 years ago

For what we need beginPage() and endPage()?

egorpromo commented 10 years ago

What is the cause to invent them?

egorpromo commented 10 years ago

I think they are for layouts only. I don't see other meaning.

egorpromo commented 10 years ago

The meaning of endPage() is to register asset files, to substitute markup which generated by beginBody(), endBody(), etc, and to unset assets. So it is for layouts only. The same for beginPage().

egorpromo commented 10 years ago

There is a little problem if someone wants to use so called 'mark methods' like beginBody() or head() in views (not layouts). If he does it then in view there is some markup like <![CDATA[YII-BLOCK-HEAD]]>. I think it is possible to prohibit to generate any markup if head() is used in view (not in layout).

qiangxue commented 10 years ago

They are provided so that you can respond to the corresponding events and possibly preprocess or postprocess the content in between. They don't have to be put in a layout file. They also serve as insertion points like beginBody does.

beginPage() can be dropped by adding additional methods to View, but then you will find the new methods are not easily named to avoid confusion with existing render methods. It also requires similar new methods in Controller that delegates the render methods from View.

egorpromo commented 10 years ago

I don't have anything against beginBody(), endBody(), head() etc. They work fine. I think beginPage() and endPage() can be removed safetly from layouts.

They don't have to be put in a layout file.

I don't agree. For what beginPage() and endPage() are? For layouts only. Correct me if I am wrong.

It also requires similar new methods in Controller that delegates the render methods from View.

Yes, I think so. But we will get rid of beginPage() and endPage() in layouts.

egorpromo commented 10 years ago

They are provided so that you can respond to the corresponding events and possibly preprocess or postprocess the content in between.

It is possible to trigger event in rendering method lke Controller::render() or in something similar.

qiangxue commented 10 years ago

I don't agree. For what beginPage() and endPage() are? For layouts only. Correct me if I am wrong.

I have answered you. When you render a partial view for AJAX response purpose, there is no layout.

Yes, I think so. But we will get rid of beginPage() and endPage() in layouts.

As I said, I am not saying we can't remove them. I'm saying removing them in favor of putting them in render method has more trouble. Since you already write beginBody etc. in a view, why are you against write beginPage?

If you insist in removing beginPage, please submit a PR and I will review it.

egorpromo commented 10 years ago

When you render a partial view for AJAX response purpose, there is no layout.

My thoughts are simple. beginPage() and endPage() are for layout only because they modify the markup of head(), beginBody(), etc only. head(), beginBody() are in layouts only, so what is relation to Ajax? I don't understand...

Since you already write beginBody etc. in a view, why are you against write beginPage?

  1. I can forget them in layout.
  2. It is easy to understand the layouts.
  3. It is simpler for me not to use it and forget about its existence
qiangxue commented 10 years ago

I'm closing this issue, as we are clearly not making any more constructive discussion. Please submit a PR if you insist in removing beginPage() and have a replacement for it. Thanks.

starrychloe commented 9 years ago

I agree. Rails doesn't need all those extra function calls in their layouts. They just use a method to find and insert the Javascript and CSS links.

http://guides.rubyonrails.org/layouts_and_rendering.html#asset-tag-helpers

I just got done trying to create a simplified layout for use in an iframe, but it was not at all obvious that I needed $this->endBody() and $this->endPage(). I only found this issue because Yii kept inserting a <![CDATA[YII-BLOCK-HEAD]]> where the link to Bootstrap should be and this was the only result when I searched.

samdark commented 9 years ago

Yii 1.1 worked like that and there were issues with such approach of search and replace.

numibu commented 7 years ago

how insert this block with method $this->beginPage() ?!

<!DOCTYPE html>
<!--[if lt IE 7]>      <html class="no-js lt-ie9 lt-ie8 lt-ie7"> <![endif]-->
<!--[if IE 7]>         <html class="no-js lt-ie9 lt-ie8"> <![endif]-->
<!--[if IE 8]>         <html class="no-js lt-ie9"> <![endif]-->
<!--[if gt IE 8]><!--> <html class="no-js"> <!--<![endif]-->
  <head>