input_pill: Identify pills by DOM nodes, not random IDs.

Signed-off-by: Anders Kaseorg <anders@zulip.com>

frontend_tests/node_tests/input_pill.js

@@ -161,6 +161,7 @@ function set_up() {
 run_test("copy from pill", ({override_rewire, mock_template}) => {
     mock_template("input_pill.hbs", true, (data, html) => {
         assert.ok(["BLUE", "RED"].includes(data.display_value));
+        $(html)[0] = `<pill-stub ${data.display_value}>`;
         return html;
     });
 
@@ -177,10 +178,7 @@ run_test("copy from pill", ({override_rewire, mock_template}) => {
     let copied_text;
 
     const $pill_stub = {
-        data: (field) => {
-            assert.equal(field, "id");
-            return "some_id2";
-        },
+        [0]: "<pill-stub RED>"
     };
 
     const e = {
@@ -408,6 +406,7 @@ run_test("insert_remove", ({override_rewire, mock_template}) => {
     mock_template("input_pill.hbs", true, (data, html) => {
         assert.equal(typeof data.display_value, "string");
         assert.ok(html.startsWith, "<div class='pill'");
+        $(html)[0] = `<pill-stub ${data.display_value}>`;
         return html;
     });
 
@@ -496,10 +495,7 @@ run_test("insert_remove", ({override_rewire, mock_template}) => {
 
     const $focus_pill_stub = {
         next: () => $next_pill_stub,
-        data: (field) => {
-            assert.equal(field, "id");
-            return "some_id1";
-        },
+        [0]: "<pill-stub BLUE>",
     };
 
     $container.set_find_results(".pill:focus", $focus_pill_stub);
@@ -518,6 +514,7 @@ run_test("exit button on pill", ({override_rewire, mock_template}) => {
     mock_template("input_pill.hbs", true, (data, html) => {
         assert.equal(typeof data.display_value, "string");
         assert.ok(html.startsWith, "<div class='pill'");
+        $(html)[0] = `<pill-stub ${data.display_value}>`;
         return html;
     });
 
@@ -549,10 +546,7 @@ run_test("exit button on pill", ({override_rewire, mock_template}) => {
 
     const $curr_pill_stub = {
         next: () => $next_pill_stub,
-        data: (field) => {
-            assert.equal(field, "id");
-            return "some_id1";
-        },
+        [0]: "<pill-stub BLUE>",
     };
 
     const exit_button_stub = {

frontend_tests/node_tests/search_future.js

@@ -17,7 +17,7 @@ const search_suggestion = mock_esm("../../static/js/search_suggestion");
 
 mock_esm("../../static/js/search_pill_widget", {
     widget: {
-        getByID: () => true,
+        getByElement: () => true,
     },
 });
 mock_esm("../../static/js/ui_util", {

static/js/input_pill.js

@@ -152,14 +152,14 @@ export function create(opts) {
             return true;
         },
 
-        // this searches given a particular pill ID for it, removes the node
+        // this searches given the DOM node for a pill, removes the node
         // from the DOM, removes it from the array and returns it.
         // this would generally be used for DOM-provoked actions, such as a user
         // clicking on a pill to remove it.
-        removePill(id) {
+        removePill(element) {
             let idx;
             for (let x = 0; x < store.pills.length; x += 1) {
-                if (store.pills[x].id === id) {
+                if (store.pills[x].$element[0] === element) {
                     idx = x;
                 }
             }
@@ -228,8 +228,8 @@ export function create(opts) {
             return drafts.length === 0;
         },
 
-        getByID(id) {
-            return store.pills.find((pill) => pill.id === id);
+        getByElement(element) {
+            return store.pills.find((pill) => pill.$element[0] === element);
         },
 
         _get_pills_for_testing() {
@@ -324,8 +324,7 @@ export function create(opts) {
                     break;
                 case "Backspace": {
                     const $next = $pill.next();
-                    const id = $pill.data("id");
-                    funcs.removePill(id);
+                    funcs.removePill($pill[0]);
                     $next.trigger("focus");
                     // the "Backspace" key in Firefox will go back a page if you do
                     // not prevent it.
@@ -363,9 +362,8 @@ export function create(opts) {
             e.stopPropagation();
             const $pill = $(this).closest(".pill");
             const $next = $pill.next();
-            const id = $pill.data("id");
 
-            funcs.removePill(id);
+            funcs.removePill($pill[0]);
             $next.trigger("focus");
 
             compose.update_fade();
@@ -378,8 +376,8 @@ export function create(opts) {
         });
 
         store.$parent.on("copy", ".pill", (e) => {
-            const id = store.$parent.find(":focus").data("id");
-            const data = funcs.getByID(id);
+            const $element = store.$parent.find(":focus");
+            const data = funcs.getByElement($element[0]);
             e.originalEvent.clipboardData.setData(
                 "text/plain",
                 store.get_text_from_item(data.item),
@@ -393,7 +391,7 @@ export function create(opts) {
         appendValue: funcs.appendPill.bind(funcs),
         appendValidatedData: funcs.appendValidatedData.bind(funcs),
 
-        getByID: funcs.getByID,
+        getByElement: funcs.getByElement,
         items: funcs.items,
 
         onPillCreate(callback) {

static/js/search.js

@@ -184,8 +184,8 @@ export function initialize() {
         // really it would be OK if they did).
 
         if (page_params.search_pills_enabled) {
-            const pill_id = $(e.relatedTarget).closest(".pill").data("id");
-            const search_pill = search_pill_widget.widget.getByID(pill_id);
+            const $element = $(e.relatedTarget).closest(".pill");
+            const search_pill = search_pill_widget.widget.getByElement($element[0]);
             if (search_pill) {
                 // The searchbox loses focus while the search
                 // pill element gains focus.