Merge pull request #495 from opf/feature/border-box-list-arguments

Teach BorderBox component a `list_arguments` param

Author: myabc

.changeset/rare-coats-hide.md

@@ -0,0 +1,5 @@
+---
+'@openproject/primer-view-components': patch
+---
+
+Teach BorderBox component a list_arguments param

app/components/primer/beta/border_box.rb

@@ -71,9 +71,12 @@ class BorderBox < Primer::Component
       }
 
       # @param padding [Symbol] <%= one_of(Primer::Beta::BorderBox::PADDING_MAPPINGS.keys) %>
+      # @param list_arguments [Hash] <%= link_to_system_arguments_docs %>
+      # @param list_id [String] Deprecated. Use <code>list_arguments: { id: ... }</code> instead.
       # @param system_arguments [Hash] <%= link_to_system_arguments_docs %>
-      def initialize(padding: DEFAULT_PADDING, **system_arguments)
+      def initialize(padding: DEFAULT_PADDING, list_arguments: {}, **system_arguments)
         list_id = system_arguments.delete(:list_id)
+        deprecation_warn("The `list_id:` param is deprecated. Use `list_arguments: { id: ... }` instead. It will be removed in a future version.") if list_id
 
         @system_arguments = deny_tag_argument(**system_arguments)
         @system_arguments[:tag] = :div
@@ -84,9 +87,10 @@ def initialize(padding: DEFAULT_PADDING, **system_arguments)
         )
 
         @system_arguments[:system_arguments_denylist] = { [:p, :pt, :pb, :pr, :pl] => PADDING_SUGGESTION }
-        @list_arguments = { tag: :ul }
-        @list_arguments[:id] = list_id if list_id
-        @list_arguments[:classes] = "Box-list"
+        @list_arguments = deny_tag_argument(**list_arguments)
+        @list_arguments[:tag] = :ul
+        @list_arguments[:id] ||= list_id if list_id
+        @list_arguments[:classes] = class_names("Box-list", @list_arguments[:classes])
       end
 
       def render?
@@ -98,9 +102,16 @@ def render?
       def before_render
         return unless header
 
-        @list_arguments[:aria] = {
-          labelledby: header.id
-        }
+        @list_arguments[:aria] = merge_aria(
+          @list_arguments,
+          { aria: { labelledby: header.id } }
+        )
+      end
+
+      def deprecation_warn(message)
+        return if Rails.env.production? || silence_deprecations?
+
+        ::Primer::ViewComponents.deprecation.warn(message)
       end
     end
   end

previews/primer/beta/border_box_preview.rb

@@ -6,11 +6,11 @@ module Beta
     class BorderBoxPreview < ViewComponent::Preview
       # @label Playground
       #
+      # @param list_id text
       # @param padding [Symbol] select [default, condensed, spacious]
       # @param scheme [Symbol] select [default, neutral, info, warning]
-      # @param list_id [String] text
-      def playground(padding: :default, scheme: :default, list_id: nil)
-        render(Primer::Beta::BorderBox.new(padding: padding, list_id: list_id)) do |component|
+      def playground(padding: :default, scheme: :default, list_id: "my-list")
+        render(Primer::Beta::BorderBox.new(padding: padding, list_arguments: { id: list_id })) do |component|
           component.with_header { "Header" }
           component.with_body { "Body" }
           component.with_row(scheme: scheme) { "#{scheme.to_s.capitalize} row one" }

static/arguments.json

@@ -4250,6 +4250,18 @@
         "default": "`:default`",
         "description": "One of `:condensed`, `:default`, or `:spacious`."
       },
+      {
+        "name": "list_arguments",
+        "type": "Hash",
+        "default": "`{}`",
+        "description": "[System arguments](/system-arguments)"
+      },
+      {
+        "name": "list_id",
+        "type": "String",
+        "default": "N/A",
+        "description": "Deprecated. Use <code>list_arguments: { id: ... }</code> instead."
+      },
       {
         "name": "system_arguments",
         "type": "Hash",

static/info_arch.json

@@ -13509,6 +13509,18 @@
         "default": "`:default`",
         "description": "One of `:condensed`, `:default`, or `:spacious`."
       },
+      {
+        "name": "list_arguments",
+        "type": "Hash",
+        "default": "`{}`",
+        "description": "{{link_to_system_arguments_docs}}"
+      },
+      {
+        "name": "list_id",
+        "type": "String",
+        "default": "N/A",
+        "description": "Deprecated. Use <code>list_arguments: { id: ... }</code> instead."
+      },
       {
         "name": "system_arguments",
         "type": "Hash",

test/components/beta/border_box_test.rb

@@ -33,6 +33,28 @@ def test_labels_list_with_header
     assert_selector "ul[aria-labelledby='#{id}']"
   end
 
+  def test_renders_list_with_list_arguments
+    render_inline(Primer::Beta::BorderBox.new(list_arguments: { id: "fake-tbody", role: "rowgroup" })) do |component|
+      component.with_row { "Row" }
+    end
+
+    assert_selector "ul#fake-tbody"
+    assert_selector 'ul[role="rowgroup"]'
+  end
+
+  def test_labels_list_with_header_and_additional_label
+    render_inline(Primer::Beta::BorderBox.new(list_arguments: { aria: { labelledby: "my-footer" } })) do |component|
+      component.with_header { "Header" }
+      component.with_row { "Row" }
+      component.with_footer(id: "my-footer") { "Footer" }
+    end
+
+    header_id = page.find_css(".Box-header").first[:id]
+    footer_id = page.find_css(".Box-footer").first[:id]
+    assert_selector "ul[aria-labelledby*='#{header_id}']"
+    assert_selector "ul[aria-labelledby*='#{footer_id}']"
+  end
+
   def test_renders_body
     render_inline(Primer::Beta::BorderBox.new) do |component|
       component.with_body { "Body" }
@@ -69,6 +91,24 @@ def test_renders_the_list_element_with_an_id_if_provided
     assert_selector("li.Box-row", count: 1)
   end
 
+  def test_warns_when_list_id_param_passed
+    with_silence_deprecations(false) do
+      ::Primer::ViewComponents.deprecation.expects(:warn).with("The `list_id:` param is deprecated. Use `list_arguments: { id: ... }` instead. It will be removed in a future version.").once
+      render_inline(Primer::Beta::BorderBox.new(list_id: "an-id")) do |component|
+        component.with_row { "First" }
+      end
+    end
+  end
+
+  def test_list_arguments_id_takes_precedence_over_deprecated_list_id
+    render_inline(Primer::Beta::BorderBox.new(list_id: "old-id", list_arguments: { id: "new-id" })) do |component|
+      component.with_row { "First" }
+    end
+
+    assert_selector("ul#new-id", count: 1)
+    assert_no_selector("ul#old-id")
+  end
+
   def test_renders_condensed
     render_inline(Primer::Beta::BorderBox.new(padding: :condensed)) do |component|
       component.with_body { "Body" }