Add parameters injections to ViewRenderer + minor refactoring #122

vjik · 2020-08-08T18:59:28Z

Q	A
Is bugfix?	❌
New feature?	✔️
Breaks BC?	❌
Fixed issues	#123

Move adding of "csrf" parameters to external class CsrfViewInjection. This change will allow without changing ViewRenderer class add any parameters to view or layout.
Add class ApplicationViewInjection to add params, meta- and link-tags to views.
Method getName() make static and added cache by object class name.

Idea of next steps:

Move ViewRenderer to package (separately or yiisoft/view or yiisoft/web)
Move CsrfViewInjection to package (separately or yiisoft/view or yiisoft/web)
ApplicationViewInjection use in App for add parameters, meta and link tags to views.

Example of config:

'viewRenderer' => [
	'viewBasePath' => '@views',
	'layout' => '@views/layout/main',
	'injections' => [
		Reference::to(CsrfViewInjection::class),
		Reference::to(ApplicationViewInjection::class),
	],
],

Example of ApplicationViewInjection:

class ApplicationViewInjection implements
    LayoutParamsInjectionInterface,
    MetaTagsInjectionInterface,
    LinkTagsInjectionInterface
{

    private User $user;
    private UrlMatcherInterface $urlMatcher;

    public function __construct(
        User $user,
        UrlMatcherInterface $urlMatcher
    ) {
        $this->user = $user;
        $this->urlMatcher = $urlMatcher;
    }

    public function getLayoutParams(): array
    {
        return [
            'user' => $this->user->getIdentity(),
            'currentUrl' => (string)$this->urlMatcher->getLastMatchedRequest()->getUri(),
            'brandLabel' => 'Yii Demo',
        ];
    }

    public function getMetaTags(): array
    {
        return [
            [
                '__key' => 'generator',
                'name' => 'generator',
                'value' => 'Yii',
            ],
        ];
    }

    public function getLinkTags(): array
    {
        return [
            [
                '__key' => 'favicon',
                'name' => 'icon',
                'value' => 'favicon.ico',
            ],
        ];
    }
}

src/ViewRenderer/ViewRenderer.php

samdark · 2020-08-09T19:52:22Z

That looks overcomplicated for the problem it solves.

…ections, withLayoutInjections to ViewRenderer.

vjik · 2020-08-16T06:27:10Z

Made it a little simpler (merge contentInjections and layoutInjections), and create ApplicationInjection — this is convenient for add parameters, meta and link tags to App views.

src/ViewRenderer/CsrfViewInjection.php

src/ViewRenderer/ViewRenderer.php

src/ViewRenderer/ContentParamsInjectionInterface.php

src/ViewRenderer/ViewRenderer.php

samdark

I like that layout parameters are separated from content parameters. Still, I think that overall it can be simpler.

src/Contact/ContactController.php

samdark · 2020-08-19T13:08:16Z

src/ViewRenderer/ApplicationViewInjection.php

