bh icon indicating copy to clipboard operation
bh copied to clipboard

added no_container option to link_to helper

Open Incanus3 opened this issue 10 years ago • 13 comments

this way, one can manually create (and customize) the surrounding li container inside nav or dropdown and tell link_to not to create it

Incanus3 avatar Jan 22 '15 20:01 Incanus3

Coverage Status

Coverage decreased (-0.05%) to 99.95% when pulling b476ab4c6c1cac19bad3002889dda31cbd4c4d0a on Incanus3:master into f708ad029f8d7dc821a37a452359d705989187c0 on Fullscreen:master.

coveralls avatar Jan 22 '15 20:01 coveralls

Coverage Status

Coverage remained the same at 100.0% when pulling 5c1d6b55cdfd56fea7ed6d5cc40f893627fd9f12 on Incanus3:master into f708ad029f8d7dc821a37a452359d705989187c0 on Fullscreen:master.

coveralls avatar Jan 22 '15 21:01 coveralls

Btw interesting thing the

expect(:link_to).to generate html

Confused me for a moment, before I realized it's a custom matcher.

Incanus3 avatar Jan 22 '15 22:01 Incanus3

@Incanus3 before proceeding with this PR, could you show me a real example where you need this kind of customization? Thanks!

claudiofullscreen avatar Feb 02 '15 23:02 claudiofullscreen

Also… I think this option should be on the wrapping nav selector, so you don't have to add it to each link_to.

claudiofullscreen avatar Feb 03 '15 00:02 claudiofullscreen

= nav do
  - categories.each do |category|
    %li{ class: active_if(selected_category == category) }
      = link_to category.capitalize, events_path(category: category), no_container: true

(active_if just returns 'active' if given true, '' otherwise)

and you're right, the option should probably be on nav, but the container is being added in the link_to helper, so it seemed like the easiest thing to do.

Incanus3 avatar Feb 07 '15 12:02 Incanus3

@Incanus3 Did you know that bh already sets the active class on the wrapping <li> if the link matches the current page? See http://fullscreen.github.io/bh/#links

claudiofullscreen avatar Feb 07 '15 15:02 claudiofullscreen

I do, but that's not flexible enough for my (and presumably other's) needs. The problem here is (among others), that when I first come to this page with category tabs, there's no category param, so the first tab is not highlighted. I cound of course add the param there, but that would mean that every link to that page needs to add the category param for it to be correctly highlighted.

Incanus3 avatar Feb 07 '15 20:02 Incanus3

If I understand correctly, your nav links are something like

  • /events?category=foo
  • /events?category=bar
  • /events?category=boo

and you would live one of the links to be active not because it is exactly the current page (i.e., because your browser URL is exactly /events?category=foo), but because somewhere else in the code you set a variable called selected_category to one of those values.

For instance, if you are in the page /events?category=foo and you have set your selected_category variable to bar, then you would want the second link to be active, not the first one. Is that right?

claudiofullscreen avatar Feb 08 '15 19:02 claudiofullscreen

ad 1st paragraph: exactly. ad 2nd one: I don't override the url param by setting selected_category elsewhere, but I may set selected_category, but keep the url /events

Incanus3 avatar Feb 12 '15 18:02 Incanus3

also, I'd like the first tab to be default, but don't want every link_to to pass the first category, so if the param is missing entirely, I set selected_category to first category, but still need the tab to be highlighted.

Incanus3 avatar Feb 12 '15 18:02 Incanus3

Hello @Incanus3 – I think the case you are making is very custom.

You want to set the active class to a link that does not exactly match the current URL.

In the most general case, that is an "unexpected" behavior… the active class is meant to identify the current page.

You can definitely do that in your code, but I don't see this as something worth generalizing to the bh gem.

The solution for your case is quite simple: do not use the nav helper from Bh in that specific context, but simply write out the <ul nav ..><li>..</li><li>..</li></nav> elements the way you want them. Then, you will still be able to use link_to and the links won't be wrapped in a container.

Makes sense?

claudiofullscreen avatar Feb 13 '15 19:02 claudiofullscreen

Well, I'd agree that wanting the active class on a link that's realy different from the current page would be "unexpected". But I think that the latter case I mentioned (making the first tab default, thus '/events' ~= '/events?category=first_category') is quite common and in case of '/events' I need to highlight the first tab manually.

Also, I think this option is not that much added code for greater flexibility. Of course, your suggested solution would work, but the reason I use bh is exactly so I don't need to do this. It's not just that I need to write a few more characters, but the view is much more readable when using the helper.

Incanus3 avatar Feb 13 '15 19:02 Incanus3