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 arguments to be passed to content areas #285 #323

Closed

Conversation

jonspalmer
Copy link
Contributor

@jonspalmer jonspalmer commented May 2, 2020

Summary

Alternative solution than #322 for passing arguments to content areas as requested in #285

Other Information

I'm not 100% convinced we need this complexity. I'd love more example use cases. But assuming an agreed need I think this is the way we'd do it

@jonspalmer
Copy link
Contributor Author

ping @andrewmcodes @rosendi @thomasbrus

@jonspalmer jonspalmer force-pushed the arguments_to_content_areas branch from 97d3e51 to b1bcb89 Compare May 2, 2020 14:40
assert_selector(".title strong", text: "Hello!")
assert_selector(".body", text: "Have a nice day.")
assert_selector(".footer", text: "Bye!")
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With this change "wrapping" the render arguments results in a stack overflow. Its a weird use case anyway and not one that I'm that bothered about supporting. Seems like anyone rendering the component can choose one mechanism or the other.

content = view_context.capture(&block)
define_singleton_method(area) do |*args|
block = ->() { content } unless block_given?
view_context.capture(*args, &block)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the key change. Rather than capturing the block immediately and storing the result in the attr_reader instance variable we redefine the method and capture the block when its called passing the arguments.

@joelhawksley
Copy link
Member

@jonspalmer thanks for putting this together. I vote that we let #325 play out before moving forward with any further changes to content areas ❤️

@jonspalmer
Copy link
Contributor Author

@joelhawksley Agreed. We're getting there on #325 I think the answer will be clear out of that

@joelhawksley
Copy link
Member

Closing as stale, feel free to reopen if you'd like ❤️

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.

2 participants