[NFR] Implement ViewModels

nikolasha commented 11 years ago

In my opinion, Phalcon\Mvc\View is not made very well. This is especially noticeable when there are complex templates with a lot of partials that are nested into each other. In particular, the problem is that all variables are set to View are common to all used templates. Another problem is the inability to create a complex sequence of rendering templates. Here are lacking in something like the Zend\View\Model\ViewModel.

phalcon commented 11 years ago

Hi, could you please follow this guide https://github.com/phalcon/cphalcon/wiki/New-Feature-Request---NFR to submit new NFR?

nikolasha commented 11 years ago

Example:

<!-- main layout -->
<html>
<body>
    <?= $this->partial('sidebar') ?>
    <?= $this->getContent() ?>
    <?= $this->partial('footer') ?>
</body>
</html>

<!-- sidebar partial-->
<div class="sidebar">
    <h1><?= $sidebarTitle ?></h1>
    <?= $sidebarContent ?> <!-- $sidebarContent depends on the page -->
    <!-- ... -->
</div>

<!-- list-docs partial -->
<ul class="list-docs">
    <?php foreach($docs as $doc): >
    ...
</ul>

<?php
class DocsController extends \Phalcon\Mvc\Controller
{
    public function indexAction()
    {
        $this->view->sidebarTitle = 'Documents'; //this is a little magic
        $this->view->sidebarContent = ???; //how to generate a list-docs?
        $this->view->docs = \App\Model\Documents::find();
    }
}

<!-- views/docs/index.phtml-->
<?php
$this->view->sidebarContent = $this->partial('list-docs'); // also magic
// UPD: it does not work, partial() immediately produces output data :(
?>
...

nikolasha commented 11 years ago

I would like to see something like this:

class DocsController extends \Phalcon\Mvc\Controller
{
    public function indexAction()
    {
        $sidebar = $this->getLayout()->sidebar;
        // or  $sidebar = $this->view->sidebar;
        // $sidebar is ViewModel created somewhere in the initialization
        $sidebar->title = 'Documents';
        $sidebar->content = new ViewModel(['docs' => Documents::find()], 'partials/list-docs');

        return new ViewModel(['document' => Documents::findFirst()])
    }
}

<!-- sidebar partial-->
<div class="sidebar">
    <h1><?= $title ?></h1>
    <?= $content ?> <!-- call $content->__toString() -> rendering "partials/list-docs" ?>
    <!-- ... -->
</div>

phalcon commented 11 years ago

I changed the title to 'Implement ViewModels', this is already requested in #218

nikolasha commented 11 years ago

Hi, @phalcon!

You can see my version of the implementation of the ViewModels within the Phalcon? Maybe you'll be interested in this variant. There are examples of use, though a bit confusing.

Can say this is an adaptation ZF2 ViewModel in Phalcon. Phalcon now allows only strictly linear rendering from the action view to the main layout. My version allows you to build a tree-like hierarchy of view models at each level of the rendering. It is important that the current behavior of the fully preserved.

phalcon commented 11 years ago

Hey, thanks for the extensive example, the implementation looks great, but since 1.0.0 is close to being released this could be addressed in 1.1, could you please help us adding unit-tests to the 1.1 branch (once 1.0 is released) ensuring that the implementation is correct?

nikolasha commented 11 years ago

I can try :) But first you need to determine how the ViewModel will be implemented Phalcon - just like me or you make your edits. I, for example, automatically convert the name of the method "actionName" to the name of template "action-name", etc.

phalcon commented 11 years ago

I was thinking about this, maybe we need to introduce a complete brand new View component sharing some of the current functionality but setting up explicitly which view must be rendered, leaving the current implementation as it is.

<?php

class PostsController extends Phalcon\Mvc\Controller
{
        public function showAction()
        {
               return $this->view->render('posts/show', array('posts' => Posts::find());
        }
}

nikolasha commented 11 years ago

Sorry, I do not quite understand what you mean, but your example is not what we need :(

phalcon commented 11 years ago

Right now, the view component implements automatic rendering, so if the latest controller is Posts and the latest action is "show", if tries to render posts/show automatically.

Changing that behavior, or introducing a new subcomponent/component that doesn't do that, but the developer must explicitly tell which view must be rendered in each action.

nikolasha commented 11 years ago

What good is it? Your example is just extra unnecessary steps and no benefit. The current implementation of the View component is acceptable. I proposed an extension that fully preserves the current behavior, while allowing very flexible configuration chain rendering and better structured code.

In most cases, for simple pages of current behavior is more than enough. My option is useful when the page contains a lot of blocks and each block can vary greatly depending on the situation. In this case, the current implementation of the View component templates are very dirty (contain a lot of server-side code, loops, conditional branches), and there's enormous number of variables that are common to all the templates which are easy to get confused.

And then, in my case, your example would be:

<?php

class PostsController extends Phalcon\Mvc\Controller
{
        public function showAction()
        {
               return array('posts' => Posts::find());
        }
}

Is this version is worse? ;)

phalcon commented 11 years ago

I'm going to start working on a single level of ViewModels first and see how things go.

Secondly, returning values from the actions to change the View behavior, I don't know..., it's simpler but if a developer wants to return those values to be used as json/xml output, or hmvc responses, the code/bevahior becomes less clear

https://github.com/nikolasha/PhalconTest/blob/master/app/source/Controller/ViewModelController.php#L25 https://github.com/nikolasha/PhalconTest/blob/master/app/source/Controller/ViewModelController.php#L47

nikolasha commented 11 years ago

Good news ;)

What you mean by the "single level of ViewModels"?

On the second, I do not understand what the problem is. Could you give an example of why the behavior becomes less obvious (two options - the current behavior and a variant with return a value)?

The task of the controller (or rather the action of controller) is prepare the data for the view. It is on the contrary, very clearly and ideologically correct, that the action takes the input parameters as method arguments and returns the prepared data. Determination in what format the data should be sent, in general, is not a task of the controller, it is a task of the View component.

For example, like this: http://framework.zend.com/manual/2.1/en/modules/zend.view.quick-start.html#creating-and-registering-alternate-rendering-and-response-strategies

I think you have to necessarily implement the ability to return data from the action. After all, who does not want just will not use this feature. About return false; I agree - it's not obvious, apparently, from this can refuse.

roman-kulish commented 11 years ago

:+1:

sergeyklay commented 7 years ago

I'm closing this issue due to the lack of any reaction. I'll open it again if the need arises

emansom commented 6 years ago

I think the original poster meant Phalcon to implement something along the likes of spatie/laravel-view-models.

The rundown of it is that it makes your controller specific logic for views lighter. Imagine the scenario of having a controller with a unique dynamic form with custom ACL logic; that would make your controller "fat". ViewModels migitate this.

phalcon / cphalcon

[NFR] Implement ViewModels #476