Skip to content

[5.2] Blade push with layouts weird ordering#12808

Merged
taylorotwell merged 3 commits into
laravel:5.2from
mark86092:working-on-push-2
Mar 28, 2016
Merged

[5.2] Blade push with layouts weird ordering#12808
taylorotwell merged 3 commits into
laravel:5.2from
mark86092:working-on-push-2

Conversation

@mark86092

Copy link
Copy Markdown
Contributor

Modified version of #12769, this time, I separate the logic of @push and @section. So, only the push and stack part are modified.

Reason to separate push and section:

  1. In original method, they share the same compiling result startSection. If I keep it, I have to break the structure of internal $sections or break the behavior of appendSection
  2. There is no reason to use @push and @section with same name together. i.e.
@push('me') @endpush
@section('me') @endsection

is undefined. So, we can separate them.

@taylorotwell

Copy link
Copy Markdown
Member

Can you provide more unit tests to prove this fixes your issue?

$this->assertEquals('hi', $factory->yieldContent('foo'));
$factory->stopPush();
$this->assertEquals('hi', $factory->yieldPushContent('foo'));
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Since @push uses startPush, so this tests whether the startPush, stopPush, yieldPushContent works

@taylorotwell taylorotwell merged commit fd7b441 into laravel:5.2 Mar 28, 2016
@mark86092 mark86092 deleted the working-on-push-2 branch March 28, 2016 15:42
@taylorotwell

Copy link
Copy Markdown
Member

Reports of this breaking things: #12948

@GrahamCampbell

Copy link
Copy Markdown
Collaborator

Ping @mark86092.

@mark86092

Copy link
Copy Markdown
Contributor Author

@GrahamCampbell sorry, the time is my mid sleep night, I reply it ASAP when morning.

See #12948 below

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