Allow ActionList dividers to be added manually (#1875)

.changeset/selfish-glasses-count.md

@@ -0,0 +1,5 @@
+---
+'@primer/view-components': patch
+---
+
+Allow ActionList dividers to be added manually

app/components/primer/alpha/action_list.html.erb

@@ -4,7 +4,8 @@
   <% end %>
   <%= render(Primer::BaseComponent.new(tag: :ul, **@system_arguments)) do %>
     <% items.each_with_index do |item, index| %>
-      <% if index > 0 && @show_dividers %>
+      <%# the conditions here make sure two dividers are never rendered one after the other %>
+      <% if index > 0 && @show_dividers && !item.is_a?(Divider) && !items[index - 1].is_a?(Divider) %>
         <%= render(Primer::Alpha::ActionList::Divider.new) %>
       <% end %>
       <%= item %>

app/components/primer/alpha/action_list.rb

@@ -48,6 +48,15 @@ def custom_element_name
         end
       }
 
+      # Adds a divider to the list of items.
+      #
+      # @param system_arguments [Hash] The arguments accepted by <%= link_to_component(Primer::Alpha::ActionList::Divider) %>.
+      def with_divider(**system_arguments, &block)
+        # This is a giant hack that should be removed when :items can be converted into a polymorphic slot.
+        # This feature needs to land in view_component first: https://github.com/ViewComponent/view_component/pull/1652
+        set_slot(:items, { renderable: Divider, collection: true }, **system_arguments, &block)
+      end
+
       # @param role [Boolean] ARIA role describing the function of the list. listbox and menu are a common values.
       # @param item_classes [String] Additional CSS classes to attach to items.
       # @param scheme [Symbol] <%= one_of(Primer::Alpha::ActionList::SCHEME_OPTIONS) %>. `inset` children are offset (vertically and horizontally) from list edges. `full` (default) children are flush (vertically and horizontally) with list edges.
@@ -81,12 +90,13 @@ def initialize(
       # @private
       def before_render
         aria_label = aria(:label, @system_arguments)
+        aria_labelledby = aria(:labelledby, @system_arguments)
 
         if heading.present?
           @system_arguments[:"aria-labelledby"] = @id
           raise ArgumentError, "An aria-label should not be provided if a heading is present" if aria_label.present?
-        elsif aria_label.blank?
-          raise ArgumentError, "An aria-label or heading must be provided"
+        elsif aria_label.blank? && aria_labelledby.blank?
+          raise ArgumentError, "An aria-label, aria-labelledby, or heading must be provided"
         end
       end
 

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

@@ -16,8 +16,8 @@ class Divider < Primer::Component
         # @param system_arguments [Hash] <%= link_to_system_arguments_docs %>
         def initialize(scheme: DEFAULT_SCHEME, **system_arguments)
           @system_arguments = system_arguments
-          @system_arguments[:tag] = :div
-          @system_arguments[:role] = :separator
+          @system_arguments[:tag] = :li
+          @system_arguments[:role] = :presentation
           @system_arguments[:'aria-hidden'] = true
           @scheme = fetch_or_fallback(SCHEME_OPTIONS, scheme, DEFAULT_SCHEME)
           @system_arguments[:classes] = class_names(

previews/primer/alpha/action_list_preview.rb

@@ -89,6 +89,39 @@ def leading_visuals(
         end
       end
 
+      # @label With manual dividers
+      #
+      # @param role text
+      # @param scheme [Symbol] select [full, inset]
+      # @param show_dividers toggle
+      def with_manual_dividers(
+        role: Primer::Alpha::ActionList::DEFAULT_ROLE,
+        scheme: Primer::Alpha::ActionList::DEFAULT_SCHEME,
+        show_dividers: false
+      )
+        render(Primer::Alpha::ActionList.new(
+          role: role,
+          scheme: scheme,
+          show_dividers: show_dividers
+        )) do |component|
+          component.with_heading(title: "Action List")
+          component.with_item(label: "Item one", href: "/") do |item|
+            item.with_leading_visual_icon(icon: :gear)
+          end
+          component.with_divider
+          component.with_item(label: "Item two", href: "/") do |item|
+            item.with_leading_visual_icon(icon: :star)
+          end
+          component.with_item(label: "Item three", href: "/") do |item|
+            item.with_leading_visual_icon(icon: :heart)
+          end
+          component.with_divider
+          component.with_item(label: "Item four", href: "/") do |item|
+            item.with_leading_visual_icon(icon: :bug)
+          end
+        end
+      end
+
       # @label Divider
       #
       # @param scheme [Symbol] select [subtle, filled]

test/components/alpha/action_list_test.rb

@@ -12,7 +12,7 @@ def test_invalid_list
           render_inline(Primer::Alpha::ActionList.new)
         end
 
-        assert_includes(error.message, "label or heading must be provided")
+        assert_includes(error.message, "aria-label, aria-labelledby, or heading must be provided")
       end
 
       def test_active_item
@@ -83,6 +83,32 @@ def test_heading_denies_tag_argument
 
         assert_match(/This component has a fixed tag/, error.message)
       end
+
+      def test_manual_dividers
+        render_inline(Primer::Alpha::ActionList.new(aria: { label: "List" })) do |component|
+          component.with_item(label: "Item 1", href: "/item1")
+          component.with_item(label: "Item 2", href: "/item2")
+          component.with_divider
+          component.with_item(label: "Item 3", href: "/item3")
+        end
+
+        list_items = page.find_css("ul.ActionListWrap li").map do |list_item|
+          classes = list_item["class"].split
+
+          if classes.include?("ActionListItem")
+            { type: :item, href: list_item.css("a").first["href"] }
+          elsif classes.include?("ActionList-sectionDivider")
+            { type: :divider }
+          end
+        end
+
+        assert_equal list_items, [
+          { type: :item, href: "/item1" },
+          { type: :item, href: "/item2" },
+          { type: :divider },
+          { type: :item, href: "/item3" }
+        ]
+      end
     end
   end
 end