codeplant / simple-navigation

A ruby gem for creating navigations (with multiple levels) for your Rails, Sinatra or Padrino applications. Render your navigation as html list, link list or breadcrumbs.
http://simple-navigation-demo.codeplant.ch/
MIT License
885 stars 136 forks source link

A more powerful name_generator #144

Closed simonc closed 10 years ago

simonc commented 10 years ago

I'm trying to use Font-Awesome in my menu and I'm using name_generator to do so but I think using the name of the menu entry doesn't really make sense since it can change depending of the language.

I'd like to submit a PR to provide access to self in this block. There are two ways to do so, one which is not retro-compatible, passing self instead of just @name. The second is more conservative, it's just passing self as second argument.

What do you think?

andi commented 10 years ago

So do I understand this correctly: You need something more "stable" than the name (e.g. the item key) in the name generator in order to decide which icon class to apply?

simonc commented 10 years ago

Yes, exactly. Maybe there is a cleaner way to do so I'm not aware of :) But if not that would be nice to be able to access any information on the item and let the user decide what to use.

simonc commented 10 years ago

Just to be clear, here is the code I use with a temporary patch applied:

navigation.name_generator = proc do |name, item|
  %Q(<i class="fa fa-#{NAV_ICONS[item.key]}"></i> #{name})
end
andi commented 10 years ago

You could just add the necessary common fa classes and then add the icon specific class through sass mixin:

navigation.name_generator = proc do |name, item|
  %Q(<i class="fa"></i> #{name})
end

This would generate something like this:

<ul>
  <li class='my_navi_key'><a..><i class="fa"></i>Navi 1</a></li>
</ul>

The you could extend through sass

ul li.my_navi_key i {
  @extend .fa-whatever-icon
}

etc...

But I think it would not do any harm to pass in the item (aka self) to the generator. But we should not break existing code, so pls add it as a second param and send PR.

simonc commented 10 years ago

This would work since I use the font-awesome-rails gem but for other cases it would indeed be useful :) I'll PR for it soon :) Thanks!

andi commented 10 years ago

PR update? :-)

simonc commented 10 years ago

Done :)

simonc commented 10 years ago

Do you want me to rebase this PR ?

andi commented 10 years ago

It can be merged, so no need to...

On Tue, Feb 25, 2014 at 3:53 PM, Simon Courtois notifications@github.comwrote:

Do you want me to rebase this PR ?

— Reply to this email directly or view it on GitHubhttps://github.com/codeplant/simple-navigation/pull/144#issuecomment-36015019 .