diff --git a/.changeset/selfish-glasses-count.md b/.changeset/selfish-glasses-count.md new file mode 100644 index 0000000000..535fe6c653 --- /dev/null +++ b/.changeset/selfish-glasses-count.md @@ -0,0 +1,5 @@ +--- +'@primer/view-components': patch +--- + +Allow ActionList dividers to be added manually diff --git a/app/components/primer/alpha/action_list.html.erb b/app/components/primer/alpha/action_list.html.erb index fe2724dc2a..d65e86ffdb 100644 --- a/app/components/primer/alpha/action_list.html.erb +++ b/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 %> diff --git a/app/components/primer/alpha/action_list.rb b/app/components/primer/alpha/action_list.rb index fd409d63a8..eca08f52f1 100644 --- a/app/components/primer/alpha/action_list.rb +++ b/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 diff --git a/app/components/primer/alpha/action_list/divider.rb b/app/components/primer/alpha/action_list/divider.rb index 684cb63acc..4534ccba65 100644 --- a/app/components/primer/alpha/action_list/divider.rb +++ b/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( diff --git a/previews/primer/alpha/action_list_preview.rb b/previews/primer/alpha/action_list_preview.rb index 9e98674da9..abbc053b4b 100644 --- a/previews/primer/alpha/action_list_preview.rb +++ b/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] diff --git a/test/components/alpha/action_list_test.rb b/test/components/alpha/action_list_test.rb index 8344800284..a9d31a3057 100644 --- a/test/components/alpha/action_list_test.rb +++ b/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