Blank url and highligh_on_subpath = true causes error.
gondalez opened this issue · 4 comments
Thanks for the neat solution :)
I have hit a small (but blocking) bug!
Repro steps
simple-navigation v4.0.3
mri 2.2.1p85
rails 4.2.3
- configure multiple groups of items whose parents have no URL (are just groupings)
- set
highlight_on_subpath
to true
Observations
- setting highlight_on_subpath to false stops the error, but using
highlights_on
is much less fun! - setting the url of the parent stops the error, but the parent items are of course links then
- setting the url of the second parent also stops the error
Example
# config/tasty_nav.rb
SimpleNavigation::Configuration.run do |navigation|
# this is important
navigation.highlight_on_subpath = true
navigation.items do |primary|
primary.item :first_id, 'first' do |sub_nav| # note no url argument
sub_nav.item :first_child_id, 'first child', 'some url'
end
primary.item :second_id, 'second' do |sub_nav| # no url argument here either
sub_nav.item :second_child_id, 'first child', 'some other url'
end
end
end
And render it like this
<%= render_navigation context: :tasty_nav, expand_all: true %>
(I tried disabling expand_all as well, but no effect)
Error
TypeError: no implicit conversion of nil into String
simple-navigation (4.0.3) lib/simple_navigation/item.rb:177:in `escape'
simple-navigation (4.0.3) lib/simple_navigation/item.rb:177:in `selected_by_subpath?'
simple-navigation (4.0.3) lib/simple_navigation/item.rb:163:in `autohighlight_by_subpath?'
simple-navigation (4.0.3) lib/simple_navigation/item.rb:159:in `selected_by_autohighlight?'
simple-navigation (4.0.3) lib/simple_navigation/item.rb:104:in `selected_by_condition?'
simple-navigation (4.0.3) lib/simple_navigation/item.rb:45:in `selected?'
simple-navigation (4.0.3) lib/simple_navigation/item_container.rb:109:in `any?'
simple-navigation (4.0.3) lib/simple_navigation/item_container.rb:109:in `selected?'
simple-navigation (4.0.3) lib/simple_navigation/item.rb:99:in `selected_by_subnav?'
simple-navigation (4.0.3) lib/simple_navigation/item.rb:45:in `selected?'
simple-navigation (4.0.3) lib/simple_navigation/item.rb:73:in `selected_class'
simple-navigation (4.0.3) lib/simple_navigation/item.rb:55:in `html_options'
simple-navigation (4.0.3) lib/simple_navigation/renderer/list.rb:32:in `block in list_content'
simple-navigation (4.0.3) lib/simple_navigation/renderer/list.rb:31:in `map'
simple-navigation (4.0.3) lib/simple_navigation/renderer/list.rb:31:in `list_content'
simple-navigation (4.0.3) lib/simple_navigation/renderer/list.rb:23:in `render'
simple-navigation (4.0.3) lib/simple_navigation/item_container.rb:103:in `render'
simple-navigation (4.0.3) lib/simple_navigation/helpers.rb:88:in `render_navigation'
...
https://github.com/codeplant/simple-navigation/blob/master/lib/simple_navigation/item.rb#L177
Hi @gondalez
time is currently very limited, if this is blocking you, it would be awesome if you could investigate yourself, maybe add a test that reproduces this behaviour.
We will look into this, but it might take a while..
Cheers
Andi
Hi @andi
No probs; I too am short on time so added this issue while I had the detail, with the intent to come back and code a fix some day.
I'm not blocked any more.. the workaround/hack I went with was to set the top level nav urls to "" and implemented my own renderer that renders top level items without links.
I just wanted to add the info I found since I just ran into this today. This issue also happens when using an empty string url in the config. It looks to be because the code to determine if the link should be selected calls #remove_anchors in "lib/simple_navigation/item.rb". This method calls "split('#').first", which returns nil. That causes the method to return nil, which ends up causing a problem in #selected_by_subpath? when you try to escape the URL.
The #remove_query_params method may also have the same issue, since it also does "split('?').first"
If they did something like:
def remove_anchors(url_with_anchors)
url_with_anchors && url_with_anchors.split('#').first || ''
end
They would just return an empty string, and I think the escape call would be fine with it.
They would just return an empty string, and I think the escape call would be fine with it.
It would be fine, but it would yield the wrong result, i.e. items with empty url would be all highlighted according to subpath. See linked pr for a simple solution.