Skip to content

Commit

Permalink
Include empty attributes in HTML output (#543)
Browse files Browse the repository at this point in the history
This is mostly a full revert of commit 1526789 which was a bad change. Empty attributes are valid HTML and should be supported in Arbre (e.g. boolean attributes). The bad commit was for just addressing the class attribute but that applied to any attribute in HTML when it shouldn't have, even for class. Note that if an attribute value is nil then it is removed. This follows similar output logic to Rails `tag.attributes` helper.
  • Loading branch information
javierjulio authored Oct 24, 2023
1 parent 70ed0b9 commit 8164455
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 17 deletions.
11 changes: 1 addition & 10 deletions lib/arbre/html/attributes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,11 @@ module HTML
class Attributes < Hash

def to_s
flatten_hash.map do |name, value|
next if value_empty?(value)
flatten_hash.compact.map do |name, value|
"#{html_escape(name)}=\"#{html_escape(value)}\""
end.compact.join ' '
end

def any?
super{ |k,v| !value_empty?(v) }
end

protected

def flatten_hash(hash = self, old_path = [], accumulator = {})
Expand All @@ -29,10 +24,6 @@ def flatten_hash(hash = self, old_path = [], accumulator = {})
accumulator
end

def value_empty?(value)
value.respond_to?(:empty?) ? value.empty? : !value
end

def html_escape(s)
ERB::Util.html_escape(s)
end
Expand Down
14 changes: 7 additions & 7 deletions spec/arbre/unit/html/tag_attributes_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@
expect(tag.to_s).to eq "<tag id=\"my_id\"></tag>\n"
end

it "shouldn't render attributes that are empty" do
it "should still render attributes that are empty but not nil" do
tag.class_list # initializes an empty ClassList
tag.set_attribute :foo, ''
tag.set_attribute :bar, nil

expect(tag.to_s).to eq "<tag id=\"my_id\"></tag>\n"
expect(tag.to_s).to eq "<tag id=\"my_id\" class=\"\" foo=\"\"></tag>\n"
end

context "with hyphenated attributes" do
Expand All @@ -41,28 +41,28 @@
expect(tag.to_s).to eq "<tag id=\"my_id\" data-action=\"some_action\"></tag>\n"
end

it "shouldn't render attributes that are empty" do
it "should still render attributes that are empty but not nil" do
tag.class_list # initializes an empty ClassList
tag.set_attribute :foo, { bar: '' }
tag.set_attribute :bar, { baz: nil }

expect(tag.to_s).to eq "<tag id=\"my_id\" data-action=\"some_action\"></tag>\n"
expect(tag.to_s).to eq "<tag id=\"my_id\" data-action=\"some_action\" class=\"\" foo-bar=\"\"></tag>\n"
end
end

context "when there is a deeply nested attribute" do
before { tag.build id: "my_id", foo: { bar: { baz: 'foozle' } } }
before { tag.build id: "my_id", foo: { bar: { bat: nil, baz: 'foozle' } } }

it "should flatten the attributes when rendering to html" do
expect(tag.to_s).to eq "<tag id=\"my_id\" foo-bar-baz=\"foozle\"></tag>\n"
end
end

context "when there are multiple nested attributes" do
before { tag.build id: "my_id", foo: { bar: 'foozle1', baz: 'foozle2' } }
before { tag.build id: "my_id", foo: { bar: 'foozle1', bat: nil, baz: '' } }

it "should flatten the attributes when rendering to html" do
expect(tag.to_s).to eq "<tag id=\"my_id\" foo-bar=\"foozle1\" foo-baz=\"foozle2\"></tag>\n"
expect(tag.to_s).to eq "<tag id=\"my_id\" foo-bar=\"foozle1\" foo-baz=\"\"></tag>\n"
end
end
end
Expand Down

0 comments on commit 8164455

Please sign in to comment.