fixed missing slots in layout

raffaelj commented 10 months ago

After the recent change to App->render(), I can't access the slots in the layout file anymore.

Usage:

layout file default.php:

<!DOCTYPE html>
<html lang="<?=$lang?>">
    <head>
        <title><?=$title?></title>
    </head>
    <body>
        <?=$content_for_layout?>
    </body>
</html>

view file page.php:

<h1><?=$title?></h1>
<?=$content?>

Calling render method:

$app->render('source:page.php with source:default.php', [
    'lang'    => 'en',
    'title'   => 'Cats',
    'content' => '<p>are cute</p>'
]);

aheinze commented 10 months ago

Hey 👋 The reason for the refactoring was to reduce the possibility of variable name collisions. But I see your point.

Can we introduce some context here? I suggest to make the slots available in a dedicated $viewSlots variable in the layout.

So the code would be:

$contents = $render($layout, ['viewSlots' => $slots, 'content_for_layout' => $contents]);

raffaelj commented 10 months ago

I suggest to make the slots available in a dedicated $viewSlots variable in the layout.

In this case I would call extract($viewSlots) at the top of all layout files, which defeats the purpose of adding the context in the first place. I could work with it, but it would make the template files ugly.

Right now, the layout files only know about $content_for_layout, $this and $setViewLayout (which has no effect in a layout).

My work flow in v1 was to add some variables to $this->app->viewvars, e. g. the current collection name and $lang, so all nested partials can access that information.

After dropping LimeExtra the functionality $this->app->viewvars was also dropped.

My (simplified) Controller now looks like this:

<?php

class Page extends \App\Controller\Base {

    public function index(string $permalink) {

        $model = 'pages';
        $lang  = 'de';

        $filter = [
            '_state'    => 1,
            'permalink' => $permalink,
        ];

        $item = $this->module('content')->item($model, $filter);

        if (!$item) $this->stop(404);

        // keep original item without language processing
        $_item = $item;

        // apply localized values
        $item = $this->app->helper('locales')->applyLocales($item, $lang);

        // add helper meta for marking up partial translated data
        $_lang_attrs = [];
        // skipped custom logic for simplicity

        // make meta variables available in all template files
        // Can't use `$viewvars` from LimeExtra in v1 anymore
        $this->app->on('app.render.view', function($template, &$slots) use($lang, $_lang_attrs, $_item, $model) {
            $slots['_lang']       = $lang;
            $slots['_lang_attrs'] = $_lang_attrs;
            $slots['_item']       = $_item;
            $slots['_model']      = $model;
        });

        // alternative way to access variables from all templates
        // `extract($this['viewvars']);` --> Indirect modification of overloaded element of Lime\App has no effect
        // `extract($this->retrieve('viewvars'));` --> works
        $this->app->set('viewvars', [
            'qwe' => 'asd',
            'zui' => 'hjk',
        ]);

        // view contains `$setViewLayout('layouts:default.php');`
        return $this->render('views:page.php', [
            ...$item
        ]);
    }

    protected function render(string $view, array $params = []): mixed {

        $view .= $this->layout ? " with {$this->layout}" : '';

        // some logic for caching output as static files

        return $this->app->render($view, $params);
    }

}

After thinking some more about it, I'm not sure, if I really need the slots available in the layout file. I actually want the $app->viewvars from LimeExtra back.

Also I want layout inheritance like in jekyll:

page.php has layout default.php
post.php has layout blog.php, which itself has layout default.php

Alternative syntax:

$this->render('views:page.php with layouts:default.php')
$this->render('views:post.php with layouts:blog.php with layouts:default.php')

This way Lime\App can be used as a render engine, that supports child theming by design via $app->path().

If this is not too much out of scope, I can write a prototype and send a new PR. Just let me know, what you think about it.

raffaelj commented 10 months ago

fixed via https://github.com/Cockpit-HQ/Cockpit/commit/451e4a98a9b13b1819fdd58366dd7ceb188d2a8a

Cockpit-HQ / Cockpit

fixed missing slots in layout #135