Jacob, Your patch adding the level tag in <r:navigation> is quiet tidy. In my own Swiss Army tag library extension which I dump into nearly every Radiant site I put up I have a similar tag <r:navigation_level level="2">. Your version is cleaner. My tag gets use in combined with an <r:level /> tag which returns the current depth/level in the tree.
Speaking to the bigger picture of core changes to the <r:navigation> tag I think that Sean is right to suggest a more drastic refactoring of the tag is in order. If I understand right Sean's been playing with some of these more drastic revisions himself. Sean, do I have you wrong? Can we have a peek? Loren On Jun 1, 2007, at 9:30 AM, Jacob Burkhart wrote: > Thanks Sean. > > Looks like posting on the mailing list gets a much faster reply that > posting on Trac. > > So then a follow-up question would be, what's wrong with these > patches, why havn't they been replied-to? > http://dev.radiantcms.org/radiant/ticket/504 > http://dev.radiantcms.org/radiant/ticket/502 > > Jacob > > On 6/1/07, Sean Cribbs <[EMAIL PROTECTED]> wrote: >> We would love a patch, of course! This will also help in a future >> refactoring toward REST which is on the horizon. >> >> Sean >> >> Jacob Burkhart wrote: >>> Hey, >>> >>> I just read this post: >>> http://www.therailsway.com/2007/6/1/railsconf-recap-skinny- >>> controllers >>> >>> And it made me think immediately of the old >>> handle_new_or_edit_post in >>> page_controller >>> http://dev.radiantcms.org/radiant/browser/trunk/radiant/app/ >>> controllers/admin/page_controller.rb?rev=57 >>> >>> >>> But now I go and look at the latest version, and I see it's been >>> cleaned up a bit, and there a new AbstractModelController seperating >>> the logic a bit. >>> >>> http://dev.radiantcms.org/radiant/browser/trunk/radiant/app/ >>> controllers/admin/page_controller.rb >>> and >>> http://dev.radiantcms.org/radiant/browser/trunk/radiant/app/ >>> controllers/admin/abstract_model_controller.rb >>> >>> >>> But still... you have a lot of logic in the controllers. And a >>> lot of >>> that logic, if moved to the model, would be a lot easier to override >>> and modify in extensions. >>> >>> For instance, I'd like to see all the part-creating/arranging logic >>> moved into the Page model. >>> >>> 96 parts_to_update = {} >>> 97 (params[:part]||{}).each {|k,v| parts_to_update[v >>> [:name]] = v } >>> 98 >>> 99 parts_to_remove = [] >>> 100 @page.parts.each do |part| >>> 101 if(attrs = parts_to_update.delete(part.name)) >>> 102 part.attributes = part.attributes.merge(attrs) >>> 103 else >>> 104 parts_to_remove << part >>> 105 end >>> 106 end >>> 107 parts_to_update.values.each do |attrs| >>> 108 @page.parts.build(attrs) >>> 109 end >>> 110 if result = @page.save >>> 111 new_parts = @page.parts - parts_to_remove >>> 112 new_parts.each { |part| part.save } >>> 113 @page.parts = new_parts >>> 114 end >>> >>> So, my question to the core devs is... how open would you be to >>> patch >>> coming from me, that addresses some of these things. >>> >>> I must admit, my agenda is based on making my own extensions more >>> elegantly integrated with Radiant. But I'm hoping to appeal to any >>> skinny-controller philosophy-followers as well. >>> >>> thoughts? >>> >>> Jacob >>> _______________________________________________ >>> Radiant mailing list >>> Post: [email protected] >>> Search: http://radiantcms.org/mailing-list/search/ >>> Site: http://lists.radiantcms.org/mailman/listinfo/radiant >>> >>> >> >> _______________________________________________ >> Radiant mailing list >> Post: [email protected] >> Search: http://radiantcms.org/mailing-list/search/ >> Site: http://lists.radiantcms.org/mailman/listinfo/radiant >> > _______________________________________________ > Radiant mailing list > Post: [email protected] > Search: http://radiantcms.org/mailing-list/search/ > Site: http://lists.radiantcms.org/mailman/listinfo/radiant _______________________________________________ Radiant mailing list Post: [email protected] Search: http://radiantcms.org/mailing-list/search/ Site: http://lists.radiantcms.org/mailman/listinfo/radiant
