-
Notifications
You must be signed in to change notification settings - Fork 15
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
fix: view fragments #78
Conversation
/** | ||
* Whether we should display fragments tags or not. | ||
*/ | ||
protected function showFragments(bool $display = true): RendererInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not self
or static
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to follow the current convention used in the original View
class: https://github.com/codeigniter4/CodeIgniter4/blob/develop/system/View/View.php
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good. This is a notification in the IDE
Everything is fine for multi-fragments. After the request, you need to return the template and toasts from the flash for it. There was no data before PR, we need to review it again. Will the clarification of the work be indicated in the documentation?
|
$this->assertSame($expected, $result); | ||
} | ||
|
||
public function testHugeView(): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By adding a test for view(),
the result is not obvious - the layout inside the section. Is this a problem with the parent view()
?
public function testHugeViewFragment(): void
{
$result = view_fragment('huge/view', ['fragment_one']);
$expected = <<<'EOD'
Fragment one (from "huge/view")
Fragment one (from "huge/include")
Fragment one (from "huge/layout")
EOD;
$this->assertSame($expected, $result);
}
public function testHugeView(): void
{
$result = view('huge/view');
$expected = <<<'EOD'
View top
Fragment one (from "huge/view")
# include start in "huge/view"
Layout top
Section (from "huge/view")
Include top
Fragment one (from "huge/include")
Include bottom
Fragment one (from "huge/layout")
Layout bottom
# include end in "huge/view"
View bottom
EOD;
$this->assertSame($expected, $result);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, bug framework
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you place the content outside the section this is what you will get. I don't see any issues here.
/** | ||
* Whether we should display fragments tags or not. | ||
*/ | ||
protected function showFragments(bool $display = true): RendererInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good. This is a notification in the IDE
This PR fixes view fragments:
Closes #75