Skip to content

Conversation

rustamwin
Copy link
Member

@rustamwin rustamwin commented Apr 20, 2023

Q A
Is bugfix? ✔️/❌
New feature?
Breaks BC? ✔️
Fixed issues comma-separated list of tickets # fixed by the PR, if any

⚠️ Waiting yiisoft/view#226

@rustamwin rustamwin requested a review from a team April 20, 2023 08:01
@rustamwin rustamwin added type:docs Documentation status:code review The pull request needs review. labels Apr 20, 2023
Copy link
Member

@vjik vjik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be improve TemplateRendererInterface from yiisoft/view. For example:

interface TemplateRendererInterface
{
    public function render(string $templateFile, array $parameters, TemplateContext $templateContext): string;
}

final class TemplateContext {
  public function __construct(
    private ViewInterface $view,
    private string $template,
  ) {
  }
}

It's allow send additional information from view to template renderer. Also we can normalize template path in view instead of TemplateRendererInterface implementation.

@vjik vjik mentioned this pull request Apr 20, 2023
@codecov
Copy link

codecov bot commented Jun 24, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (bce1991) 100.00% compared to head (83d0c31) 100.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##              master       #61   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
+ Complexity         6         4    -2     
===========================================
  Files              2         1    -1     
  Lines             28        18   -10     
===========================================
- Hits              28        18   -10     
Impacted Files Coverage Δ
src/TemplateRenderer.php 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

# Conflicts:
#	README.md
#	composer.json
#	src/ViewRenderer.php
#	tests/TemplateRendererTest.php
@rustamwin rustamwin requested a review from a team October 7, 2025 05:48
@samdark
Copy link
Member

samdark commented Oct 7, 2025

Please fix failing static analysis.

@samdark samdark requested a review from vjik October 7, 2025 07:09
* TemplateRenderer allows using Twig with a View service.
*/
final class ViewRenderer implements TemplateRendererInterface
final class TemplateRenderer implements TemplateRendererInterface
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be call it "TwigTemplateRenderer"? What do you think?

## 2.1.1 under development

- Chg #61: Rename `ViewRenderer` to more understandable `TemplateRenderer` (@rustamwin)
- Chg #61: Remove `YiiTwigExtension` (@rustamwin)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create UPGRADE.md also with instructions to upgrade. These changes breaks BC.

* YiiTwigExtension adds additional functionality to the Twig engine.
*/
final class YiiTwigExtension extends AbstractExtension
final class SimpleExtension extends AbstractExtension
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Why remove YiiTwigExtension ?

  2. If remove, why does it need in tests?


echo $environment->render($file, array_merge($parameters, ['this' => $view]));
};
$templateFile = str_replace(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if $template placed not in base path?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:code review The pull request needs review. type:docs Documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants