Allow NavList items to be selected by the current page

.changeset/twelve-doors-tease.md

@@ -0,0 +1,5 @@
+---
+'@primer/view-components': patch
+---
+
+Allow NavList items to be selected by the current page

app/components/primer/alpha/action_list/item.rb

@@ -112,7 +112,7 @@ class Item < Primer::Component
         # @private
         renders_one :private_content
 
-        attr_reader :list, :active, :disabled, :parent
+        attr_reader :list, :href, :active, :disabled, :parent
 
         # Whether or not this item is active.
         #
@@ -176,8 +176,7 @@ def initialize(
           @system_arguments[:classes] = class_names(
             @system_arguments[:classes],
             SCHEME_MAPPINGS[@scheme],
-            "ActionListItem",
-            "ActionListItem--navActive" => @active
+            "ActionListItem"
           )
 
           @system_arguments[:role] = role
@@ -223,7 +222,8 @@ def before_render
           @system_arguments[:classes] = class_names(
             @system_arguments[:classes],
             "ActionListItem--withActions" => trailing_action.present?,
-            "ActionListItem--trailingActionHover" => @trailing_action_on_hover
+            "ActionListItem--trailingActionHover" => @trailing_action_on_hover,
+            "ActionListItem--navActive" => active?
           )
 
           return unless leading_visual

app/components/primer/alpha/nav_list/item.rb

@@ -16,13 +16,6 @@ class Item < Primer::Alpha::ActionList::Item
 
           @list.build_item(parent: self, sub_item: true, **system_arguments).tap do |item|
             @list.will_add_item(item)
-
-            if item.active?
-              @content_arguments[:classes] = class_names(
-                @content_arguments[:classes],
-                "ActionListContent--hasActiveSubItem"
-              )
-            end
           end
         }
 
@@ -55,17 +48,29 @@ def initialize(selected_item_id: nil, selected_by_ids: [], sub_item: false, expa
           }
 
           overrides = { "data-item-id": @selected_by_ids.join(" ") }
-          overrides[:active] = @selected_by_ids.include?(@selected_item_id)
 
           super(**system_arguments, **overrides)
         end
 
+        def active?
+          item_active?(self)
+        end
+
         # Cause this item to show its list of sub items when rendered.
         def expand!
           @expanded = true
         end
 
         def before_render
+          if active_sub_item?
+            expand!
+
+            @content_arguments[:classes] = class_names(
+              @content_arguments[:classes],
+              "ActionListContent--hasActiveSubItem"
+            )
+          end
+
           super
 
           raise "Cannot render a trailing visual for an item with subitems" if items.present? && trailing_visual.present?
@@ -83,6 +88,35 @@ def before_render
             "ActionListItem--hasSubItem"
           )
         end
+
+        private
+
+        # Normally it would be easier to simply ask each item for its active status, eg.
+        # items.any?(&:active?), but unfortunately the view context is not set on each
+        # item until _after_ the parent's before_render, etc methods have been called.
+        # This means helper methods like current_page? will blow up with an error, since
+        # `helpers` is simply an alias for the view context (i.e. an instance of
+        # ActionView::Base). Since we know the view context for the parent object must
+        # be set before `before_render` is invoked, we can call helper methods here in
+        # the parent and bypass the problem entirely. Maybe not the most OO approach,
+        # but it works.
+        def item_active?(item)
+          if item.selected_by_ids.present?
+            item.selected_by_ids.include?(@selected_item_id)
+          elsif item.href
+            current_page?(item.href)
+          else
+            false
+          end
+        end
+
+        def active_sub_item?
+          items.any? { |subitem| item_active?(subitem) }
+        end
+
+        def current_page?(url)
+          helpers.current_page?(url)
+        end
       end
     end
   end

app/components/primer/alpha/nav_list/section.rb

@@ -77,11 +77,6 @@ def build_item(component_klass: NavList::Item, **system_arguments)
             list: self
           )
         end
-
-        # @private
-        def will_add_item(item)
-          item.parent.expand! if item.active? && item.parent
-        end
       end
     end
   end

test/components/alpha/nav_list_test.rb

@@ -64,6 +64,34 @@ def test_item_can_be_selected_by_any_of_its_ids
         refute_selector ".ActionListItem--navActive", text: "Item 2"
       end
 
+      class CurrentPageItem < Primer::Alpha::NavList::Item
+        def initialize(current_page_href:, **system_arguments)
+          @current_page_href = current_page_href
+          super(**system_arguments)
+        end
+
+        private
+
+        def current_page?(url)
+          url == @current_page_href
+        end
+      end
+
+      def test_item_can_be_selected_by_current_page
+        current_page_href = "/item2"
+
+        render_inline(Primer::Alpha::NavList.new) do |c|
+          c.with_section(aria: { label: "Nav list" }) do |section|
+            # use CurrentPageItem instead of a mock since the API supports it
+            section.with_item(component_klass: CurrentPageItem, label: "Item 1", href: "/item1", current_page_href: current_page_href)
+            section.with_item(component_klass: CurrentPageItem, label: "Item 2", href: "/item2", current_page_href: current_page_href)
+          end
+        end
+
+        refute_selector ".ActionListItem--navActive", text: "Item 1"
+        assert_selector ".ActionListItem--navActive", text: "Item 2"
+      end
+
       def test_max_nesting_depth
         error = assert_raises(RuntimeError) do
           render_inline(Primer::Alpha::NavList.new) do |c|