Repository: woylie/flop_phoenix (elixir)

Issue:
only show sort indicator on first ordered column
To match the visual representation with the specification of the `aria-sort` attribute (which is already implemented correctly), the sort indicator (symbol, arrow) should only be rendered on the first column that is sorted, not on all columns that are sorted.

Pull request:
Order indicator first field only
resolves #220 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ab4ae97..726f1dc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,12 @@
 
 ## Unreleased
 
+### Changed
+
+- The table component only renders an ascending/descending order indicator in
+  the column header for the first order field, instead rendering one in the
+  column headers for all ordered fields.
+
 ## [0.19.0] - 2023-05-30
 
 ### Changed
diff --git a/lib/flop_phoenix/table.ex b/lib/flop_phoenix/table.ex
index b8bd280..f1f361e 100644
--- a/lib/flop_phoenix/table.ex
+++ b/lib/flop_phoenix/table.ex
@@ -215,20 +215,12 @@ defmodule Flop.Phoenix.Table do
   attr :opts, :any, required: true
 
   defp header_column(assigns) do
-    index = order_index(assigns.meta.flop, assigns.field)
-    direction = order_direction(assigns.meta.flop.order_directions, index)
-
-    assigns =
-      assigns
-      |> assign(:order_index, index)
-      |> assign(:order_direction, direction)
+    direction = order_direction(assigns.meta.flop, assigns.field)
+    assigns = assign(assigns, :order_direction, direction)
 
     ~H"""
     <%= if sortable?(@field, @meta.schema) do %>
-      <th
-        {@opts[:thead_th_attrs]}
-        aria-sort={aria_sort(@order_index, @order_direction)}
-      >
+      <th {@opts[:thead_th_attrs]} aria-sort={aria_sort(@order_direction)}>
         <span {@opts[:th_wrapper_attrs]}>
           <%= if @event do %>
             <.sort_link
@@ -258,30 +250,37 @@ defmodule Flop.Phoenix.Table do
     """
   end
 
-  defp aria_sort(0, direction), do: direction_to_aria(direction)
-  defp aria_sort(_, _), do: nil
-
-  defp direction_to_aria(:desc), do: "descending"
-  defp direction_to_aria(:desc_nulls_last), do: "descending"
-  defp direction_to_aria(:desc_nulls_first), do: "descending"
-  defp direction_to_aria(:asc), do: "ascending"
-  defp direction_to_aria(:asc_nulls_last), do: "ascending"
-  defp direction_to_aria(:asc_nulls_first), do: "ascending"
+  defp aria_sort(:desc), do: "descending"
+  defp aria_sort(:desc_nulls_last), do: "descending"
+  defp aria_sort(:desc_nulls_first), do: "descending"
+  defp aria_sort(:asc), do: "ascending"
+  defp aria_sort(:asc_nulls_last), do: "ascending"
+  defp aria_sort(:asc_nulls_first), do: "ascending"
+  defp aria_sort(_), do: nil
 
   attr :direction, :atom, required: true
   attr :opts, :list, required: true
 
   defp arrow(assigns) do
     ~H"""
-    <%= if @direction in [:asc, :asc_nulls_first, :asc_nulls_last] do %>
-      <span {@opts[:symbol_attrs]}><%= @opts[:symbol_asc] %></span>
-    <% end %>
-    <%= if @direction in [:desc, :desc_nulls_first, :desc_nulls_last] do %>
-      <span {@opts[:symbol_attrs]}><%= @opts[:symbol_desc] %></span>
-    <% end %>
-    <%= if is_nil(@direction) && !is_nil(@opts[:symbol_unsorted]) do %>
-      <span {@opts[:symbol_attrs]}><%= @opts[:symbol_unsorted] %></span>
-    <% end %>
+    <span
+      :if={@direction in [:asc, :asc_nulls_first, :asc_nulls_last]}
+      {@opts[:symbol_attrs]}
+    >
+      <%= @opts[:symbol_asc] %>
+    </span>
+    <span
+      :if={@direction in [:desc, :desc_nulls_first, :desc_nulls_last]}
+      {@opts[:symbol_attrs]}
+    >
+      <%= @opts[:symbol_desc] %>
+    </span>
+    <span
+      :if={is_nil(@direction) && !is_nil(@opts[:symbol_unsorted])}
+      {@opts[:symbol_attrs]}
+    >
+      <%= @opts[:symbol_unsorted] %>
+    </span>
     """
   end
 
@@ -298,15 +297,14 @@ defmodule Flop.Phoenix.Table do
     """
   end
 
-  defp order_index(%Flop{order_by: nil}, _), do: nil
-
-  defp order_index(%Flop{order_by: order_by}, field) do
-    Enum.find_index(order_by, &(&1 == field))
+  defp order_direction(
+         %Flop{order_by: [field | _], order_directions: [direction | _]},
+         field
+       ) do
+    direction
   end
 
-  defp order_direction(_, nil), do: nil
-  defp order_direction(nil, _), do: :asc
-  defp order_direction(directions, index), do: Enum.at(directions, index)
+  defp order_direction(%Flop{}, _), do: nil
 
   defp sortable?(nil, _), do: false
   defp sortable?(_, nil), do: true


Test patch:
diff --git a/test/flop_phoenix_test.exs b/test/flop_phoenix_test.exs
index 71e3beb..6e84975 100644
--- a/test/flop_phoenix_test.exs
+++ b/test/flop_phoenix_test.exs
@@ -1896,7 +1896,7 @@ defmodule Flop.PhoenixTest do
                  "th a:fl-contains('Email') + span.order-direction"
                )
 
-      assert Floki.text(span) == "▴"
+      assert span |> Floki.text() |> String.trim() == "▴"
 
       html =
         render_table(
@@ -1911,7 +1911,32 @@ defmodule Flop.PhoenixTest do
                  "th a:fl-contains('Email') + span.order-direction"
                )
 
-      assert Floki.text(span) == "▾"
+      assert span |> Floki.text() |> String.trim() == "▾"
+    end
+
+    test "only renders order direction symbol for first order field" do
+      html =
+        render_table(
+          meta: %Flop.Meta{
+            flop: %Flop{
+              order_by: [:name, :email],
+              order_directions: [:asc, :asc]
+            }
+          }
+        )
+
+      assert [span] =
+               Floki.find(
+                 html,
+                 "th a:fl-contains('Name') + span.order-direction"
+               )
+
+      assert span |> Floki.text() |> String.trim() == "▴"
+
+      assert Floki.find(
+               html,
+               "a:fl-contains('Email') + span.order-direction"
+             ) == []
     end
 
     test "allows to set symbol class" do
@@ -1936,7 +1961,7 @@ defmodule Flop.PhoenixTest do
         )
 
       assert [span] = Floki.find(html, "span.order-direction")
-      assert Floki.text(span) == "asc"
+      assert span |> Floki.text() |> String.trim() == "asc"
 
       html =
         render_table(
@@ -1947,7 +1972,7 @@ defmodule Flop.PhoenixTest do
         )
 
       assert [span] = Floki.find(html, "span.order-direction")
-      assert Floki.text(span) == "desc"
+      assert span |> Floki.text() |> String.trim() == "desc"
     end
 
     test "allows to set indicator for unsorted column" do
@@ -1965,7 +1990,7 @@ defmodule Flop.PhoenixTest do
                  "th a:fl-contains('Email') + span.order-direction"
                )
 
-      assert Floki.text(span) == "random"
+      assert span |> Floki.text() |> String.trim() == "random"
     end
 
     test "renders notice if item list is empty" do