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

Eliminating beginPage() and endPage() in layouts #1548

Closed
wants to merge 2 commits into from

Conversation

egorpromo
Copy link
Contributor

Sorry. My knowlege about Git are very bad. I make some proposal to eliminate beginPage() and endPage() for #1507

@samdark
Copy link
Member

samdark commented Dec 16, 2013

This one should not be merged (there changes from other branches). Keeping it for discussion purpose.

@egorpromo
Copy link
Contributor Author

Ok. Just take a look how it is easy.

@samdark
Copy link
Member

samdark commented Dec 16, 2013

It looks like it's eliminating a chance to register assets at the very top of layout like the following:

<?php
use yii\helpers\Html;
use yii\bootstrap\Nav;
use yii\bootstrap\NavBar;
use yii\widgets\Breadcrumbs;
use app\assets\AppAsset;

/**
 * @var \yii\web\View $this
 * @var string $content
 */
AppAsset::register($this);
?>
<?php $this->beginPage() ?>

@qiangxue
Copy link
Member

@egorpromo Why do you still insist that beginPage/endPage belongs to layout? As I repeatedly many times, this is not the design. beginPage/endPage just marks the begin/end of a complete view-based response. You can use it in layouts or views if layout is not needed.

@qiangxue
Copy link
Member

@samdark Yes, this is a side effect of eliminating beginPage/endPage. I actually thought about eliminating beginPage/endPage as @egorpromo suggested. But I didn't come up with a clean solution.

@egorpromo
Copy link
Contributor Author

@samdark No. Why do you think now it is not possible?

@egorpromo
Copy link
Contributor Author

@qiangxue The endPage() is only for layouts file. There is no need to have endPage() if we don't have head(), beginBody() and endBody(). So, endPage() is for layouts only.

@qiangxue
Copy link
Member

Let me try to explain one last time. beginPage/endPage marks the begin/end of a complete view-based response. What you saw in yii\web\View is just a special use case of them. Note that beginPage/endPage is defined in yii\base\View. You can use beginPage/endPage in non-layout view files so that 3rd party code has a chance to inject custom code at these places.

@lucianobaraglia
Copy link
Contributor

Maybe renaming those to beginView() and endView() @egorpromo would be happy?

@egorpromo
Copy link
Contributor Author

Note that beginPage/endPage is defined in yii\base\View.

We had one View class recently. It is @samdark has made strange stuff in View.

You can use beginPage/endPage in non-layout view files so that 3rd party code has a chance to inject custom code at these places.

It is abstract meaning. My thoughts simple is better. It is juggernaut you can't ignore.

@egorpromo
Copy link
Contributor Author

@lucianobaraglia I was unhappy because I have a look at Yii1. After many years of developement Yii has lose his initial good arhitecture. I was very upset. Now it sems yii-developers take into accout the last deficiencies and make more good framework. So I become a little happy ☺️

@qiangxue
Copy link
Member

@egorpromo I'm not saying beginPage() cannot be eliminated (I have repeatedly this several times). I'm not asking for a proof-of-concept implementation here. It is the API design detail that makes eliminating beginPage() not a simple task. The current PR doesn't reach the goal yet.

@egorpromo
Copy link
Contributor Author

@qiangxue What prevents to accept my PR?

@qiangxue
Copy link
Member

Please read my previous response.

@egorpromo
Copy link
Contributor Author

I'm not saying beginPage() cannot be eliminated

You say it is possible to eliminate beginPage(). If it is possible could you eliminate it?

@qiangxue
Copy link
Member

I was saying I couldn't find a clean solution to this. Also having beginPage() isn't that bad as you would claim. If you could accept beginBody(), I see no reason why you couldn't accept beginPage(). Saying "I may forget beginPage()" isn't a good argument.

@egorpromo
Copy link
Contributor Author

@qiangxue Ok. I have seen so many times these methods... Of course now I will not forget it.

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

Successfully merging this pull request may close these issues.

None yet

4 participants