Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Refactoring ActionView::Partials

#881

I saw some potential refactorings for ActionView::Partials to remove some duplication and clean things up. Attached is a patch.

Here's an overview of what I did.

  • render_partial and render_partial_collection now take option hashes to simplify their argument list.
  • render_partial is the gateway to all partial rendering, this simplifies the Base render methods
  • split up rendering into multiple methods to handle the various object types: array, form, string, and other objects. These methods call each other to stay dry.
  • the counter is now internally stored in local_assigns as "collection_counter" to simplify rendering of collections. I'm not entirely happy with moving this logic into Renderable#compile, but I couldn't think of a cleaner solution.

Let me know what you think.

Reported by Ryan Bates · August 22nd, 2008 @ 01:04 AM

State: wontfix
Milestone: none
Assigned to: josh josh
Importance: none

Activity

  1. josh
    josh
    • State changed from new to wontfix
    • Assigned user set to josh
    • Milestone cleared.

    I don't like

    • I'm against adding more private methods to the view context. It would be better to push that logic into RenderablePartial and break it up if possible.
    • The Renderable#compile! method should have no logic concerning partials. I'd override compile! in RenderablePartial to add extra behavior.
    • Why do you need to change the assertion in test_render_partial_collection_without_as?

    I do like

    • Moving useful logic (from ActionController) into ActionView.
    • The idea of cleaning up Partial logic in general ;)

    August 22nd, 2008 @ 01:52 AM

  2. josh
    josh

    Took some of your ideas and cleaned up a few others things. Gave you the cred though.

    1129a24

    August 22nd, 2008 @ 03:05 AM

  3. Ryan Bates
    Ryan Bates

    Cool, thanks for doing that.

    August 22nd, 2008 @ 05:32 AM