navbar: what's the "Rails Way"?

Hi all.

I've the following navbar on my site:

<div id="nav">
<ul>
<li id="current"><%= link_to "Home", home_url
%></li>
<li><%= link_to "Link 1", link1_url %></li>
<li><%= link_to "Link 2", link2_url %></li>
<li><%= link_to "Link 3", link3_url %></li>
<li><%= link_to "Link 4", link4_url %></li>
<li><%= link_to "Link 5", link5_url %></li>
</ul>
</div>

My question is: what's the best approach against id="current"? I mean:
I want to my current controller to be the current "selected" item.

I was thinking about use something like my.url/?current=item_name and
an helper. So:

#app/helpers/home_helper.rb
def output_li(name, current, &content)
output = "<li"
output = (name.to_upper == current.to_upper) ? ' id="current"' : ''
output = ">"

output = yield

output = "</li>"
end

#app/views/layouts
<div id="nav">
<ul>
<%= output_li "home", params[:current] { link_to "Home",
home_url(:current="home") } %>
<%= output_li "link1", params[:current] { link_to "Link
1", link1_url(:current="link1") } %>
<%= output_li "link2", params[:current] { link_to "Link
2", link2_url(:current="link2") } %>
<%= output_li "link3", params[:current] { link_to "Link
3", link3_url(:current="link3") } %>
<%= output_li "link4", params[:current] { link_to "Link
4", link4_url(:current="link4") } %>
<%= output_li "link5", params[:current] { link_to "Link
5", link5_url(:current="link5") } %>
</ul>
</div>

What about my solution? I believe that there is an best approach than
it, since I unfamiliar with "view logic".

Sorry for my poor English.

Best regards,

davi vidal

davividal@siscompar.com.br wrote:

I've the following navbar on my site:

<div id="nav">
        <ul>
                <li id="current"><%= link_to "Home", home_url
%></li>
                <li><%= link_to "Link 1", link1_url %></li>
                <li><%= link_to "Link 2", link2_url %></li>
                <li><%= link_to "Link 3", link3_url %></li>
                <li><%= link_to "Link 4", link4_url %></li>
                <li><%= link_to "Link 5", link5_url %></li>
        </ul> </div>

Start with link_to_unless_current. If you rewrite your solution using it, most of your logic might disappear inside.

Then, I'm going to nit-pick some common irritations in your solution, so even if you rewrite it we will all learn a little.

def output_li(name, current, &content)
  output = "<li"
  output = (name.to_upper == current.to_upper) ? ' id="current"' : ''
  output = ">"

  output = yield

  output = "</li>"
end

If you must generate HTML, use content_tag (or Builder::XmlMarkup).

        <%= output_li "home", params[:current] { link_to "Home",
home_url(:current="home") } %>

params is visible inside your helper, so you don't need to pass it in. Next, the {} block is passing to params[:current], which ignores it. You were missing a comma.

Finally, your solution duplications "link1" etc. Roll such things up into a loop.

Next, a chain of <%= x %><%= x %><%= x %> is really a single <%= x+x+x %>, so if you find yourself writing so many <%= %> next time you can often clean them up, too.

Finally, don't use an 'id' for a 'class'. Some CSS tools do that (and some CSS tool users do it!). IDs must not duplicate in a page, and you are always a refactor away from that!

Citando Phlip <phlip2005@gmail.com>:

I've the following navbar on my site:

<div id="nav">
        <ul>
                <li id="current"><%= link_to "Home", home_url
%></li>
                <li><%= link_to "Link 1", link1_url %></li>
                <li><%= link_to "Link 2", link2_url %></li>
                <li><%= link_to "Link 3", link3_url %></li>
                <li><%= link_to "Link 4", link4_url %></li>
                <li><%= link_to "Link 5", link5_url %></li>
        </ul>
</div>

Start with link_to_unless_current. If you rewrite your solution using it, most
of your logic might disappear inside.

Thank you very much for your response, Phlip. And thank you very much for the tip. It's VERY useful.

Then, I'm going to nit-pick some common irritations in your solution, so even if
you rewrite it we will all learn a little.

def output_li(name, current, &content)
  output = "<li"
  output = (name.to_upper == current.to_upper) ? ' id="current"' : ''
  output = ">"

  output = yield

  output = "</li>"
end

If you must generate HTML, use content_tag (or Builder::XmlMarkup).

Thanks again.

        <%= output_li "home", params[:current] { link_to "Home",
home_url(:current="home") } %>

params is visible inside your helper, so you don't need to pass it in. Next, the
{} block is passing to params[:current], which ignores it. You were missing a comma.

Yeah. Typo. Sorry.

Finally, your solution duplications "link1" etc. Roll such things up into a loop.

I was using "linkN" as sample.

So, here is what am I doing right now:

#app/views/layouts/post.html.erb
<div id="nav">
  <ul>
    <%= build_navbar %w(home, posts, articles, photos) %>
  </ul>
</div>

#app/helpers/home_helper.rb
module HomeHelper
   def build_navbar(items)
     items.each do |item|
       content_tag :li, :class => is_current?(item) do
         link_to_unless_current item.capitalize, item.singularize + "_url"( :current => item )
       end
     end
   end

   def is_current?(item)
     (item == params[:current]) ? "current" : nil
   end
end

I know that, by convention, methods ending by '?' returns true or false. But I think that it will increase readbility, right?

Sorry for my very poor English.

Thanks,

davi vidal

OK. My problem is solved. Thank you very much.

  What I'm doing, right now, is using an helper and content_tag.

#app/helpers/home_helper.rb
module HomeHelper
  def build_navbar(items)
    # If I reached the site right now, params[:current]
    # is null.
    params[:current] = "home" if params[:current].nil?

    menu=""

    items.each do |item|
      menu += content_tag :li, :class => is_current?(item) do
        link_to item, send( "#{item}_url", { :current => item } )
      end
    end

    return menu
  end

  # Methods ending by '?' returns either "true" or "false"
  # But using it here in "other" context increases
  # readbility.
  def is_current?(item)
    (item == params[:current]) ? "current" : nil
  end
end

#app/views/layouts/post.html.erb
[...]
<div id="nav">
  <ul>
    <%= build_navbar %w(home posts articles photos) %>
  </ul>
</div>
[...]

  Thank you very much.