search: Move group user remove logic out of input_pill.

web/src/input_pill.ts

@@ -6,7 +6,6 @@ import assert from "minimalistic-assert";
 import render_input_pill from "../templates/input_pill.hbs";
 
 import * as keydown_util from "./keydown_util";
-import type {SearchUserPill} from "./search_pill";
 import * as ui_util from "./ui_util";
 
 // See https://zulip.readthedocs.io/en/latest/subsystems/input-pills.html
@@ -28,9 +27,14 @@ type InputPillCreateOptions<ItemType> = {
     get_text_from_item: (item: ItemType) => string;
     get_display_value_from_item: (item: ItemType) => string;
     generate_pill_html?: (item: ItemType) => string;
+    on_pill_exit?: (
+        clicked_pill: HTMLElement,
+        all_pills: InputPill<ItemType>[],
+        remove_pill: (pill: HTMLElement) => void,
+    ) => void;
 };
 
-type InputPill<ItemType> = {
+export type InputPill<ItemType> = {
     item: ItemType;
     $element: JQuery;
 };
@@ -45,6 +49,7 @@ type InputPillStore<ItemType> = {
     get_text_from_item: InputPillCreateOptions<ItemType>["get_text_from_item"];
     get_display_value_from_item: InputPillCreateOptions<ItemType>["get_display_value_from_item"];
     generate_pill_html: InputPillCreateOptions<ItemType>["generate_pill_html"];
+    on_pill_exit: InputPillCreateOptions<ItemType>["on_pill_exit"];
     onPillCreate?: () => void;
     onPillRemove?: (pill: InputPill<ItemType>, trigger: RemovePillTrigger) => void;
     createPillonPaste?: () => void;
@@ -89,6 +94,7 @@ export function create<ItemType extends {type: string}>(
         split_text_on_comma: opts.split_text_on_comma ?? true,
         convert_to_pill_on_enter: opts.convert_to_pill_on_enter ?? true,
         generate_pill_html: opts.generate_pill_html,
+        on_pill_exit: opts.on_pill_exit,
     };
 
     // a dictionary of internal functions. Some of these are exposed as well,
@@ -205,52 +211,6 @@ export function create<ItemType extends {type: string}>(
             return undefined;
         },
 
-        // TODO: This function is only used for the search input supporting multiple user
-        // pills within an individual top-level pill. Ideally, we'd encapsulate it in a
-        // subclass used only for search so that this code can be part of search_pill.ts.
-        removeUserPill(user_container: HTMLElement, user_id: number, trigger: RemovePillTrigger) {
-            // First get the outer pill that contains the user pills.
-            let container_idx: number | undefined;
-            for (let x = 0; x < store.pills.length; x += 1) {
-                if (store.pills[x]!.$element[0] === user_container) {
-                    container_idx = x;
-                }
-            }
-            assert(container_idx !== undefined);
-            assert(store.pills[container_idx]!.item.type === "search_user");
-            // TODO: Figure out how to get this typed correctly.
-            // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
-            const user_pill_container = store.pills[container_idx]!
-                .item as unknown as SearchUserPill;
-
-            // If there's only one user in this pill, delete the whole pill.
-            if (user_pill_container.users.length === 1) {
-                assert(user_pill_container.users[0]!.user_id === user_id);
-                this.removePill(user_container, trigger);
-                return;
-            }
-
-            // Remove the user id from the pill data.
-            let user_idx: number | undefined;
-            for (let x = 0; x < user_pill_container.users.length; x += 1) {
-                if (user_pill_container.users[x]!.user_id === user_id) {
-                    user_idx = x;
-                }
-            }
-            assert(user_idx !== undefined);
-            user_pill_container.users.splice(user_idx, 1);
-
-            // Remove the user pill from the DOM.
-            const $user_pill = $(store.pills[container_idx]!.$element.children(".pill")[user_idx]!);
-            assert($user_pill.data("user-id") === user_id);
-            $user_pill.remove();
-
-            // This is needed to run the "change" event handler registered in
-            // compose_recipient.js, which calls the `update_on_recipient_change` to update
-            // the compose_fade state.
-            store.$input.trigger("change");
-        },
-
         // this will remove the last pill in the container -- by default tied
         // to the "Backspace" key when the value of the input is empty.
         // If quiet is a truthy value, the event handler associated with the
@@ -446,20 +406,14 @@ export function create<ItemType extends {type: string}>(
         // when the "×" is clicked on a pill, it should delete that pill and then
         // select the input field.
         store.$parent.on("click", ".exit", function (this: HTMLElement, e) {
-            const $user_pill_container = $(this).parents(".user-pill-container");
+            if (store.on_pill_exit) {
-            if ($user_pill_container.length) {
+                store.on_pill_exit(this, store.pills, (pill: HTMLElement): void => {
-                // The user-pill-container container class is used exclusively for
+                    funcs.removePill(pill, "close");
-                // group-DM search pills, where multiple user pills sit inside a larger
+                });
-                // pill. The exit icons in those individual user pills should remove
+                // This is needed to run the "change" event handler registered in
-                // just that pill, not the outer pill.
+                // compose_recipient.js, which calls the `update_on_recipient_change` to update
-                // TODO: Figure out how to move this code into search_pill.ts.
+                // the compose_fade state.
-                const user_id = $(this).closest(".pill").attr("data-user-id");
+                store.$input.trigger("change");
-                assert(user_id !== undefined);
-                funcs.removeUserPill(
-                    $user_pill_container[0]!,
-                    Number.parseInt(user_id, 10),
-                    "close",
-                );
             } else {
                 e.stopPropagation();
                 const $pill = $(this).closest(".pill");

web/src/search_pill.ts

@@ -6,7 +6,7 @@ import render_search_user_pill from "../templates/search_user_pill.hbs";
 
 import {Filter} from "./filter";
 import * as input_pill from "./input_pill";
-import type {InputPillContainer} from "./input_pill";
+import type {InputPill, InputPillContainer} from "./input_pill";
 import * as people from "./people";
 import type {User} from "./people";
 import type {NarrowTerm} from "./state_data";
@@ -60,6 +60,54 @@ export function get_search_string_from_item(item: SearchPill): string {
     return `${sign}${item.operator}: ${get_search_operand(item)}`;
 }
 
+// This is called when the a pill is closed. We have custom logic here
+// because group user pills have pills inside of them, and it's possible
+// to e.g. remove a user from a group-DM pill without deleting the whole
+// DM pill.
+function on_pill_exit(
+    clicked_pill: HTMLElement,
+    all_pills: InputPill<SearchPill>[],
+    remove_pill: (pill: HTMLElement) => void,
+): void {
+    const $user_pill_container = $(clicked_pill).parents(".user-pill-container");
+    if (!$user_pill_container.length) {
+        // This is just a regular search pill, so we don't need to do fancy logic.
+        remove_pill(clicked_pill);
+        return;
+    }
+    // The user-pill-container container class is used exclusively for
+    // group-DM search pills, where multiple user pills sit inside a larger
+    // pill. The exit icons in those individual user pills should remove
+    // just that pill, not the outer pill.
+    const user_id_string = $(clicked_pill).closest(".pill").attr("data-user-id");
+    assert(user_id_string !== undefined);
+    const user_id = Number.parseInt(user_id_string, 10);
+
+    // First get the outer pill that contains the user pills.
+    const outer_idx = all_pills.findIndex((pill) => pill.$element[0] === $user_pill_container[0]);
+    assert(outer_idx !== -1);
+    const user_container_pill = all_pills[outer_idx]!.item;
+    assert(user_container_pill?.type === "search_user");
+
+    // If there's only one user in this pill, delete the whole pill.
+    if (user_container_pill.users.length === 1) {
+        assert(user_container_pill.users[0]!.user_id === user_id);
+        remove_pill($user_pill_container[0]!);
+        return;
+    }
+
+    // Remove the user id from the pill data.
+    const user_idx = user_container_pill.users.findIndex((user) => user.user_id === user_id);
+    assert(user_idx !== -1);
+    user_container_pill.users.splice(user_idx, 1);
+
+    // Remove the user pill from the DOM.
+    const $outer_container = all_pills[outer_idx]!.$element;
+    const $user_pill = $($outer_container.children(".pill")[user_idx]!);
+    assert($user_pill.attr("data-user-id") === user_id.toString());
+    $user_pill.remove();
+}
+
 export function create_pills($pill_container: JQuery): SearchPillWidget {
     const pills = input_pill.create({
         $container: $pill_container,
@@ -77,6 +125,7 @@ export function create_pills($pill_container: JQuery): SearchPillWidget {
             });
         },
         get_display_value_from_item: get_search_string_from_item,
+        on_pill_exit,
     });
     // We don't automatically create pills on paste. When the user
     // presses enter, we validate the input then.

web/tests/input_pill.test.js

@@ -485,9 +485,6 @@ run_test("exit button on pill", ({mock_template}) => {
                 assert.equal(sel, ".pill");
                 return $curr_pill_stub;
             },
-            parents() {
-                return [];
-            },
         }),
     };