Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify merge_aria to combine plural attributes; introduce merge_data #1895

Merged
merged 3 commits into from
Mar 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/eight-rockets-brush.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/view-components': patch
---

Modify merge_aria to combine plural attributes; introduce merge_data
62 changes: 59 additions & 3 deletions app/components/primer/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
#
Expand All @@ -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
Expand Down
21 changes: 18 additions & 3 deletions test/components/component_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down