From 81644550883aee7ba59feade78fa32dfbf7f9984 Mon Sep 17 00:00:00 2001 From: Javier Julio Date: Tue, 24 Oct 2023 17:31:29 -0400 Subject: [PATCH] Include empty attributes in HTML output (#543) 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. --- lib/arbre/html/attributes.rb | 11 +---------- spec/arbre/unit/html/tag_attributes_spec.rb | 14 +++++++------- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/lib/arbre/html/attributes.rb b/lib/arbre/html/attributes.rb index 8fd92bf6..b0030495 100644 --- a/lib/arbre/html/attributes.rb +++ b/lib/arbre/html/attributes.rb @@ -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 = {}) @@ -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 diff --git a/spec/arbre/unit/html/tag_attributes_spec.rb b/spec/arbre/unit/html/tag_attributes_spec.rb index 19bb80a4..46167579 100644 --- a/spec/arbre/unit/html/tag_attributes_spec.rb +++ b/spec/arbre/unit/html/tag_attributes_spec.rb @@ -18,12 +18,12 @@ expect(tag.to_s).to eq "\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 "\n" + expect(tag.to_s).to eq "\n" end context "with hyphenated attributes" do @@ -41,17 +41,17 @@ expect(tag.to_s).to eq "\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 "\n" + expect(tag.to_s).to eq "\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 "\n" @@ -59,10 +59,10 @@ 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 "\n" + expect(tag.to_s).to eq "\n" end end end