Repository: woylie/flop_phoenix (elixir)

Issue:
Overriding `attrs` on `col` not working as advertised?
Hello! I just got started with Flop and I'm liking it so far.
I ran into an issue where the behaviour doesn't seem to agree with the documentation.

The [table slots docs](https://hexdocs.pm/flop_phoenix/Flop.Phoenix.html#table/1-slots) for `attrs` says:
>  Note that these attributes will override any conflicting `tbody_td_attrs` that are set at the table level.

I'm not finding that to be the case.

I have default table opts that includes:
```elixir
[
  tbody_td_attrs: [class: ["py-4", "pr-6"]]
]
```

Then, I have numeric columns that I want to align to the right, so I do:

```
<:col :let={{_id, transaction}} label="Amount" attrs={[class: ["py-4", "pr-6", "text-right"]]} field={:amount}>
```

The resulting HTML for each column then looks like:

```
<td class="py-4 pr-6">189.89 GBP</td>
```

I've tried lots of different combinations (strings, lists, maps), and the only thing that works is to remove the `tbody_td_attrs` from my defaults entirely, but then I'd have to set them manually for every column.

It looks like [the code that renders columns](https://github.com/woylie/flop_phoenix/blob/bf32828738a6e0a0579a82c45bdc98551c983db8/lib/flop_phoenix/table.ex#L192-L193) emits both the global/default options and any `attrs` provided to the slot. I can't find anywhere in the HEEx documentation that describes the expected behaviour when there are conflicts.

One other issue comes to mind then: getting the header to also align to the right. If `col_style` were changed to `col_attrs` for example, it would allow more flexible use of the `colgroup`. Then you could do something like:

```
<:col label="Amount" col_attrs={[class: "text-right", "text-monospaced"]} ...
```

Versions in use:
* flop_phoenix `0.22.1`
* phoenix `1.7.7`
* phoenix_live_view `0.19.5`

Pull request:
fix: td attrs override
resolve #244 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 62e6f3e..8943b39 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,21 @@
 
 ## Unreleased
 
+### Added
+
+- Allow passing a function to the `attrs` option of table component's `:action`
+  slot. Before, this was only supported in the `:col` slot.
+
+### Changed
+
+- Improve some error messages and documentation examples.
+
+### Fixed
+
+- In the `:col` and `:action` slots of the table component, the `attrs` option
+  did not properly override the attributes set with the `:tbody_td_attrs`
+  option.
+
 ## [0.21.0] - 2023-07-17
 
 ### Changed
diff --git a/lib/flop_phoenix/table.ex b/lib/flop_phoenix/table.ex
index 1ba3f68..58155f4 100644
--- a/lib/flop_phoenix/table.ex
+++ b/lib/flop_phoenix/table.ex
@@ -118,16 +118,14 @@ defmodule Flop.Phoenix.Table do
           <td
             :for={col <- @col}
             :if={show_column?(col)}
-            {@opts[:tbody_td_attrs]}
-            {maybe_invoke_options_callback(Map.get(col, :attrs, []), item)}
+            {merge_td_attrs(@opts[:tbody_td_attrs], col, item)}
             phx-click={@row_click && @row_click.(item)}
           >
             <%= render_slot(col, @row_item.(item)) %>
           </td>
           <td
             :for={action <- @action}
-            {@opts[:tbody_td_attrs]}
-            {Map.get(action, :attrs, [])}
+            {merge_td_attrs(@opts[:tbody_td_attrs], action, item)}
           >
             <%= render_slot(action, @row_item.(item)) %>
           </td>
@@ -138,8 +136,14 @@ defmodule Flop.Phoenix.Table do
     """
   end
 
-  defp maybe_invoke_options_callback(option, item) when is_function(option),
-    do: option.(item)
+  defp merge_td_attrs(tbody_td_attrs, col, item) do
+    attrs = col |> Map.get(:attrs, []) |> maybe_invoke_options_callback(item)
+    Keyword.merge(tbody_td_attrs, attrs)
+  end
+
+  defp maybe_invoke_options_callback(option, item) when is_function(option) do
+    option.(item)
+  end
 
   defp maybe_invoke_options_callback(option, _item), do: option
 


Test patch:
diff --git a/test/flop_phoenix_test.exs b/test/flop_phoenix_test.exs
index 030fed1..a0abb4b 100644
--- a/test/flop_phoenix_test.exs
+++ b/test/flop_phoenix_test.exs
@@ -1791,6 +1791,34 @@ defmodule Flop.PhoenixTest do
       assert [_] = Floki.find(html, "td.child")
     end
 
+    test "evaluates attrs function in action columns" do
+      assigns = %{
+        attrs_fun: fn item ->
+          [class: if(item.age > 17, do: "adult", else: "child")]
+        end
+      }
+
+      html =
+        ~H"""
+        <Flop.Phoenix.table
+          items={[
+            %{name: "Mary Cratsworth-Shane", age: 99},
+            %{name: "Bart Harley-Jarvis", age: 1}
+          ]}
+          meta={%Flop.Meta{flop: %Flop{}}}
+          on_sort={%JS{}}
+        >
+          <:col :let={u} label="Name"><%= u.name %></:col>
+          <:action label="Buttons" attrs={@attrs_fun}>some action</:action>
+        </Flop.Phoenix.table>
+        """
+        |> rendered_to_string()
+        |> Floki.parse_fragment!()
+
+      assert [_] = Floki.find(html, "td.adult")
+      assert [_] = Floki.find(html, "td.child")
+    end
+
     test "allows to set td class on action" do
       html =
         render_table(
@@ -1802,7 +1830,7 @@ defmodule Flop.PhoenixTest do
           &test_table_with_action/1
         )
 
-      assert [_, _, _, _, _, _] = Floki.find(html, "td.tolerance")
+      assert [_, _] = Floki.find(html, "td.tolerance")
     end
 
     test "adds additional attributes to td within action" do
@@ -1817,6 +1845,55 @@ defmodule Flop.PhoenixTest do
       assert [_, _] = Floki.find(html, "td.age-column")
     end
 
+    test "overrides table_td_attrs with td attrs" do
+      assigns = %{
+        meta: %Flop.Meta{flop: %Flop{}, schema: MyApp.Pet},
+        items: [%{name: "George", age: 8}],
+        opts: [tbody_td_attrs: [class: "default-td-class"]]
+      }
+
+      html =
+        ~H"""
+        <Flop.Phoenix.table items={@items} meta={@meta} on_sort={%JS{}} opts={@opts}>
+          <:col :let={i} attrs={[class: "name-td-class"]}><%= i.name %></:col>
+          <:col :let={i}><%= i.age %></:col>
+        </Flop.Phoenix.table>
+        """
+        |> rendered_to_string()
+        |> Floki.parse_fragment!()
+
+      assert [{"td", [{"class", "name-td-class"}], _}] =
+               Floki.find(html, "td:first-child")
+
+      assert [{"td", [{"class", "default-td-class"}], _}] =
+               Floki.find(html, "td:last-child")
+    end
+
+    test "overrides table_td_attrs with td attrs in action columns" do
+      assigns = %{
+        meta: %Flop.Meta{flop: %Flop{}, schema: MyApp.Pet},
+        items: [%{name: "George", age: 8}],
+        opts: [tbody_td_attrs: [class: "default-td-class"]]
+      }
+
+      html =
+        ~H"""
+        <Flop.Phoenix.table items={@items} meta={@meta} on_sort={%JS{}} opts={@opts}>
+          <:col :let={i}><%= i.name %></:col>
+          <:action attrs={[class: "action-1-td-class"]}>action 1</:action>
+          <:action>action 2</:action>
+        </Flop.Phoenix.table>
+        """
+        |> rendered_to_string()
+        |> Floki.parse_fragment!()
+
+      assert [{"td", [{"class", "action-1-td-class"}], _}] =
+               Floki.find(html, "td:nth-child(2)")
+
+      assert [{"td", [{"class", "default-td-class"}], _}] =
+               Floki.find(html, "td:last-child")
+    end
+
     test "doesn't render table if items list is empty" do
       assert [{"p", [], ["No results."]}] = render_table(items: [])
     end