Merge 2289d1c into c7e0aed

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## 4.0.0 (unreleased)
 
+* Creating an Item no longer changed its container, only adding it to a continer
+  does
+* `Item#autogenerate_item_ids?` has been removed
 * `SN.config_file_name`, `SN.config_file` and `SN.config_file?` have been
   removed
 * `ConfigFileFinder` and `ConfigFile` handle the configuration logic

lib/simple_navigation/item.rb

@@ -20,7 +20,7 @@ def initialize(container, key, name, url_or_options = {}, options_or_nil = {}, i
       @key = key
       @method = options.delete(:method)
       @name = name
-      @container = setup_container(container, options)
+      @container = container
 
       setup_sub_navigation(items, &sub_nav_block)
     end
@@ -33,7 +33,7 @@ def initialize(container, key, name, url_or_options = {}, options_or_nil = {}, i
     def name(options = {})
       options = { apply_generator: true }.merge(options)
       if (options[:apply_generator])
-        SimpleNavigation.config.name_generator.call(@name, self)
+        config.name_generator.call(@name, self)
       else
         @name
       end
@@ -42,26 +42,23 @@ def name(options = {})
     # Returns true if this navigation item should be rendered as 'selected'.
     # An item is selected if
     #
-    # * it has been explicitly selected in a controller or
     # * it has a subnavigation and one of its subnavigation items is selected or
     # * its url matches the url of the current request (auto highlighting)
     #
     def selected?
-      @selected ||= selected_by_config?    ||
-                    selected_by_subnav?    ||
-                    selected_by_condition?
+      @selected ||= selected_by_subnav? || selected_by_condition?
     end
 
     # Returns the html-options hash for the item, i.e. the options specified
     # for this item in the config-file.
     # It also adds the 'selected' class to the list of classes if necessary.
     def html_options
       options = @html_options
-      options[:id] ||= autogenerated_item_id if autogenerate_item_ids?
+      options[:id] ||= autogenerated_item_id
 
       classes = [@html_options[:class], selected_class, active_leaf_class]
       classes = classes.flatten.compact.join(' ')
-      options[:class] = classes unless classes.nil? || classes.empty?
+      options[:class] = classes if classes && !classes.empty?
 
       options
     end
@@ -70,15 +67,15 @@ def html_options
     # nil otherwise
     def active_leaf_class
       if !selected_by_subnav? && selected_by_condition?
-        SimpleNavigation.config.active_leaf_class
+        config.active_leaf_class
       end
     end
 
     # Returns the configured selected_class if the item is selected,
     # nil otherwise
     def selected_class
       if selected?
-        container.selected_class || SimpleNavigation.config.selected_class
+        container.selected_class || config.selected_class
       end
     end
 
@@ -90,10 +87,6 @@ def selected_by_subnav?
       sub_navigation && sub_navigation.selected?
     end
 
-    def selected_by_config?
-      false
-    end
-
     # Returns true if the item's url matches the request's current url.
     def selected_by_condition?
       highlights_on ? selected_by_highlights_on? : selected_by_autohighlight?
@@ -104,19 +97,14 @@ def root_path_match?
       url == '/' && SimpleNavigation.request_path == '/'
     end
 
-    # Returns true if the item's id should be added to the rendered output.
-    def autogenerate_item_ids?
-      SimpleNavigation.config.autogenerate_item_ids
-    end
-
     # Returns the item's id which is added to the rendered output.
     def autogenerated_item_id
-      SimpleNavigation.config.id_generator.call(key)
+      config.id_generator.call(key) if config.autogenerate_item_ids
     end
 
     # Return true if auto_highlight is on for this item.
     def auto_highlight?
-      SimpleNavigation.config.auto_highlight && container.auto_highlight
+      config.auto_highlight && container.auto_highlight
     end
 
     def url_without_anchor
@@ -131,6 +119,14 @@ def url_without_anchor
                 :sub_navigation,
                 :url
 
+    def config
+      SimpleNavigation.config
+    end
+
+    def request_uri
+      SimpleNavigation.request_uri
+    end
+
     def selected_by_autohighlight?
       auto_highlight? &&
       (root_path_match? ||
@@ -139,41 +135,17 @@ def selected_by_autohighlight?
     end
 
     def selected_by_highlights_on?
-      return false unless highlights_on
-
       case highlights_on
-      when Regexp then SimpleNavigation.request_uri =~ highlights_on
+      when Regexp then request_uri =~ highlights_on
       when Proc then highlights_on.call
       when :subpath
         escaped_url = Regexp.escape(url_without_anchor)
-        !!(SimpleNavigation.request_uri =~ /^#{escaped_url}(\/|$|\?)/i)
+        !!(request_uri =~ /^#{escaped_url}(\/|$|\?)/i)
       else
         fail ArgumentError, ':highlights_on must be a Regexp, Proc or :subpath'
       end
     end
 
-    def setup_container(container, options = {})
-      if options[:container_class]
-        container.dom_class = options.delete(:container_class)
-      end
-
-      if options[:container_id]
-        container.dom_id = options.delete(:container_id)
-      end
-
-      container.dom_attributes = if options[:container_attributes]
-                                   options.delete(:container_attributes)
-                                 else
-                                   {}
-                                 end
-
-      if options[:selected_class]
-        container.selected_class = options.delete(:selected_class)
-      end
-
-      container
-    end
-
     def setup_url_and_options(url_or_options, options_or_nil)
       case url_or_options
       when Hash then options = url_or_options # there is no url

lib/simple_navigation/item_adapter.rb

@@ -1,4 +1,5 @@
 require 'forwardable'
+require 'ostruct'
 
 module SimpleNavigation
   # This class acts as an adapter to items that are not defined using the DSL
@@ -30,7 +31,7 @@ class ItemAdapter
     attr_reader :item
 
     def initialize(item)
-      @item = item.is_a?(Hash) ? to_object(item) : item
+      @item = item.is_a?(Hash) ? OpenStruct.new(item) : item
     end
 
     # Returns the options for this item. If the wrapped item does not implement
@@ -49,20 +50,5 @@ def items
     def to_simple_navigation_item(item_container)
       SimpleNavigation::Item.new(item_container, key, name, url, options, items)
     end
-
-    protected
-
-    # Converts the specified hash into an object. Each key will be added
-    # as method.
-    def to_object(hash)
-      mod = Module.new do
-        hash.each_pair do |key, value|
-          define_method key do
-            value
-          end
-        end
-      end
-      Object.new.extend(mod)
-    end
   end
 end

lib/simple_navigation/item_container.rb

@@ -62,19 +62,16 @@ def dom_attributes
     def item(key, name, url_or_options = {}, options_or_nil = {}, &block)
       options = url_or_options.is_a?(Hash) ? url_or_options : options_or_nil
       return unless should_add_item?(options)
-      items << SimpleNavigation::Item.new(self,
-                                          key,
-                                          name,
-                                          url_or_options,
-                                          options_or_nil,
-                                          nil,
-                                          &block)
+      item = Item.new(self, key, name, url_or_options, options_or_nil, nil, &block)
+      add_item item, options
     end
 
     def items=(new_items)
-      @items += new_items.map { |item| ItemAdapter.new(item) }
-                         .keep_if { |item| should_add_item?(item.options) }
-                         .map { |item| item.to_simple_navigation_item(self) }
+      new_items.each do |item|
+        item_adapter = ItemAdapter.new(item)
+        next unless should_add_item?(item_adapter.options)
+        add_item item_adapter.to_simple_navigation_item, item_adapter.options
+      end
     end
 
     # Returns the Item with the specified key, nil otherwise.
@@ -148,6 +145,18 @@ def empty?
 
     private
 
+    def add_item(item, options)
+      items << item
+      modify_dom_attributes(options)
+    end
+
+    def modify_dom_attributes(options)
+      self.dom_attributes = options.delete(:container_attributes) { dom_attributes }
+      self.dom_class = options.delete(:container_class) { dom_class }
+      self.dom_id = options.delete(:container_id) { dom_id }
+      self.selected_class = options.delete(:selected_class) { selected_class }
+    end
+
     # FIXME: raise an exception if :rederer is a symbol and it is not registred
     #        in SimpleNavigation.registered_renderers
     def renderer_instance(options)

spec/simple_navigation/item_container_spec.rb

@@ -308,6 +308,93 @@ module SimpleNavigation
             end
           end
         end
+
+        describe "container options" do
+          before do
+            item_container.stub(should_add_item?: add_item)
+            item_container.item :key, 'name', 'url', options
+          end
+
+          context 'when the :container_id option is specified' do
+            let(:options) {{ container_id: 'c_id' }}
+
+            context 'and the item should be added' do
+              let(:add_item) { true }
+
+              it 'changes its dom_id' do
+                expect(item_container.dom_id).to eq 'c_id'
+              end
+            end
+
+            context "and the item shouldn't be added" do
+              let(:add_item) { false }
+
+              it "doesn't change its dom_id" do
+                expect(item_container.dom_id).to be_nil
+              end
+            end
+          end
+
+          context 'when the :container_class option is specified' do
+            let(:options) {{ container_class: 'c_class' }}
+
+            context 'and the item should be added' do
+              let(:add_item) { true }
+
+              it 'changes its dom_class' do
+                expect(item_container.dom_class).to eq 'c_class'
+              end
+            end
+
+            context "and the item shouldn't be added" do
+              let(:add_item) { false }
+
+              it "doesn't change its dom_class" do
+                expect(item_container.dom_class).to be_nil
+              end
+            end
+          end
+
+          context 'when the :container_attributes option is specified' do
+            let(:options) {{ container_attributes: { option: true } }}
+
+            context 'and the item should be added' do
+              let(:add_item) { true }
+
+              it 'changes its dom_attributes' do
+                expect(item_container.dom_attributes).to eq(option: true)
+              end
+            end
+
+            context "and the item shouldn't be added" do
+              let(:add_item) { false }
+
+              it "doesn't change its dom_attributes" do
+                expect(item_container.dom_attributes).to eq({})
+              end
+            end
+          end
+
+          context 'when the :selected_class option is specified' do
+            let(:options) {{ selected_class: 'sel_class' }}
+
+            context 'and the item should be added' do
+              let(:add_item) { true }
+
+              it 'changes its selected_class' do
+                expect(item_container.selected_class).to eq 'sel_class'
+              end
+            end
+
+            context "and the item shouldn't be added" do
+              let(:add_item) { false }
+
+              it "doesn't change its selected_class" do
+                expect(item_container.selected_class).to be_nil
+              end
+            end
+          end
+        end
       end
 
       describe 'Conditions' do