On Tue, Feb 9, 2010 at 4:31 AM, Bigos <ruby.obj...@googlemail.com> wrote:
>
>
> On Feb 9, 9:24 am, Frederick Cheung <frederick.che...@gmail.com>
> wrote:
>> On Feb 9, 9:20 am, Bigos <ruby.obj...@googlemail.com> wrote:
>>
>>
>>
>> > So here are my questions:
>> > 1 Is it appropriate to put such code here?
>>
>> No. You don't really want any code in your view, other than the very
>> bare minimum. Sounds like you want to put a l lot of this in a view
>> helper
> Thanks for prompt reply, I don't want to put any more code than that.
> Just want a simple change of CSS style depending which page is being
> displayed at the moment.

Well you already have WAY too much code in the view, and that code
deals with stuff like urls/routing which really shouldn't be the
business of a view.

The way I approach the issue of highlighting the active tab (which it
looks like is what this code is trying to do, would be to have a
helper to generate the html, which uses either the controller name (if
there's a one-one correspondence between controllers and tabs) or a
current_tab instance variable set by the controller.  And the helper
should use path/url helpers rather than coding absolute uris.

So something like this.

First make sure that routes.rb has a sensible routing for the various
urls.  Just an example:

map.home '/', :controller => 'welcome', :action => 'show'
map.how_we_work '/how_we_work', :controller => 'welcome', :action =>
'our_process'
map.contact_us 'contact_us', :controller => 'contacts', :action => 'index'

Note that these route are just for example purposes, I'd probably do
it quite differently, but that's not relevant to the current problem.
In fact, the whole point is that the routing is something which might
well change over the life of the app, and that's one of the things
which makes view code like your starting point fragile.

Then in the controller(s) in the action methods which handle the
various end points

class WelcomeController < ApplicationController
  def show
      @current_tab = 'Home page'
  end

  def our_process
     @current_tab = 'How we work'
  end

end

class ContactsController < ApplicationController
  def index
     @current_tab = 'Contact Us'
  end
end

in a helper, probably application_helper.rb

def tab_link(tab_name, target)
   link_to(tab_name, target, :class => tab_name == @current_tab ?
'current_tab' : 'jtab')
end

def navigation_bar
   [
    ['Home page', home_path],
    ['How we work',how_we_work_path],
    ['Contact Us', contact_us_path ]
   ].map {|tab_name, target | tab_link(tab_name, target)}.join("\n")

In the view:


<%= navigation_bar %>

Again this is probably not exactly the code I'd come up with, but it's
close to what you started with and adheres much better to what the
responsibilities of routes, controllers, and views should have in a
Rails app. IMHO

-- 
Rick DeNatale

Blog: http://talklikeaduck.denhaven2.com/
Twitter: http://twitter.com/RickDeNatale
WWR: http://www.workingwithrails.com/person/9021-rick-denatale
LinkedIn: http://www.linkedin.com/in/rickdenatale

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Talk" group.
To post to this group, send email to rubyonrails-t...@googlegroups.com.
To unsubscribe from this group, send email to 
rubyonrails-talk+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/rubyonrails-talk?hl=en.

Reply via email to