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

render widget: Views::Hello does not work as expected #14

Closed
leafo opened this issue Nov 20, 2014 · 8 comments
Closed

render widget: Views::Hello does not work as expected #14

leafo opened this issue Nov 20, 2014 · 8 comments

Comments

@leafo
Copy link
Contributor

leafo commented Nov 20, 2014

  • The delegate_object you use for the rendering_context is a controller instance instead of the view_context, so view helpers are not available (and other weird stuff happens)
  • If you have Erector installed alongside Fortitude, because you don't strip the :widget option, Erector tries to render the Fortitude widget somewhere deep within render_without_fortitude
  • Passing a class instead of an instance does not work (it should use view_context.assigns for needs by default)
  • After patching some of these things, I was unable it to make it actually work at all. It manipulates the options with the :text option correctly, but my layout still renders empty. (I noticed if I disabled the layout then the content would come through)

Lastly, I'm curious why you chose to do it the way you did, Erector does the following to register a :widget renderer:

require 'action_controller/metal/renderers'

ActionController.add_renderer :widget do |widget, options|
  # return result of render
end
@leafo
Copy link
Contributor Author

leafo commented Nov 20, 2014

Using add_renderer will also make render_to_string widget: Widget work correctly too.

@ageweke
Copy link
Owner

ageweke commented Nov 20, 2014

OK, I'm looking at this right now. Some of this stuff is just me not knowing about view_context or add_renderer, which I will now use. It's also test cases that I was missing. I imagine it will all be relatively quick to fix…stand by.

@leafo
Copy link
Contributor Author

leafo commented Nov 20, 2014

As a side note, I don't think the add_renderer approach adds the layout when passing layout: true. It never worked in Erector, so I'm not too concerned, but it would probably be nice to have.

@leafo
Copy link
Contributor Author

leafo commented Nov 20, 2014

view_context is equal to the template_handler argument that you receive in Fortitude::Rails::Renderer.render btw

@leafo
Copy link
Contributor Author

leafo commented Nov 20, 2014

add_renderer does not work well if two renderers of the same name are added. It appears to keep the first one only. I ended up patching Erector::Rails.render in my initializer so I could still render widgets even if Erector is loaded first: (this also handles passing a class as the widget options)

If you're planning on keeping erector/fortitude at same time support you can use or modify this.

module Erector
  module Rails
    class << self
      def render_with_fortitude(widget, view, local_assigns = {}, is_partial = false, options = {})
        if widget.is_a?(Class) && widget < ::Fortitude::Widget
          widget = widget.new widget.extract_needed_assigns_from(view.assigns)
        end

        if widget.is_a? ::Fortitude::Widget
          context = ::Fortitude::RenderingContext.new helpers_object: view

          widget.render_to(context)
          return context.output_buffer_holder.output_buffer.to_s
        end

        render_without_fortitude(widget, view, local_assigns, is_partial, options)
      end
      alias_method_chain :render, :fortitude
    end
  end
end

@ageweke
Copy link
Owner

ageweke commented Nov 20, 2014

OK, just added a whole bunch of specs to the branch ageweke/render_widget that, I think, handle all the cases you reported above. They all fail right now, but I'm going to work on making them pass, using add_renderer. They also cover the case of being able to use render :widget with both Erector and Fortitude widgets simultaneously, so I'm going to do my best to make that work correctly, too.

@ageweke
Copy link
Owner

ageweke commented Nov 20, 2014

(I have to take off for a few hours shortly, so the fixes will come later this evening, just as a heads-up.)

@ageweke
Copy link
Owner

ageweke commented Nov 20, 2014

OK, master now has fixes for all this stuff, and there are no issues that I know of remaining around this. I think I even took care of the difficult cases above — you now get layouts (unless you turn them off) using render :widget, and I monkeypatched ActionController.add_renderer so that even if Erector comes in to register its own handler for :widget, we re-register after to make sure both will still work. (It seems like ActionController is last-call-wins there.)

Assuming Travis is happy, I plan to cut 0.0.9 tomorrow with all these fixes.

@ageweke ageweke closed this as completed Nov 20, 2014
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

No branches or pull requests

2 participants