Modify merge_aria to combine plural attributes; introduce merge_data (#…

…1895)

.changeset/eight-rockets-brush.md

@@ -0,0 +1,5 @@
+---
+'@primer/view-components': patch
+---
+
+Modify merge_aria to combine plural attributes; introduce merge_data

app/components/primer/component.rb

@@ -25,6 +25,8 @@ class Component < ViewComponent::Base
     include Audited::Dsl
 
     INVALID_ARIA_LABEL_TAGS = [:div, :span, :p].freeze
+    PLURAL_ARIA_ATTRIBUTES = %i[describedby labelledby].freeze
+    PLURAL_DATA_ATTRIBUTES = %i[target targets].freeze
 
     def self.deprecated?
       status == :deprecated
@@ -64,6 +66,12 @@ def aria(val, system_arguments)
     # Eg. merge_aria({ "aria-disabled": "true" }, { aria: { invalid: "true" } })
     #     => { disabled: "true", invalid: "true" }
     #
+    # Certain aria attributes can contain multiple values separated by spaces. merge_aria
+    # will combine these plural attributes into a composite string.
+    #
+    # Eg. merge_aria({ "aria-labelledby": "foo" }, { aria: { labelledby: "bar" } })
+    #     => { labelledby: "foo bar" }
+    #
     # It's designed to be used to normalize and merge aria information from system_arguments
     # hashes. Consider using this pattern in component initializers:
     #
@@ -72,17 +80,65 @@ def aria(val, system_arguments)
     #   { aria: { labelled_by: id } }
     # )
     def merge_aria(*hashes)
+      merge_prefixed_attribute_hashes(
+        *hashes, prefix: :aria, plural_keys: PLURAL_ARIA_ATTRIBUTES
+      )
+    end
+
+    # Merges hashes that contain "data-*" keys and nested data: hashes. Removes keys from
+    # each hash and returns them in the new hash.
+    #
+    # Eg. merge_data({ "data-foo": "true" }, { data: { bar: "true" } })
+    #     => { foo: "true", bar: "true" }
+    #
+    # Certain data attributes can contain multiple values separated by spaces. merge_data
+    # will combine these plural attributes into a composite string.
+    #
+    # Eg. merge_data({ "data-target": "foo" }, { data: { target: "bar" } })
+    #     => { target: "foo bar" }
+    #
+    # It's designed to be used to normalize and merge data information from system_arguments
+    # hashes. Consider using this pattern in component initializers:
+    #
+    # @system_arguments[:data] = merge_aria(
+    #   @system_arguments,
+    #   { data: { foo: "bar" } }
+    # )
+    def merge_data(*hashes)
+      merge_prefixed_attribute_hashes(
+        *hashes, prefix: :data, plural_keys: PLURAL_DATA_ATTRIBUTES
+      )
+    end
+
+    def merge_prefixed_attribute_hashes(*hashes, prefix:, plural_keys:)
       {}.tap do |result|
         hashes.each do |hash|
           next unless hash
 
-          result.merge!(hash.delete(:aria) || {})
+          prefix_hash = hash.delete(prefix) || {}
+
+          prefix_hash.each_pair do |key, val|
+            result[key] =
+              if plural_keys.include?(key)
+                [*(result[key] || "").split, val].join(" ").strip
+              else
+                val
+              end
+          end
 
           hash.delete_if do |key, val|
             key_s = key.to_s
 
-            if key.start_with?("aria-")
-              result[key_s.sub("aria-", "").to_sym] = val
+            if key.start_with?("#{prefix}-")
+              bare_key = key_s.sub("#{prefix}-", "").to_sym
+
+              result[bare_key] =
+                if plural_keys.include?(bare_key)
+                  [*(result[bare_key] || "").split, val].join(" ").strip
+                else
+                  val
+                end
+
               true
             else
               false

test/components/component_test.rb

@@ -217,12 +217,27 @@ def test_deny_aria_key_does_not_raise_in_production
   def test_merge_aria
     component = Primer::Component.new
 
-    hash1 = { "aria-disabled": "true", aria: { labelledby: "foo" }, foo: "foo" }
-    hash2 = { aria: { invalid: "true" }, "aria-label": "bar", bar: "bar" }
+    hash1 = { "aria-disabled": "true", aria: { labelledby: "foo", describedby: "foo" }, foo: "foo" }
+    hash2 = { aria: { invalid: "true", labelledby: "bar" }, "aria-label": "bar", bar: "bar", "aria-labelledby": "baz", "aria-describedby": nil }
 
     merged_arias = component.send(:merge_aria, hash1, hash2)
 
-    assert_equal merged_arias, { disabled: "true", invalid: "true", labelledby: "foo", label: "bar" }
+    assert_equal merged_arias, { disabled: "true", invalid: "true", labelledby: "foo bar baz", label: "bar", describedby: "foo" }
+
+    # assert aria info removed from original hashes
+    assert_equal hash1, { foo: "foo" }
+    assert_equal hash2, { bar: "bar" }
+  end
+
+  def test_merge_data
+    component = Primer::Component.new
+
+    hash1 = { "data-foo": "true", data: { target: "foo" }, foo: "foo" }
+    hash2 = { data: { bar: "false", target: "bar" }, "data-baz": "baz", bar: "bar", "data-target": "baz" }
+
+    merged_data = component.send(:merge_data, hash1, hash2)
+
+    assert_equal merged_data, { foo: "true", bar: "false", target: "foo bar baz", baz: "baz" }
 
     # assert aria info removed from original hashes
     assert_equal hash1, { foo: "foo" }