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

Allow outer partial access when capturing #49

Merged
merged 1 commit into from
Jul 29, 2022

Conversation

kaspth
Copy link
Contributor

@kaspth kaspth commented Jul 29, 2022

Due to rendering happening in the same ActionView::Base instance,
the @partial instance variable was effectively shared between all
render calls.

As such, doing this:

render "card" do |cp|
  cp.content_for :title, partial.content_for(:title)
end

…wouldn't work because partial would point to the inner cp.

This commit fixes it so we no longer shadow the partial method.

Fixes #38

@kaspth
Copy link
Contributor Author

kaspth commented Jul 29, 2022

Got back to check more on this, but looks like it's causing some empty content areas. I'll fix.

Due to rendering happening in the same `ActionView::Base` instance,
the `@partial` instance variable was effectively shared between all
render calls.

As such, doing this:

```ruby
render "card" do |cp|
  cp.content_for :title, partial.content_for(:title)
end
```

…wouldn't work because `partial` would point to the inner `cp`.

This commit fixes it so we no longer shadow the partial method.
@kaspth kaspth force-pushed the make-previous-partial-accessible branch from e8cb828 to c4891fb Compare July 29, 2022 21:30
@kaspth
Copy link
Contributor Author

kaspth commented Jul 29, 2022

Ok, I replaced the previous "fix" with a stack based approach that's more reliable.

With the previous version of the fix rendering creative_concepts#show would render themes/light/_page.html.erb, which wouldn't properly reset to the outer partial after a render call. Like this:

<%= render 'account/shared/title' do |cp| %>
  <% cp.content_for :title, partial.content_for(:title) %>
  <% cp.content_for :actions, partial.content_for(:actions) %>
<% end %>

<%= render 'account/shared/notices' %>

<div class="space-y-8 py-4 xl:py-8 xl:px-8">
  <%# ⚠️ After the `render 'account/shared/title' render, `partial` would really point to `cp` even though it shouldn't %>
  <%= partial.yield :body %>
</div>

The stack based approach fixes this.

@andrewculver andrewculver merged commit 6ebd030 into main Jul 29, 2022
@andrewculver andrewculver deleted the make-previous-partial-accessible branch July 29, 2022 22:53
kaspth added a commit that referenced this pull request Sep 6, 2022
* main:
  Ignore /tmp if you're running BT in tmp/starter like CI does
  Fix some CHANGELOG issues and refit some examples + words
  Skip needless CI steps (#60)
  We don't need to depend on sqlite3 that was for an earlier version of the ViewComponent integration
  Fix #54 to use new section yield syntax
  Mixing `yield` call styles (#54)
  Expose Sections as an alternative to `content_for` (#57)
  Introduce Capybara to the test suite (#55)
  Add `content_from` to let partials relay contents (#53)
  v0.1.9
  v0.1.8
  Document release 🎉 (#52)
  Rename `Partial#output_buffer` to `Partial#yield` (#41)
  Allow outer partial access when capturing (#49)
  Fix accessing `partial` before rendering leaks state (#47)
  Use load hooks for monkey_patch loading (#48)
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.

Can't remove yield p = np from a specific partial.
2 participants