+    public function getLayoutParams(): array
+    {
+        return [
+            'user' => $this->user->getIdentity(),


https://github.com/yiisoft/app/blob/master/config/params.php#L83

In defaultParameters set to both: content and layout.
If need only for layout defaultParameters not suitable.

I've meant that such configuration is way more exposed to end user and simpler than modifying ApplicationViewInjection itself. It could be generalized and made configurable.

samdark · 2020-08-19T13:09:10Z

src/ViewRenderer/ApplicationViewInjection.php

+    {
+        return [
+            [
+                '__key' => 'favicon',


That's static. It's better to have it in HTML directly.

I didn't understand... Return HTML, for example <link rel="icon" type="image/png" href="/myicon.png"> ?

I mean there's no reason to inject it. It could be just a line of HTML in layout like https://github.com/yiisoft/yii-demo/blob/master/views/layout/main.php#L32

samdark · 2020-08-19T13:09:17Z

src/ViewRenderer/ApplicationViewInjection.php

+    {
+        return [
+            [
+                '__key' => 'generator',


That's static. It's better to have it in HTML directly.

src/ViewRenderer/ViewRenderer.php

samdark · 2020-08-19T13:13:58Z

src/ViewRenderer/ViewRenderer.php

 use Yiisoft\Strings\Inflector;
 use Yiisoft\View\ViewContextInterface;
 use Yiisoft\View\WebView;
 use Yiisoft\DataResponse\DataResponseFactoryInterface;
-use Yiisoft\Yii\Web\Middleware\Csrf;
-use Yiisoft\Yii\Web\User\User;

 final class ViewRenderer implements ViewContextInterface


Can part of these changes such as having defaults for parameters / layout be applied to yiisoft/view package directly?

Also do you plan to move this class somewhere out of the application?

Also do you plan to move this class somewhere out of the application?

https://github.com/yiisoft/yii-view

Can part of these changes such as having defaults for parameters / layout be applied to yiisoft/view package directly?

This can be partially done. For link and meta tags.
For content/layout params this it's impossible: in yiisoft/view there is no division into content and layout.

Let's add default link and meta-tags to yiisoft/view.

vjik · 2020-08-19T19:51:47Z

Moved ViewRenderer and interfaces to yiisoft/yii-view.

Where to move CsrfViewInjection:

to yiisoft/yii-web (add dependences yii-view, data-response, view, files, html, json);
to separately package.

?

samdark · 2020-08-20T11:44:26Z

Definitely no.
Likely it should stay in the application (this package).

vjik · 2020-08-20T11:51:33Z

Likely it should stay in the application (this package).

But CsrfViewInjection doesn't require changes to be used in any applications.

Advantages separately package:

reuse in any apps
default config for container (common.php):

CsrfViewInjectionInterface::class => CsrfViewInjection::class,

Maybe create package, for example, yiisoft/yii-csrf-view-injection or yiisoft/yii-csrf and additional move yii-web/src/Middleware/Csrf.php to him?

samdark · 2020-08-20T12:34:33Z

How about having it in yii-view?

vjik · 2020-08-20T12:46:39Z

How about having it in yii-view?

This added dependences for yii-view from router and yii-web. It's ok?

samdark · 2020-08-20T12:59:48Z

Yes since it's yii- packages are framework specific.

vjik · 2020-08-20T13:06:11Z

Ok, I make it.

samdark · 2020-09-06T22:31:44Z

Should be adjusted since https://github.com/yiisoft/csrf is there now.

vjik · 2020-09-08T14:21:57Z

Should be adjusted since https://github.com/yiisoft/csrf is there now.

See yiisoft/csrf#2

…erer # Conflicts: # config/common.php # src/Blog/Post/PostController.php # src/ViewRenderer/ViewRenderer.php

vjik · 2020-09-14T20:22:30Z

Done...
Before merge need merge:

TiaNex-Com · 2020-09-16T00:48:28Z

please merge these functions to one,thanks

/**
* @param object $injection
* @return self
*/

public function withAddedInjections(array $injections): self
{
$new = clone $this;
$new->injections = array_merge($this->injections, $injections);
return $new;
}

/**
 * @param object $injection
 * @return self
 */
public function withAddedInjection($injection): self
{
    return $this->withAddedInjections([$injection]);
}

/**
 * @param object[] $injections
 * @return self
 */
public function withInjections(array $injections): self
{
    $new = clone $this;
    $new->injections = $injections;
    return $new;
}

vjik · 2020-09-16T05:58:19Z

please merge these functions to one,thanks

This is different functions.

withInjections replace current injections to new injections.
withAddedInjections merge current injections with new injections.

yiiliveext reviewed Aug 8, 2020

View reviewed changes

src/ViewRenderer/ViewRenderer.php Outdated Show resolved Hide resolved

samdark added the status:under discussion label Aug 9, 2020

vjik added 10 commits August 16, 2020 08:45

Add parameters injections in ViewRenderer.

47c0cb7

Minor improve findLayoutFile method in ViewRenderer.

0956c0d

Refactoring method getName in ViewRenderer.

6fb446d

Add ViewRenderer params.

1002f1f

Remove use ContainerInterface in ViewRenderer.

780a4b3

Remove withConfig() method from InjectionInterface.

02021a2

Add methods addContentInjections, withContentInjections, addLayoutInj…

62a8e87

…ections, withLayoutInjections to ViewRenderer.

Add methods getMetaTags() and getLinkTags to InjectionInterface.

88f93d9

Merge contentInjections and layoutInjections.

c1259d2

Add ApplicationInjection.

a3aa9f3

vjik force-pushed the view-renderer branch from 1704d33 to a3aa9f3 Compare August 16, 2020 06:11

Add example of meta and link tags to ApplicationInjection.

85578b6

vjik added 2 commits August 16, 2020 10:55

Split private method inject() to contentInject() and layoutInject().

c58402f

Split InjectionInterface to separately interface for each type.

d872bd0

xepozz requested a review from a team August 16, 2020 08:44

xepozz requested changes Aug 16, 2020

View reviewed changes

Minor refactoring.

5e54115

yiiliveext reviewed Aug 16, 2020

View reviewed changes

src/ViewRenderer/ViewRenderer.php Outdated Show resolved Hide resolved

Refactoring ViewRenderer.

61ab00d

rustamwin requested changes Aug 16, 2020

View reviewed changes

src/ViewRenderer/ViewRenderer.php Outdated Show resolved Hide resolved

src/ViewRenderer/ViewRenderer.php Outdated Show resolved Hide resolved

vjik added 3 commits August 16, 2020 14:07

Code style fix.

ec03ffb

Move config injections from params.php to web.php.

5007096

Rename addInjections to withAddedInjections.

7811c73

samdark added the status:code review The pull request needs review. label Aug 17, 2020

vjik added 2 commits August 18, 2020 21:43

Replace isset to array_key_exists in getName().

e447fb7

Add phpdoc to interfaces.

d564fca

Minor fix in phpdoc.

e85a1dc

samdark reviewed Aug 19, 2020

View reviewed changes

vjik added 2 commits August 19, 2020 17:07

Make static method getName() is dynamic.

7608fbc

Add withCsrf().

0884161

samdark added status:under development Someone is working on a pull request. and removed status:code review The pull request needs review. labels Sep 6, 2020

vjik added 3 commits September 10, 2020 21:48

Merge remote-tracking branch 'remotes/upstream/master' into view-rend…

ad098b9

…erer # Conflicts: # config/common.php # src/Blog/Post/PostController.php # src/ViewRenderer/ViewRenderer.php

Use session from yiisoft/session.

e289acb

Use yiisoft/yii-view.

be5f6b7

samdark approved these changes Sep 14, 2020

View reviewed changes

samdark merged commit 07b88b6 into yiisoft:master Sep 14, 2020

vjik deleted the view-renderer branch September 15, 2020 15:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add parameters injections to ViewRenderer + minor refactoring #122

Add parameters injections to ViewRenderer + minor refactoring #122

vjik commented Aug 8, 2020 •

edited

Loading

samdark commented Aug 9, 2020

vjik commented Aug 16, 2020

samdark left a comment

samdark Aug 19, 2020

vjik Aug 19, 2020

samdark Aug 19, 2020

samdark Aug 19, 2020

vjik Aug 19, 2020 •

edited

Loading

samdark Aug 19, 2020

samdark Aug 19, 2020

samdark Aug 19, 2020

samdark Aug 19, 2020

yiiliveext Aug 19, 2020

vjik Aug 19, 2020

samdark Aug 19, 2020

vjik commented Aug 19, 2020

samdark commented Aug 20, 2020

vjik commented Aug 20, 2020 •

edited

Loading

samdark commented Aug 20, 2020

vjik commented Aug 20, 2020

samdark commented Aug 20, 2020

vjik commented Aug 20, 2020

samdark commented Sep 6, 2020

vjik commented Sep 8, 2020

vjik commented Sep 14, 2020

TiaNex-Com commented Sep 16, 2020 •

edited

Loading

vjik commented Sep 16, 2020

Add parameters injections to ViewRenderer + minor refactoring #122

Add parameters injections to ViewRenderer + minor refactoring #122

Conversation

vjik commented Aug 8, 2020 • edited Loading

samdark commented Aug 9, 2020

vjik commented Aug 16, 2020

samdark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vjik Aug 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vjik commented Aug 19, 2020

samdark commented Aug 20, 2020

vjik commented Aug 20, 2020 • edited Loading

samdark commented Aug 20, 2020

vjik commented Aug 20, 2020

samdark commented Aug 20, 2020

vjik commented Aug 20, 2020

samdark commented Sep 6, 2020

vjik commented Sep 8, 2020

vjik commented Sep 14, 2020

TiaNex-Com commented Sep 16, 2020 • edited Loading

vjik commented Sep 16, 2020

vjik commented Aug 8, 2020 •

edited

Loading

vjik Aug 19, 2020 •

edited

Loading

vjik commented Aug 20, 2020 •

edited

Loading

TiaNex-Com commented Sep 16, 2020 •

edited

Loading