fix: pr comments

docs/groups-architecture.md

@@ -314,17 +314,18 @@ lib/
 - Display group name and description
 - List all members in group
 - Link to member detail pages
-- Add members to group (via modal with search/autocomplete)
+- Add members to group (via inline combobox with search/autocomplete)
 - Remove members from group (via remove button per member)
 - Edit group button (navigates to `/groups/:slug/edit`)
 - Delete group button (with confirmation modal)
 
 **Add Member Functionality:**
 - "Add Member" button displayed above member table (only for users with `:update` permission)
-- Opens modal with member search/autocomplete
+- Opens inline add member area with member search/autocomplete (combobox)
 - Search filters out members already in the group
 - Selecting a member adds them to the group immediately
 - Success/error flash messages provide feedback
+- "Cancel" button closes the inline add member area without adding
 
 **Remove Member Functionality:**
 - "Remove" button (icon button) for each member in table (only for users with `:update` permission)
@@ -818,7 +819,7 @@ Each functional unit can be implemented as a **separate issue**:
 
 **Tasks:**
 1. Add "Add Member" button above member table in Group Detail View
-2. Implement modal with member search/autocomplete
+2. Implement inline add member with search/autocomplete
 3. Add "Remove" button for each member in table
 4. Implement add/remove functionality with flash messages
 5. Ensure proper authorization checks
@@ -1001,7 +1002,7 @@ Each functional unit can be implemented as a **separate issue**:
 
 **Tasks:**
 - Add "Add Member" button above member table in Group Detail View (`/groups/:slug`)
-- Implement modal for member selection with search/autocomplete
+- Implement inline add member for member selection with search/autocomplete
 - Add "Remove" button for each member in the member table
 - Implement add member functionality (create MemberGroup association)
 - Implement remove member functionality (destroy MemberGroup association)
@@ -1012,7 +1013,7 @@ Each functional unit can be implemented as a **separate issue**:
 
 **Acceptance Criteria:**
 - "Add Member" button is visible above member table (only for users with `:update` permission)
-- Clicking "Add Member" opens a modal with member search/autocomplete
+- Clicking "Add Member" opens inline add member area with member search/autocomplete
 - Search filters members and excludes those already in the group
 - Selecting a member from search adds them to the group
 - Success flash message is displayed when member is added
@@ -1021,7 +1022,7 @@ Each functional unit can be implemented as a **separate issue**:
 - Clicking "Remove" immediately removes the member from the group (no confirmation dialog)
 - Success flash message is displayed when member is removed
 - Group member list and member count update automatically after add/remove
-- Modal closes after successful member addition
+- Inline add member area closes after successful member addition
 - Authorization is enforced server-side in event handlers
 - UI respects permission checks (buttons hidden for unauthorized users)
 
@@ -1031,15 +1032,15 @@ Each functional unit can be implemented as a **separate issue**:
 - Use `Membership.destroy_member_group/1` for removing members
 - Filter search results to exclude members already in the group (check `group.members`)
 - Reload group with `:members` and `:member_count` after operations
-- Follow existing modal patterns (similar to delete confirmation modal)
+- Use inline combobox pattern (delete group uses a separate confirmation modal)
 - Ensure accessibility: proper ARIA labels, keyboard navigation, focus management
 
 **UI/UX Details:**
-- Modal title: "Add Member to Group"
+- Inline add member section (no modal; combobox above member table)
 - Search input placeholder: "Search for a member..."
 - Search results show member name and email
-- "Add" button in modal (disabled until member selected)
+- "Add" button in inline area (disabled until member selected)
-- "Cancel" button to close modal
+- "Cancel" button to close inline add member area
 - Remove button can be an icon button (trash icon) with tooltip
 - Flash messages: "Member added successfully" / "Member removed successfully" / error messages
 

lib/mv_web/live/group_live/show.ex

@@ -15,6 +15,8 @@ defmodule MvWeb.GroupLive.Show do
   """
   use MvWeb, :live_view
 
+  require Logger
+
   import MvWeb.LiveHelpers, only: [current_actor: 1]
   import MvWeb.Authorization
 
@@ -162,7 +164,7 @@ defmodule MvWeb.GroupLive.Show do
                             phx-hook="ComboBox"
                             phx-focus="show_member_dropdown"
                             phx-debounce="300"
-                            phx-window-keydown="member_dropdown_keydown"
+                            phx-keydown="member_dropdown_keydown"
                             phx-mounted={JS.focus()}
                             value={@member_search_query}
                             placeholder={
@@ -231,6 +233,14 @@ defmodule MvWeb.GroupLive.Show do
                     >
                       <.icon name="hero-plus" class="size-5" />
                     </button>
+                    <button
+                      type="button"
+                      class="btn join-item"
+                      phx-click="hide_add_member_input"
+                      aria-label={gettext("Cancel")}
+                    >
+                      {gettext("Cancel")}
+                    </button>
                   </div>
                 <% else %>
                   <.button
@@ -439,12 +449,7 @@ defmodule MvWeb.GroupLive.Show do
 
   @impl true
   def handle_event("show_member_dropdown", _params, socket) do
-    # Reload group to ensure we have the latest members list before filtering
+    # Use existing group.members for filtering; reload only on add/remove
-    actor = current_actor(socket)
-    group = socket.assigns.group
-    socket = reload_group(socket, group.slug, actor)
-
-    # Load available members with empty query when input is focused
     socket =
       socket
       |> load_available_members("")
@@ -454,6 +459,19 @@ defmodule MvWeb.GroupLive.Show do
     {:noreply, socket}
   end
 
+  @impl true
+  def handle_event("hide_add_member_input", _params, socket) do
+    {:noreply,
+     socket
+     |> assign(:show_add_member_input, false)
+     |> assign(:member_search_query, "")
+     |> assign(:available_members, [])
+     |> assign(:selected_member_ids, [])
+     |> assign(:selected_members, [])
+     |> assign(:show_member_dropdown, false)
+     |> assign(:focused_member_index, nil)}
+  end
+
   @impl true
   def handle_event("hide_member_dropdown", _params, socket) do
     {:noreply, assign(socket, show_member_dropdown: false, focused_member_index: nil)}
@@ -514,11 +532,7 @@ defmodule MvWeb.GroupLive.Show do
 
   @impl true
   def handle_event("search_members", %{"member_search" => query}, socket) do
-    # Reload group to ensure we have the latest members list before filtering
+    # Use existing group.members for filtering; reload only on add/remove
-    actor = current_actor(socket)
-    group = socket.assigns.group
-    socket = reload_group(socket, group.slug, actor)
-
     socket =
       socket
       |> assign(:member_search_query, query)
@@ -574,7 +588,8 @@ defmodule MvWeb.GroupLive.Show do
 
     # Server-side authorization check
     if can?(actor, :update, group) do
-      perform_add_members(socket, group, socket.assigns.selected_member_ids, actor)
+      member_ids = Enum.uniq(socket.assigns.selected_member_ids)
+      perform_add_members(socket, group, member_ids, actor)
     else
       {:noreply,
        socket
@@ -648,23 +663,29 @@ defmodule MvWeb.GroupLive.Show do
   defp load_available_members(socket, query) do
     require Ash.Query
 
+    current_member_ids = group_member_ids_set(socket.assigns.group)
     base_query = available_members_base_query(query)
-    limited_query = Ash.Query.limit(base_query, 10)
+
+    # Fetch more than 10, then exclude already-in-group and take 10 (avoids empty dropdown when first N are all in group)
+    fetch_limit = 50
+    limited_query = Ash.Query.limit(base_query, fetch_limit)
     actor = current_actor(socket)
 
     case Ash.read(limited_query, actor: actor, domain: Mv.Membership) do
       {:ok, members} ->
-        current_member_ids = group_member_ids_set(socket.assigns.group)
+        available =
+          members
+          |> Enum.reject(fn m -> MapSet.member?(current_member_ids, m.id) end)
+          |> Enum.take(10)
 
-        filtered_members =
+        assign(socket, available_members: available)
-          Enum.reject(members, fn member ->
-            MapSet.member?(current_member_ids, member.id)
-          end)
 
-        assign(socket, available_members: filtered_members)
+      {:error, error} ->
+        Logger.warning("Failed to load available members for group: #{inspect(error)}")
 
-      {:error, _} ->
+        socket
-        assign(socket, available_members: [])
+        |> put_flash(:error, gettext("Could not load member search. Please try again."))
+        |> assign(:available_members, [])
     end
   end
 
@@ -681,18 +702,8 @@ defmodule MvWeb.GroupLive.Show do
   end
 
   defp group_member_ids_set(group) do
-    cond do
+    members = group.members || []
-      is_list(group.members) and group.members != [] ->
+    members |> Enum.map(& &1.id) |> MapSet.new()
-        group.members
-        |> Enum.map(& &1.id)
-        |> MapSet.new()
-
-      is_list(group.members) ->
-        MapSet.new()
-
-      true ->
-        MapSet.new()
-    end
   end
 
   defp perform_add_members(socket, group, member_ids, actor) when is_list(member_ids) do

priv/gettext/de/LC_MESSAGES/default.po

@@ -2260,6 +2260,11 @@ msgstr "Nicht berechtigt."
 msgid "Could not load data fields. Please check your permissions."
 msgstr "Datenfelder konnten nicht geladen werden. Bitte überprüfen Sie Ihre Berechtigungen."
 
+#: lib/mv_web/live/group_live/show.ex
+#, elixir-autogen, elixir-format
+msgid "Could not load member search. Please try again."
+msgstr "Mitgliedersuche konnte nicht geladen werden. Bitte versuchen Sie es erneut."
+
 #: lib/mv_web/live/group_live/show.ex
 #, elixir-autogen, elixir-format
 msgid "Add Member"

priv/gettext/default.pot

@@ -2261,6 +2261,11 @@ msgstr ""
 msgid "Could not load data fields. Please check your permissions."
 msgstr ""
 
+#: lib/mv_web/live/group_live/show.ex
+#, elixir-autogen, elixir-format
+msgid "Could not load member search. Please try again."
+msgstr ""
+
 #: lib/mv_web/live/group_live/show.ex
 #, elixir-autogen, elixir-format
 msgid "Add Member"

priv/gettext/en/LC_MESSAGES/default.po

@@ -2259,6 +2259,11 @@ msgstr ""
 msgid "Could not load data fields. Please check your permissions."
 msgstr ""
 
+#: lib/mv_web/live/group_live/show.ex
+#, elixir-autogen, elixir-format
+msgid "Could not load member search. Please try again."
+msgstr ""
+
 #: lib/mv_web/live/global_settings_live.ex
 #, elixir-autogen, elixir-format, fuzzy
 msgid "CSV files only, maximum %{size} MB"

test/mv_web/live/group_live/show_accessibility_test.exs

@@ -93,7 +93,7 @@ defmodule MvWeb.GroupLive.ShowAccessibilityTest do
   end
 
   describe "keyboard navigation" do
-    test "tab navigation works in modal", %{conn: conn} do
+    test "tab navigation works in inline add member area", %{conn: conn} do
       # This test verifies that keyboard navigation is possible
       # Actual tab order testing would require more complex setup
       group = Fixtures.group_fixture()
@@ -107,7 +107,7 @@ defmodule MvWeb.GroupLive.ShowAccessibilityTest do
 
       html = render(view)
 
-      # Modal should have focusable elements
+      # Inline add member area should have focusable elements
       assert html =~ ~r/input|button/ ||
                html =~ "#member-search-input"
     end

test/mv_web/live/group_live/show_add_member_test.exs

@@ -1,11 +1,12 @@
 defmodule MvWeb.GroupLive.ShowAddMemberTest do
   @moduledoc """
-  Tests for adding members to groups via the Add Member modal.
+  Tests for adding members to groups via the inline Add Member combobox.
   Tests successful add, error handling, and edge cases.
   """
 
   use MvWeb.ConnCase, async: false
   import Phoenix.LiveViewTest
+  import MvWeb.GroupLiveHelpers
   use Gettext, backend: MvWeb.Gettext
 
   alias Mv.Membership
@@ -28,32 +29,11 @@ defmodule MvWeb.GroupLive.ShowAddMemberTest do
 
       {:ok, view, _html} = live(conn, "/groups/#{group.slug}")
 
-      # Open inline input
+      open_add_member(view)
-      view
+      search_member(view, "Alice")
-      |> element("button", "Add Member")
+      select_member(view, member)
-      |> render_click()
+      add_selected(view)
 
-      # Search and select member
-      view
-      |> element("#member-search-input")
-      |> render_focus()
-
-      # phx-change is on the form, so we need to trigger it via the form
-      view
-      |> element("form[phx-change='search_members']")
-      |> render_change(%{"member_search" => "Alice"})
-
-      # Select member
-      view
-      |> element("[data-member-id='#{member.id}']")
-      |> render_click()
-
-      # Click Add button
-      view
-      |> element("button[phx-click='add_selected_members']")
-      |> render_click()
-
-      # Verify member appears in group list (no success flash message)
       html = render(view)
       assert html =~ "Alice"
       assert html =~ "Johnson"
@@ -198,7 +178,7 @@ defmodule MvWeb.GroupLive.ShowAddMemberTest do
       assert new_count == initial_count + 1
     end
 
-    test "modal closes after successful member addition", %{conn: conn} do
+    test "inline add member area closes after successful member addition", %{conn: conn} do
       system_actor = Mv.Helpers.SystemActor.get_system_actor()
       group = Fixtures.group_fixture()
 
@@ -242,6 +222,21 @@ defmodule MvWeb.GroupLive.ShowAddMemberTest do
       # Inline input should be closed (Add Member button should be visible again)
       refute has_element?(view, "#member-search-input")
     end
+
+    test "Cancel button closes inline add member area without adding", %{conn: conn} do
+      group = Fixtures.group_fixture()
+
+      {:ok, view, _html} = live(conn, "/groups/#{group.slug}")
+
+      open_add_member(view)
+      assert has_element?(view, "#member-search-input")
+      assert has_element?(view, "button[phx-click='hide_add_member_input']")
+
+      cancel_add_member(view)
+
+      refute has_element?(view, "#member-search-input")
+      assert has_element?(view, "button", "Add Member")
+    end
   end
 
   describe "error handling" do

test/mv_web/live/group_live/show_add_remove_members_test.exs

@@ -1,6 +1,6 @@
 defmodule MvWeb.GroupLive.ShowAddRemoveMembersTest do
   @moduledoc """
-  UI tests for Add/Remove Member buttons visibility and modal display.
+  UI tests for Add/Remove Member buttons visibility and inline add member display.
   Tests UI rendering and permission-based visibility.
   """
 

test/mv_web/live/group_live/show_authorization_test.exs

@@ -217,7 +217,7 @@ defmodule MvWeb.GroupLive.ShowAuthorizationTest do
     end
 
     @tag role: :read_only
-    test "modal cannot be opened for unauthorized users", %{conn: conn} do
+    test "inline add member area cannot be opened for unauthorized users", %{conn: conn} do
       group = Fixtures.group_fixture()
 
       {:ok, _view, html} = live(conn, "/groups/#{group.slug}")
@@ -262,14 +262,14 @@ defmodule MvWeb.GroupLive.ShowAuthorizationTest do
                match?({:error, {:live_redirect, %{to: "/groups"}}}, result)
     end
 
+    @tag :skip
     test "non-existent member IDs are handled", %{conn: conn} do
+      # Future: test add_selected_members with invalid ID (would require pushing event with forged selected_member_ids)
       group = Fixtures.group_fixture()
 
-      {:ok, _view, _html} = live(conn, "/groups/#{group.slug}")
+      {:ok, view, _html} = live(conn, "/groups/#{group.slug}")
 
-      # Try to add non-existent member (if possible)
+      assert has_element?(view, "button", "Add Member")
-      # Implementation should handle this gracefully
-      # This tests error handling for invalid IDs
     end
 
     test "non-existent group IDs are handled", %{conn: conn} do

test/mv_web/live/group_live/show_member_search_test.exs

@@ -1,6 +1,6 @@
 defmodule MvWeb.GroupLive.ShowMemberSearchTest do
   @moduledoc """
-  UI tests for member search functionality in Add Member modal.
+  UI tests for member search functionality in inline Add Member combobox.
   Tests search behavior and filtering of members already in group.
   """
 

test/support/group_live_helpers.ex

@@ -0,0 +1,59 @@
+defmodule MvWeb.GroupLiveHelpers do
+  @moduledoc """
+  Helpers for Group LiveView tests (e.g. group show add/remove member flow).
+
+  Use these to reduce duplication in tests that open the add member area,
+  search, select, and add members.
+  """
+
+  import Phoenix.LiveViewTest
+
+  @doc """
+  Opens the inline add member area by clicking "Add Member".
+  """
+  def open_add_member(view) do
+    view
+    |> element("button", "Add Member")
+    |> render_click()
+  end
+
+  @doc """
+  Triggers member search by focusing the input and sending a form change with the given query.
+  """
+  def search_member(view, query) do
+    view
+    |> element("#member-search-input")
+    |> render_focus()
+
+    view
+    |> element("form[phx-change='search_members']")
+    |> render_change(%{"member_search" => query})
+  end
+
+  @doc """
+  Clicks the option for the given member in the dropdown (by data-member-id).
+  """
+  def select_member(view, member) do
+    view
+    |> element("[data-member-id='#{member.id}']")
+    |> render_click()
+  end
+
+  @doc """
+  Clicks the "Add" button (add_selected_members).
+  """
+  def add_selected(view) do
+    view
+    |> element("button[phx-click='add_selected_members']")
+    |> render_click()
+  end
+
+  @doc """
+  Clicks the "Cancel" button to close the inline add member area.
+  """
+  def cancel_add_member(view) do
+    view
+    |> element("button[phx-click='hide_add_member_input']")
+    |> render_click()
+  end
+end