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 component inline rendering from controller #329

Merged
merged 1 commit into from
Aug 31, 2015

Conversation

garbles
Copy link
Contributor

@garbles garbles commented Jul 31, 2015

Adds a component option to the controller render method so that you can render a component in place of yield instead of a view.

For example,

class TodoController < ApplicationController
  def index
    render component: 'TodoList', props: { todos: [{...}, {...}] }
  end
end

I had done something similar to this this before here, but there were problems with it.

@garbles garbles force-pushed the add-component-renderer branch 2 times, most recently from 3086ab5 to c5da4b2 Compare July 31, 2015 23:27
@garbles garbles changed the title Add component renderer to render a component instead of a view Allow component inline rendering from controller Jul 31, 2015
@garbles garbles force-pushed the add-component-renderer branch from c5da4b2 to a3c1d7f Compare July 31, 2015 23:37
@borisrorsvort
Copy link
Contributor

👍

@borisrorsvort
Copy link
Contributor

@rmosolgo This is cool :) What is it missing to go on?

@garbles
Copy link
Contributor Author

garbles commented Aug 21, 2015

@rmosolgo @zpao - wondering what else needs to be done here?

@zpao
Copy link
Member

zpao commented Aug 22, 2015

I think it looks cool! This is @rmosolgo's show though so I'll let him take care of it.

@@ -0,0 +1,18 @@
class React::ControllerRenderer
Copy link
Member

Choose a reason for hiding this comment

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

Can this be named React::Rails::ControllerRenderer? That would match the file path (and seems like the right name for it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops. Yup you're right. 👍

@rmosolgo
Copy link
Member

Sorry I'm behind, thanks for the nudge!

Wow, this implementation is really nice. I never knew about the Renderers API before. This seems like a great fit.

I had a couple of questions I left in line. Besides that, could you add a bit to the readme?

@borisrorsvort
Copy link
Contributor

@rmosolgo thanks for the review. Hope @garbles have time to get this right 😄 Can't wait 😛

@garbles garbles force-pushed the add-component-renderer branch from a3c1d7f to ca644a0 Compare August 31, 2015 15:52
@garbles
Copy link
Contributor Author

garbles commented Aug 31, 2015

@rmosolgo @borisrorsvort I believe that the issues when the PR have been resolved. 👪

@rmosolgo
Copy link
Member

💸 🎊 thanks!

include ActionView::Helpers::TagHelper
include ActionView::Helpers::TextHelper

attr_accessor :output_buffer
Copy link
Member

Choose a reason for hiding this comment

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

Btw @garbles these few calls (include ActionView::... + attr_accessor :output_buffer), is this what's required to make your custom object work with view rendering?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct

Copy link
Member

Choose a reason for hiding this comment

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

cool, I was trying to refactor the view helper a while back and got scared off by how things kept not working, maybe i'll give it another try

@garbles
Copy link
Contributor Author

garbles commented Aug 31, 2015

@rmosolgo Should I add something to the README?

@rmosolgo
Copy link
Member

Yeah, that would be good. Can you include a note like (coming in 1.3.0) or something? Some way that will signal to people that it's on master but not on Rubygems yet?

@garbles
Copy link
Contributor Author

garbles commented Aug 31, 2015

@rmosolgo cool cool. I'll write that up tonight. 🎉

@borisrorsvort
Copy link
Contributor

Giphy

rmosolgo pushed a commit that referenced this pull request Aug 31, 2015
Allow component inline rendering from controller
@rmosolgo rmosolgo merged commit a80805c into reactjs:master Aug 31, 2015
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.

5 participants