Skip to content
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

Layout Renderer Fix #2515

Merged
merged 2 commits into from
Jan 30, 2020

Conversation

najdanovicivan
Copy link
Contributor

Description

Another take on #2491. This is done in proper way with unit test added first then working on fixing the issue.

The View class does not reset this->sections after completing the rendering. If multiple views are rendered using the same section name those sections are concatenated and displayed multiple times.

The first idea was to reset section in the render method which was not appropriate as this method works if there is only one section but fails to render other section when multiple sections are used. This is because render method is recessively called to render section so rendering the first section will cause other sections not to be rendered at all.

A different approach is used here. Instead of reseting the section array completely each section is unset separately from sections array after rendering it.

I've included unit test which covers the #2491 issue but views still lack tests for some specific cases so this should be tested.

Untested area contains of testing views with layout which renders multiple sections. Currently we have only test layout which renders a single section.

Before merging this one some additional testing should be made.

Checklist:

  • Securely signed commits
  • Component(s) with PHPdocs
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@MGatner
Copy link
Member

MGatner commented Jan 28, 2020

I will give this a shot, thanks for the work!

Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

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

This looks good, and works for my project that the other did not. Approved!

@lonnieezell lonnieezell merged commit 02565d1 into codeigniter4:develop Jan 30, 2020
@najdanovicivan najdanovicivan deleted the layout-renderer-fix branch January 31, 2020 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants