Repository: phoenixframework/phoenix_html
Pull request #331 (2021-02-08 12:44:58)

Add Tag.attributes_escape/1
This PR adds a new function called `attributes_escape/1` that will be used by the new template engine I'm working on. I'm not sure this function should be placed in the `Tag` module since neither `tag` nor `content_tag` use it. However, I believe they should use it for consistency. The problem is that, currently, the implementation sorts the attributes, which makes no sense for the new engine as it generates groups of attributes separately based on their types (static key/value, static key + dynamic value and dynamic key/value). 

I can either move this function to someplace else or change the implementation of `tag` and `content_tag` to use it. The latter might break existing code if, for any reason, the order of the attributes was taken into account, for instance, in tests.


Patch:
diff --git a/lib/phoenix_html/tag.ex b/lib/phoenix_html/tag.ex
index 3e3ab73..447538b 100644
--- a/lib/phoenix_html/tag.ex
+++ b/lib/phoenix_html/tag.ex
@@ -8,7 +8,8 @@ defmodule Phoenix.HTML.Tag do
 
   import Phoenix.HTML
 
-  @tag_prefixes [:aria, :data]
+  @special_attributes ["data", "aria", "class"]
+
   @csrf_param "_csrf_token"
   @method_param "_method"
 
@@ -49,7 +50,7 @@ defmodule Phoenix.HTML.Tag do
   def tag(name), do: tag(name, [])
 
   def tag(name, attrs) when is_list(attrs) do
-    {:safe, [?<, to_string(name), build_attrs(attrs), ?>]}
+    {:safe, [?<, to_string(name), build_attrs(attrs) |> Enum.sort() |> tag_attrs(), ?>]}
   end
 
   @doc ~S"""
@@ -87,16 +88,49 @@ defmodule Phoenix.HTML.Tag do
   def content_tag(name, content, attrs) when is_list(attrs) do
     name = to_string(name)
     {:safe, escaped} = html_escape(content)
-    {:safe, [?<, name, build_attrs(attrs), ?>, escaped, ?<, ?/, name, ?>]}
+
+    {:safe,
+     [?<, name, build_attrs(attrs) |> Enum.sort() |> tag_attrs(), ?>, escaped, ?<, ?/, name, ?>]}
+  end
+
+  @doc """
+  Escapes a list of attributes, returning iodata.
+
+  Pay attention that, unlike `tag/2` and `content_tag/2`, this
+  function does not sort the attributes.
+
+      iex> attributes_escape(title: "the title", id: "the id", selected: true)
+      {:safe,
+       [
+         [32, "title", 61, 34, "the title", 34],
+         [32, "id", 61, 34, "the id", 34],
+         [32, "selected"]
+       ]}
+
+  """
+  def attributes_escape(attrs) do
+    {:safe, attrs |> build_attrs() |> Enum.reverse() |> tag_attrs()}
   end
 
   defp build_attrs([]), do: []
   defp build_attrs(attrs), do: build_attrs(attrs, [])
 
-  defp build_attrs([], acc), do: acc |> Enum.sort() |> tag_attrs()
+  defp build_attrs([], acc), do: acc
 
-  defp build_attrs([{k, v} | t], acc) when k in @tag_prefixes and is_list(v) do
-    build_attrs(t, nested_attrs(key_escape(k), v, acc))
+  defp build_attrs([{k, v} | t], acc) when k in @special_attributes do
+    build_attrs([{String.to_atom(k), v} | t], acc)
+  end
+
+  defp build_attrs([{:data, v} | t], acc) when is_list(v) do
+    build_attrs(t, nested_attrs("data", v, acc))
+  end
+
+  defp build_attrs([{:aria, v} | t], acc) when is_list(v) do
+    build_attrs(t, nested_attrs("aria", v, acc))
+  end
+
+  defp build_attrs([{:class, v} | t], acc) when is_list(v) do
+    build_attrs(t, [{"class", class_value(v)} | acc])
   end
 
   defp build_attrs([{k, true} | t], acc) do
@@ -137,6 +171,16 @@ defmodule Phoenix.HTML.Tag do
     end)
   end
 
+  defp class_value(value) when is_list(value) do
+    value
+    |> Enum.filter(& &1)
+    |> Enum.join(" ")
+  end
+
+  defp class_value(value) do
+    value
+  end
+
   defp key_escape(value) when is_atom(value), do: String.replace(Atom.to_string(value), "_", "-")
   defp key_escape(value), do: attr_escape(value)
 


Test patch:
diff --git a/test/phoenix_html/tag_test.exs b/test/phoenix_html/tag_test.exs
index b867b01..bdbd624 100644
--- a/test/phoenix_html/tag_test.exs
+++ b/test/phoenix_html/tag_test.exs
@@ -5,6 +5,61 @@ defmodule Phoenix.HTML.TagTest do
   import Phoenix.HTML.Tag
   doctest Phoenix.HTML.Tag
 
+  describe "attributes_escape" do
+    test "key as atom" do
+      assert attributes_escape([{:title, "the title"}]) |> safe_to_string() ==
+               ~s( title="the title")
+    end
+
+    test "key as string" do
+      assert attributes_escape([{"title", "the title"}]) |> safe_to_string() ==
+               ~s( title="the title")
+    end
+
+    test "convert snake_case keys into kebab-case when key is atom" do
+      assert attributes_escape([{:my_attr, "value"}]) |> safe_to_string() == ~s( my-attr="value")
+    end
+
+    test "keep snake_case keys when key is string" do
+      assert attributes_escape([{"my_attr", "value"}]) |> safe_to_string() == ~s( my_attr="value")
+    end
+
+    test "multiple attributes" do
+      assert attributes_escape([{:title, "the title"}, {:id, "the id"}]) |> safe_to_string() ==
+               ~s( title="the title" id="the id")
+    end
+
+    test "handle nested data" do
+      assert attributes_escape([{"data", [a: "1", b: "2"]}]) |> safe_to_string() ==
+               ~s( data-a="1" data-b="2")
+
+      assert attributes_escape([{"aria", [a: "1", b: "2"]}]) |> safe_to_string() ==
+               ~s( aria-a="1" aria-b="2")
+    end
+
+    test "handle class value as string" do
+      assert attributes_escape([{:class, "btn"}]) |> safe_to_string() == ~s( class="btn")
+    end
+
+    test "handle class value as list" do
+      assert attributes_escape([{:class, ["btn", nil, false, "active"]}]) |> safe_to_string() ==
+               ~s( class="btn active")
+    end
+
+    test "handle class key as string" do
+      assert attributes_escape([{"class", "btn"}]) |> safe_to_string() == ~s( class="btn")
+    end
+
+    test "supress attribute when value is falsy" do
+      assert attributes_escape([{"title", nil}]) |> safe_to_string() == ~s()
+      assert attributes_escape([{"title", false}]) |> safe_to_string() == ~s()
+    end
+
+    test "supress value when value is true" do
+      assert attributes_escape([{"selected", true}]) |> safe_to_string() == ~s( selected)
+    end
+  end
+
   test "tag" do
     assert tag(:br) |> safe_to_string() == ~s(<br>)
 
@@ -20,8 +75,12 @@ defmodule Phoenix.HTML.TagTest do
              ~s(<input data-toggle="dropdown">)
 
     assert tag(:input, my_attr: "blah") |> safe_to_string() == ~s(<input my-attr="blah">)
-    assert tag(:input, [{"my_<_attr", "blah"}]) |> safe_to_string() == ~s(<input my_&lt;_attr="blah">)
-    assert tag(:input, [{{:safe, "my_<_attr"}, "blah"}]) |> safe_to_string() == ~s(<input my_<_attr="blah">)
+
+    assert tag(:input, [{"my_<_attr", "blah"}]) |> safe_to_string() ==
+             ~s(<input my_&lt;_attr="blah">)
+
+    assert tag(:input, [{{:safe, "my_<_attr"}, "blah"}]) |> safe_to_string() ==
+             ~s(<input my_<_attr="blah">)
 
     assert tag(:input, data: [my_attr: "blah"]) |> safe_to_string() ==
              ~s(<input data-my-attr="blah">)