Skip to content

Commit

Permalink
Include blank attributes in HTML output (#517)
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.
  • Loading branch information
javierjulio authored Jul 21, 2023
1 parent 186106d commit cf59b8f
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 16 deletions.
17 changes: 6 additions & 11 deletions lib/arbre/html/attributes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@ class Attributes < Hash

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

def any?
super{ |k,v| !value_empty?(v) }
if value.nil?
html_escape(name)
else
"#{html_escape(name)}=\"#{html_escape(value)}\""
end
end.join ' '
end

protected
Expand All @@ -29,10 +28,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
2 changes: 1 addition & 1 deletion lib/arbre/html/tag.rb
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ def child_is_text?
end

def attributes_html
" #{attributes}" if attributes.any?
attributes.any? ? " " + attributes.to_s : nil
end

def set_for_attribute(record)
Expand Down
8 changes: 4 additions & 4 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" 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=\"\" bar></tag>\n"
end

context "with hyphenated attributes" do
Expand All @@ -41,12 +41,12 @@
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" 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=\"\" bar-baz></tag>\n"
end
end

Expand Down

0 comments on commit cf59b8f

Please sign in to comment.