diff --git a/assets/css/app.css b/assets/css/app.css index 37091909..7a828ff3 100644 --- a/assets/css/app.css +++ b/assets/css/app.css @@ -957,3 +957,16 @@ margin-bottom: 0; } +/* + * Sort-header tooltips must render on top of the pinned checkbox column's + * horizontal-scroll fade (::after on .sticky-first-col-th). The hovered/focused + * header th is already lifted above the checkbox th (hover/focus-within:z-50), + * so the whole tooltip wins the stacking; this rule additionally keeps the + * daisyUI tooltip bubble's own pseudo-elements above the fade overlay. Scoped + * to the member overview header so no unrelated tooltip is affected. + */ +#members-keyboard thead .tooltip::before, +#members-keyboard thead .tooltip::after { + z-index: 60; +} + diff --git a/assets/js/app.js b/assets/js/app.js index 85bbc6ba..91172948 100644 --- a/assets/js/app.js +++ b/assets/js/app.js @@ -336,6 +336,72 @@ Hooks.SortableList = { } } +// SortTooltip hook: shows the sort-header hint in the browser TOP LAYER via the +// native Popover API, so it escapes the members table's overflow clipping +// (overflow-x:auto forces overflow-y:auto, which clips a CSS tooltip below the +// header) and paints above the sticky header, neighbor cells, and the pinned +// checkbox column's fade — no z-index fight. The popover is shown on hover AND +// keyboard focus and positioned next to the header with getBoundingClientRect +// (portable; CSS anchor-positioning is not yet supported in Safari/Firefox). +Hooks.SortTooltip = { + mounted() { + this.tip = document.getElementById(this.el.dataset.tooltipId) + // Feature-detect: without Popover API support fall back to no visual tooltip + // (the button's aria-label still conveys the hint to assistive tech). + if (!this.tip || typeof this.tip.showPopover !== "function") return + + this.position = () => { + const btn = this.el.getBoundingClientRect() + const tip = this.tip.getBoundingClientRect() + const gap = 6 + // Prefer just below the header; flip above when there is no room below. + let top = btn.bottom + gap + if (top + tip.height > window.innerHeight && btn.top - gap - tip.height >= 0) { + top = btn.top - gap - tip.height + } + // Left-align to the header, but keep the whole bubble on-screen. + let left = btn.left + const maxLeft = window.innerWidth - tip.width - gap + if (left > maxLeft) left = maxLeft + if (left < gap) left = gap + this.tip.style.top = `${Math.round(top)}px` + this.tip.style.left = `${Math.round(left)}px` + } + + this.show = () => { + try { + if (!this.tip.matches(":popover-open")) this.tip.showPopover() + // showPopover() makes it measurable; positioning in the same tick avoids + // a visible flash (JS runs before the browser paints). + this.position() + } catch (_e) {} + } + + this.hide = () => { + try { + if (this.tip.matches(":popover-open")) this.tip.hidePopover() + } catch (_e) {} + } + + this.onKey = (e) => { if (e.key === "Escape") this.hide() } + + this.el.addEventListener("mouseenter", this.show) + this.el.addEventListener("focus", this.show) + this.el.addEventListener("mouseleave", this.hide) + this.el.addEventListener("blur", this.hide) + this.el.addEventListener("keydown", this.onKey) + }, + + destroyed() { + if (this.hide) this.hide() + this.el.removeEventListener("mouseenter", this.show) + this.el.removeEventListener("focus", this.show) + this.el.removeEventListener("mouseleave", this.hide) + this.el.removeEventListener("blur", this.hide) + this.el.removeEventListener("keydown", this.onKey) + } +} + // SidebarState hook: Manages sidebar expanded/collapsed state Hooks.SidebarState = { mounted() { diff --git a/lib/mv_web/components/core_components.ex b/lib/mv_web/components/core_components.ex index 2b4dcde5..76c44b1c 100644 --- a/lib/mv_web/components/core_components.ex +++ b/lib/mv_web/components/core_components.ex @@ -1061,7 +1061,17 @@ defmodule MvWeb.CoreComponents do > {col[:label]} - + <.live_component module={MvWeb.Components.SortHeaderComponent} id={:"sort_custom_field_#{dyn_col[:custom_field].id}"} @@ -1240,8 +1250,8 @@ defmodule MvWeb.CoreComponents do end # Combines column class with optional sticky header classes (desktop only; theme-friendly bg). - # hover:z-20/focus-within:z-20 lift the active header cell above sibling sticky cells (shared z-10) - # so its hover tooltip bubble is not clipped by an adjacent opaque bg-base-100 header background. + # bg-base-100 + z-10 keep the sticky header opaque and above body rows on vertical scroll. + # Sort tooltips use the Popover API (top layer) and need no header z-lift. defp table_th_class(col, sticky_header) do base = Map.get(col, :class) sticky = if sticky_header, do: sticky_th_classes(), else: nil @@ -1253,7 +1263,7 @@ defmodule MvWeb.CoreComponents do defp table_th_sticky_class(_), do: nil defp sticky_th_classes, - do: "lg:sticky lg:top-0 bg-base-100 hover:bg-base-100 z-10 hover:z-20 focus-within:z-20" + do: "lg:sticky lg:top-0 bg-base-100 z-10" @doc """ Renders a reorderable table (sortable list) with drag handle and keyboard support. diff --git a/lib/mv_web/live/components/sort_header_component.ex b/lib/mv_web/live/components/sort_header_component.ex index d3ab1e94..a3470a75 100644 --- a/lib/mv_web/live/components/sort_header_component.ex +++ b/lib/mv_web/live/components/sort_header_component.ex @@ -6,44 +6,57 @@ defmodule MvWeb.Components.SortHeaderComponent do - label: string() # Column Heading (can be an heex template) - sort_field: atom() | nil # current sort field from parent liveview - sort_order: :asc | :desc | nil # current sorting order + - sort_hint: string() | nil # optional; overrides the generic "click + # to sort" tooltip/aria-label for columns + # whose sort key is non-obvious (e.g. the + # Name column sorts by last name) """ use MvWeb, :live_component @impl true def update(assigns, socket) do - {:ok, assign(socket, assigns)} + {:ok, socket |> assign(assigns) |> assign_new(:sort_hint, fn -> nil end)} end # Check if we can add the aria-sort label directly to the daisyUI header # aria-sort={aria_sort(@field, @sort_field, @sort_order)} @impl true def render(assigns) do + assigns = + assigns + |> assign(:tooltip_id, "sort-tooltip-#{assigns.field}") + |> assign(:hint, assigns.sort_hint || gettext("Click to sort")) + ~H"""
- <.tooltip content={aria_sort(@field, @sort_field, @sort_order)} position="bottom"> - <.button - type="button" - variant="ghost" - aria-label={aria_sort(@field, @sort_field, @sort_order)} - class="select-none" - phx-click="sort" - phx-value-field={@field} - data-testid={@field} - > - {@label} - <%= if @sort_field == @field do %> - <.icon - name={if @sort_order == :asc, do: "hero-chevron-up", else: "hero-chevron-down"} - class="sort-icon" - /> - <% else %> - <.icon - name="hero-chevron-up-down" - class="sort-icon opacity-40" - /> - <% end %> - - + +
""" end @@ -51,13 +64,13 @@ defmodule MvWeb.Components.SortHeaderComponent do # ------------------------------------------------- # Hilfsfunktionen für ARIA Attribute & Icon SVG # ------------------------------------------------- - defp aria_sort(field, sort_field, dir) when field == sort_field do + defp aria_sort(field, sort_field, dir, hint) when field == sort_field do case dir do :asc -> gettext("ascending") :desc -> gettext("descending") - nil -> gettext("Click to sort") + nil -> hint end end - defp aria_sort(_, _, _), do: gettext("Click to sort") + defp aria_sort(_, _, _, hint), do: hint end diff --git a/lib/mv_web/live/member_live/index.ex b/lib/mv_web/live/member_live/index.ex index 714efe9a..deea80df 100644 --- a/lib/mv_web/live/member_live/index.ex +++ b/lib/mv_web/live/member_live/index.ex @@ -1450,7 +1450,9 @@ defmodule MvWeb.MemberLive.Index do defp valid_sort_field_db_or_custom?(field) when is_atom(field) do non_sortable_fields = [:notes] valid_fields = Mv.Constants.member_fields() -- non_sortable_fields - field in valid_fields or custom_field_sort?(field) or field in [:groups, :membership_fee_type] + + field in valid_fields or custom_field_sort?(field) or + field in [:groups, :membership_fee_type, :name, :address] end defp valid_sort_field_db_or_custom?(field) when is_binary(field) do @@ -1458,6 +1460,8 @@ defmodule MvWeb.MemberLive.Index do cond do field == "groups" -> :groups field == "membership_fee_type" -> :membership_fee_type + field == "name" -> :name + field == "address" -> :address true -> safe_member_field_atom_only(field) end @@ -1508,10 +1512,16 @@ defmodule MvWeb.MemberLive.Index do defp determine_field(default, ""), do: default defp determine_field(default, nil), do: default + # Computed/pseudo fields that are nonetheless sortable (they resolve to DB + # sort keys in OverviewQuery): groups aggregate and the composite Name/Address + # columns. Other computed fields (e.g. membership_fee_status) are not sortable. + @sortable_computed_fields [:groups, :name, :address] + defp determine_field(default, sf) when is_binary(sf) do - # Handle "groups" specially - it's in computed_member_fields() but can be sorted - if sf == "groups" do - :groups + sortable_strings = Enum.map(@sortable_computed_fields, &Atom.to_string/1) + + if sf in sortable_strings do + String.to_existing_atom(sf) else computed_strings = Enum.map(FieldVisibility.computed_member_fields(), &Atom.to_string/1) @@ -1522,9 +1532,8 @@ defmodule MvWeb.MemberLive.Index do end defp determine_field(default, sf) when is_atom(sf) do - # Handle :groups specially - it's in computed_member_fields() but can be sorted - if sf == :groups do - :groups + if sf in @sortable_computed_fields do + sf else if sf in FieldVisibility.computed_member_fields(), do: default, diff --git a/lib/mv_web/live/member_live/index.html.heex b/lib/mv_web/live/member_live/index.html.heex index 9c0db729..7709c2cb 100644 --- a/lib/mv_web/live/member_live/index.html.heex +++ b/lib/mv_web/live/member_live/index.html.heex @@ -5,6 +5,7 @@ <.live_component module={MvWeb.Components.BulkActionsDropdown} id="bulk-actions-dropdown" + open={@bulk_actions_open} export_payload_json={@export_payload_json} selected_count={@selected_count} scope={@scope} @@ -52,7 +53,9 @@ <.button type="button" variant="secondary" - class={["gap-2", @show_current_cycle && "btn-active"]} + class="gap-2" + active={@show_current_cycle} + aria-pressed={to_string(@show_current_cycle)} phx-click="toggle_cycle_view" data-testid="toggle-cycle-view" aria-label={ @@ -135,8 +138,6 @@ sort_order={@sort_order} size_class={if @density == :compact, do: "table-xs", else: "table-md"} > - - <:col :let={member} col_click={&MvWeb.MemberLive.Index.checkbox_column_click/1} @@ -285,10 +286,12 @@ [member.postal_code, member.city] |> Enum.reject(&(&1 in [nil, ""])) |> Enum.join(" ") %> -
-
{line1}
-
{line2}
-
+ <.maybe_value value={line1 <> line2} empty_sr_text={gettext("No address")}> +
+
{line1}
+
{line2}
+
+ <:col :let={member} @@ -307,7 +310,9 @@ """ } > - {member.street} + <.maybe_value value={member.street} empty_sr_text={gettext("Not specified")}> + {member.street} + <:col :let={member} @@ -326,7 +331,9 @@ """ } > - {member.house_number} + <.maybe_value value={member.house_number} empty_sr_text={gettext("Not specified")}> + {member.house_number} + <:col :let={member} @@ -345,7 +352,9 @@ """ } > - {member.postal_code} + <.maybe_value value={member.postal_code} empty_sr_text={gettext("Not specified")}> + {member.postal_code} + <:col :let={member} @@ -364,7 +373,9 @@ """ } > - {member.city} + <.maybe_value value={member.city} empty_sr_text={gettext("Not specified")}> + {member.city} + <:col :let={member} @@ -551,7 +562,7 @@ class="sticky left-0 flex items-center justify-center gap-2 text-base-content/70" data-testid="members-loading-row" > - + <.spinner size="sm" aria-hidden="true" /> {gettext("Loading more members …")} diff --git a/lib/mv_web/live/member_live/index/overview_query.ex b/lib/mv_web/live/member_live/index/overview_query.ex index bf4ef85c..ac3051f0 100644 --- a/lib/mv_web/live/member_live/index/overview_query.ex +++ b/lib/mv_web/live/member_live/index/overview_query.ex @@ -254,6 +254,22 @@ defmodule MvWeb.MemberLive.Index.OverviewQuery do defp apply_sort(query, nil, _order, _custom_fields), do: Ash.Query.sort(query, id: :asc) defp apply_sort(query, _field, nil, _custom_fields), do: Ash.Query.sort(query, id: :asc) + # Composite "Member" column: sort by last name then first name. The email that + # shares the cell is display-only and does not affect the sort key (§1.21). + defp apply_sort(query, field, order, _custom_fields) when field in [:name, "name"] do + Ash.Query.sort(query, [{:last_name, order}, {:first_name, order}, {:id, :asc}]) + end + + # Composite "Address" column: sort by city, then postal code, then street. + defp apply_sort(query, field, order, _custom_fields) when field in [:address, "address"] do + Ash.Query.sort(query, [ + {:city, order}, + {:postal_code, order}, + {:street, order}, + {:id, :asc} + ]) + end + defp apply_sort(query, field, order, custom_fields) do case sort_key(field) do nil -> apply_custom_field_sort(query, field, order, custom_fields) diff --git a/test/mv_web/member_live/index_sort_a11y_test.exs b/test/mv_web/member_live/index_sort_a11y_test.exs new file mode 100644 index 00000000..fa032ee3 --- /dev/null +++ b/test/mv_web/member_live/index_sort_a11y_test.exs @@ -0,0 +1,55 @@ +defmodule MvWeb.MemberLive.IndexSortA11yTest do + @moduledoc """ + §1.11 — When a column is sorted, exactly one `th` carries `aria-sort` with the + correct direction, a non-color sort glyph is present, and the sort is reflected + in the URL. + """ + use MvWeb.ConnCase, async: false + + import Phoenix.LiveViewTest + + alias Mv.Helpers.SystemActor + + @moduletag :ui + + setup %{conn: conn} do + {:ok, _} = + Mv.Membership.create_member( + %{first_name: "Sortable", last_name: "Member", email: "sortable@example.com"}, + actor: SystemActor.get_system_actor() + ) + + %{conn: conn_with_oidc_user(conn)} + end + + defp aria_sort_count(html) do + ~r/aria-sort="/ |> Regex.scan(html) |> length() + end + + test "exactly one th carries aria-sort ascending with a non-color glyph", %{conn: conn} do + {:ok, view, html} = live(conn, ~p"/members?sort_field=join_date&sort_order=asc") + + assert aria_sort_count(html) == 1 + assert has_element?(view, "th[aria-sort='ascending']") + # Non-color shape glyph for the active ascending sort. + assert html =~ "hero-chevron-up" + end + + test "aria-sort reflects descending and stays unique", %{conn: conn} do + {:ok, view, html} = live(conn, ~p"/members?sort_field=join_date&sort_order=desc") + + assert aria_sort_count(html) == 1 + assert has_element?(view, "th[aria-sort='descending']") + assert html =~ "hero-chevron-down" + end + + test "clicking a header reflects the sort in the URL", %{conn: conn} do + {:ok, view, _html} = live(conn, ~p"/members") + + view |> element("[data-testid='join_date']") |> render_click() + + path = assert_patch(view) + assert path =~ "sort_field=join_date" + assert path =~ "sort_order=asc" + end +end diff --git a/test/mv_web/member_live/index_sort_name_address_test.exs b/test/mv_web/member_live/index_sort_name_address_test.exs new file mode 100644 index 00000000..519cebc6 --- /dev/null +++ b/test/mv_web/member_live/index_sort_name_address_test.exs @@ -0,0 +1,106 @@ +defmodule MvWeb.MemberLive.IndexSortNameAddressTest do + @moduledoc """ + §1.21 — the composite Member (Name) and Address columns are sortable DB-side: + Name by last name → first name (email in the cell is display-only), Address by + city → postal code → street. Ascending/descending, reflected in the URL and in + `aria-sort` (§1.11). + """ + use MvWeb.ConnCase, async: false + + import Phoenix.LiveViewTest + + alias Mv.Helpers.SystemActor + alias MvWeb.MemberLive.Index.OverviewQuery + + setup %{conn: conn} do + actor = SystemActor.get_system_actor() + + # Same last name, different first name — proves the first-name tie-breaker. + {:ok, _} = + Mv.Membership.create_member( + %{ + first_name: "Bruno", + last_name: "Meyer", + email: "z-bruno@example.com", + city: "Aachen", + postal_code: "52062", + street: "Zebra" + }, + actor: actor + ) + + {:ok, _} = + Mv.Membership.create_member( + %{ + first_name: "Anna", + last_name: "Meyer", + email: "a-anna@example.com", + city: "Zwickau", + postal_code: "08056", + street: "Alpha" + }, + actor: actor + ) + + %{conn: conn_with_oidc_user(conn), actor: actor} + end + + defp read_names(query, actor) do + query + |> Ash.read!(actor: actor) + |> Enum.map(&{&1.last_name, &1.first_name}) + end + + test "name sort orders by last name then first name, ignoring email", %{actor: actor} do + asc = OverviewQuery.build(%{sort_field: :name, sort_order: :asc}) |> read_names(actor) + + # Both share last name "Meyer"; the first-name tie-breaker orders Anna before + # Bruno, even though Bruno's email sorts first alphabetically. + assert asc == [{"Meyer", "Anna"}, {"Meyer", "Bruno"}] + + desc = OverviewQuery.build(%{sort_field: :name, sort_order: :desc}) |> read_names(actor) + assert desc == [{"Meyer", "Bruno"}, {"Meyer", "Anna"}] + end + + test "address sort orders by city, then postal code, then street", %{actor: actor} do + asc = + OverviewQuery.build(%{sort_field: :address, sort_order: :asc}) + |> Ash.read!(actor: actor) + |> Enum.map(& &1.city) + + assert asc == ["Aachen", "Zwickau"] + + desc = + OverviewQuery.build(%{sort_field: :address, sort_order: :desc}) + |> Ash.read!(actor: actor) + |> Enum.map(& &1.city) + + assert desc == ["Zwickau", "Aachen"] + end + + test "clicking the Name header sorts and reflects state in URL and aria-sort", %{conn: conn} do + {:ok, view, _html} = live(conn, ~p"/members") + + view |> element("button[phx-click='sort'][data-testid='name']") |> render_click() + + path = assert_patch(view) + assert path =~ "sort_field=name" + # Exactly one column carries aria-sort (§1.11). + assert has_element?(view, "th[aria-sort='ascending']") + assert render(view) |> aria_sort_count() == 1 + end + + test "clicking the Address header sorts and reflects state in URL", %{conn: conn} do + {:ok, view, _html} = live(conn, ~p"/members") + + view |> element("button[phx-click='sort'][data-testid='address']") |> render_click() + + path = assert_patch(view) + assert path =~ "sort_field=address" + assert has_element?(view, "th[aria-sort='ascending']") + end + + defp aria_sort_count(html) do + html |> String.split("aria-sort=") |> length() |> Kernel.-(1) + end +end