Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
egorpromo opened this issue Dec 13, 2013 · 36 comments
Closed

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

egorpromo opened this issue Dec 13, 2013 · 36 comments

Comments

@egorpromo
Copy link
Contributor

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
Copy link
Member

samdark commented Dec 13, 2013

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
Copy link
Contributor Author

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
Copy link
Member

See:
yiisoft/yii#1866
yiisoft/yii#1388

@egorpromo
Copy link
Contributor Author

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

@samdark
Copy link
Member

samdark commented Dec 13, 2013

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

@egorpromo
Copy link
Contributor Author

<?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
Copy link
Contributor Author

better:

$output = $this->getView()->endPage($ouput);

@qiangxue
Copy link
Member

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

How will you implement this?

@egorpromo
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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

@qiangxue
Copy link
Member

? 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
Copy link
Contributor Author

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

@qiangxue
Copy link
Member

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
Copy link
Contributor

Ragazzo commented Dec 14, 2013

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
Copy link
Member

We already have it in 1.1 and 2.0.

@Ragazzo
Copy link
Contributor

Ragazzo commented Dec 14, 2013

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

@qiangxue
Copy link
Member

@egorpromo
Copy link
Contributor Author

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
Copy link
Member

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

@egorpromo
Copy link
Contributor Author

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

@egorpromo
Copy link
Contributor Author

What is the cause to invent them?

@egorpromo
Copy link
Contributor Author

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

@egorpromo
Copy link
Contributor Author

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
Copy link
Contributor Author

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 . I think it is possible to prohibit to generate any markup if head() is used in view (not in layout).

@qiangxue
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link

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
Copy link
Member

samdark commented Apr 10, 2015

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

@numibu
Copy link

numibu commented Feb 19, 2017

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>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants