jejacks0n / navigasmic

Navigasmic: Semantic navigation for Rails using simple view level or configuration definitions.
322 stars 29 forks source link

Paths (as hash) with different controllers but the same action name should not be highlighted? #42

Open XA21X opened 10 years ago

XA21X commented 10 years ago

The spec I wrote shows my interpretation of the expected behaviour of Item.highlights_on?. This is how the method is used when the :highlights_on option has not been set when Navigasmic::Item.new is called with the link specified as a hash.

I do not know whether my interpretation is correct but in aefed7c where this behaviour was previously modified, the change in line 38 (hash matching using OR logic instead of AND) of item.rb seems unnecessary for the implementation of the multi-controller feature (because they are in separate hashes).

I discovered this issue while I was using navigasmic to generate links to _signin and _signup of the Devise authentication gem. The link _signin would be highlighted even though I was on the _signup page.

Related to this, it seems like it would be useful in the future to add an optional parameter to semantic_navigation to specify the default behaviour of Item.highlights_on? so that for example, a navigation menu in the top bar could highlight on the same controller but a side menu would highlight only when the action also matches.

shekibobo commented 10 years ago

Re: aefed7c, my main goal was to keep the logic that was originally there, which was attempting to detect any of the expected rules. However, the only case I was testing was for the controller key in multiple hashes. I don't think I considered that hashes would contain other keys to match on. This looks like a good change to me. Thanks for picking that up.

mileszim commented 10 years ago

+1