search_suggestion: Show pills in search suggestions.

Fixes #34750.

web/e2e-tests/message-basics.test.ts

@@ -209,7 +209,7 @@ async function search_tests(page: Page): Promise<void> {
     await search_and_check(
         page,
         "Verona",
-        "Channel",
+        "#Verona",
         expect_verona_stream,
         "#Verona - Zulip Dev - Zulip",
     );
@@ -217,7 +217,7 @@ async function search_tests(page: Page): Promise<void> {
     await search_and_check(
         page,
         "Cordelia",
-        "Direct",
+        "dm:",
         expect_cordelia_direct_messages,
         "Cordelia, Lear's daughter - Zulip Dev - Zulip",
     );

web/src/search.ts

@@ -1,8 +1,6 @@
 import $ from "jquery";
 import assert from "minimalistic-assert";
 
-import render_search_list_item from "../templates/search_list_item.hbs";
-
 import {Typeahead} from "./bootstrap_typeahead.ts";
 import type {TypeaheadInputElement} from "./bootstrap_typeahead.ts";
 import {Filter} from "./filter.ts";
@@ -195,7 +193,8 @@ export function initialize(opts: {on_narrow_search: OnNarrowSearch}): void {
         requireHighlight: false,
         item_html(item: string): string {
             const obj = search_map.get(item);
-            return render_search_list_item(obj);
+            assert(obj !== undefined);
+            return search_pill.generate_pills_html(obj);
         },
         // When the user starts typing new search operands,
         // we want to highlight the first typeahead row by default

web/src/search_pill.ts

@@ -2,6 +2,7 @@ import $ from "jquery";
 import assert from "minimalistic-assert";
 
 import render_input_pill from "../templates/input_pill.hbs";
+import render_search_list_item from "../templates/search_list_item.hbs";
 import render_search_user_pill from "../templates/search_user_pill.hbs";
 
 import {Filter} from "./filter.ts";
@@ -9,6 +10,7 @@ import * as input_pill from "./input_pill.ts";
 import type {InputPill, InputPillContainer} from "./input_pill.ts";
 import * as people from "./people.ts";
 import type {User} from "./people.ts";
+import {type Suggestion, search_term_description_html} from "./search_suggestion.ts";
 import type {NarrowTerm} from "./state_data.ts";
 import * as stream_data from "./stream_data.ts";
 import * as user_status from "./user_status.ts";
@@ -112,6 +114,73 @@ function on_pill_exit(
     $user_pill.remove();
 }
 
+// TODO: We're calculating `description_html` every time, even though
+// we only show it (in `generate_pills_html`) for lines with only one
+// pill. We can probably simplify things by separating out a function
+// that generates `description_html` from the information in a single
+// search pill, and remove `description_html` from the `Suggestion` type.
+export function generate_pills_html(suggestion: Suggestion): string {
+    const search_terms = Filter.parse(suggestion.search_string);
+
+    const pill_render_data = search_terms.map((term, index) => {
+        if (user_pill_operators.has(term.operator) && term.operand !== "") {
+            return search_user_pill_data_from_term(term);
+        }
+        const search_pill: SearchPill = {
+            type: "generic_operator",
+            operator: term.operator,
+            operand: term.operand,
+            negated: term.negated,
+        };
+
+        if (search_pill.operator === "topic" && search_pill.operand === "") {
+            return {
+                ...search_pill,
+                is_empty_string_topic: true,
+                sign: search_pill.negated ? "-" : "",
+                topic_display_name: util.get_final_topic_display_name(""),
+            };
+        }
+        if (search_pill.operator === "search") {
+            let description_html = search_term_description_html(search_pill);
+            // We capitalize the beginning of the suggestion line if it's text (not
+            // pills), which is only relevant for suggestions with search operators.
+            if (index === 0) {
+                const capitalized_first_letter = description_html.charAt(0).toUpperCase();
+                description_html = capitalized_first_letter + description_html.slice(1);
+            }
+            return {
+                ...search_pill,
+                description_html,
+            };
+        }
+        return {
+            ...search_pill,
+            display_value: get_search_string_from_item(search_pill),
+        };
+    });
+
+    // When there's a single pill on a suggestion line, we have space
+    // to provide help text (description_html) explaining what the
+    // operator does. When there's more than one pill we don't show it.
+    if (pill_render_data.length === 1) {
+        const render_data = util.the(pill_render_data);
+        // Don't add description html for search terms, since those "pills"
+        // are already set up to only display text and no pill. We also
+        // don't show it for any user pills.
+        if (render_data.type === "generic_operator" && render_data.operator !== "search") {
+            return render_search_list_item({
+                pills: pill_render_data,
+                description_html: suggestion.description_html,
+            });
+        }
+    }
+
+    return render_search_list_item({
+        pills: pill_render_data,
+    });
+}
+
 export function create_pills($pill_container: JQuery): SearchPillWidget {
     const pills = input_pill.create({
         $container: $pill_container,
@@ -144,6 +213,16 @@ export function create_pills($pill_container: JQuery): SearchPillWidget {
     return pills;
 }
 
+function search_user_pill_data_from_term(term: NarrowTerm): SearchUserPill {
+    const emails = term.operand.split(",");
+    const users = emails.map((email) => {
+        const person = people.get_by_email(email);
+        assert(person !== undefined);
+        return person;
+    });
+    return search_user_pill_data(users, term.operator, term.negated ?? false);
+}
+
 function search_user_pill_data(users: User[], operator: string, negated: boolean): SearchUserPill {
     return {
         type: "search_user",
@@ -248,11 +327,17 @@ function get_search_operand(item: SearchPill, for_display: boolean): string {
     if (item.type === "search_user") {
         return item.users.map((user) => user.email).join(",");
     }
-    if (for_display && item.operator === "channel") {
+    // When we're displaying the operand in a pill, we sometimes want to make
-        return stream_data.get_valid_sub_by_id_string(item.operand).name;
+    // it more human readable. We do this for channel pills (with channels
-    }
+    // specified) and topic pills only.
-    if (for_display && item.operator === "topic") {
+    if (for_display) {
-        return util.get_final_topic_display_name(item.operand);
+        if (item.operator === "channel" && item.operand !== "") {
+            return stream_data.get_valid_sub_by_id_string(item.operand).name;
+        }
+        if (item.operator === "topic") {
+            return util.get_final_topic_display_name(item.operand);
+        }
+        // For all other `for_display=true` cases, we just show the default operand.
     }
     return item.operand;
 }

web/styles/search.css

@@ -259,69 +259,69 @@
         &.focused .user-pill-container {
             flex-flow: row wrap;
         }
+    }
 
-        .user-pill-container {
+    .user-pill-container {
-            gap: 2px 5px;
+        gap: 2px 5px;
-            /* Don't enforce a height, as user-pill containers
+        /* Don't enforce a height, as user-pill containers
-               can contain multiple user pills that wrap onto
+            can contain multiple user pills that wrap onto
-               new lines. */
+            new lines. */
-            height: unset;
+        height: unset;
-            border: 1px solid transparent;
+        border: 1px solid transparent;
 
-            /* Not focus-visible, because we want to support mouse+backpace
+        /* Not focus-visible, because we want to support mouse+backpace
-               to delete pills */
+            to delete pills */
-            &:focus {
+        &:focus {
-                /* Unlike regular `.pill` this multi-user pill has a border,
+            /* Unlike regular `.pill` this multi-user pill has a border,
-                   so we use border instead of box-shadow on focus. */
+                so we use border instead of box-shadow on focus. */
-                box-shadow: none;
+            box-shadow: none;
-                border-color: var(--color-focus-outline-input-pill);
+            border-color: var(--color-focus-outline-input-pill);
+        }
+
+        > .pill-label {
+            min-width: fit-content;
+            white-space: nowrap;
+            width: fit-content;
+            /* Replaced by the 5px gap. */
+            margin-right: 0;
+            /* Don't inherit large line-height for user pill labels. */
+            line-height: 1.1;
+        }
+
+        .pill {
+            /* Reduce user pill height by 2px to
+                preserve an apparent border around
+                them even under `box-sizing: border-box`
+                in the area. */
+            height: calc(var(--height-input-pill) - 2px);
+            border: none;
+            /* Match border radius to image */
+            border-radius: 4px;
+            max-width: none;
+            /* Set the minimum width on the pill container;
+                this accommodates the avatar, a minimum
+                two-character username, and the closing X.
+                90px at 20px/1em */
+            min-width: 4.5em;
+            display: grid;
+            grid-template-columns:
+                var(--length-search-input-pill-image) minmax(0, 1fr)
+                var(--length-input-pill-exit);
+            align-content: center;
+            /* Don't inherit large line-height for user pills themselves, either. */
+            line-height: 1.1;
+
+            .pill-image {
+                height: var(--length-search-input-pill-image);
+                width: var(--length-search-input-pill-image);
             }
 
-            > .pill-label {
+            .pill-image-border {
-                min-width: fit-content;
-                white-space: nowrap;
-                width: fit-content;
-                /* Replaced by the 5px gap. */
-                margin-right: 0;
-                /* Don't inherit large line-height for user pill labels. */
-                line-height: 1.1;
-            }
-
-            .pill {
-                /* Reduce user pill height by 2px to
-                   preserve an apparent border around
-                   them even under `box-sizing: border-box`
-                   in the area. */
-                height: calc(var(--height-input-pill) - 2px);
                 border: none;
-                /* Match border radius to image */
+            }
-                border-radius: 4px;
-                max-width: none;
-                /* Set the minimum width on the pill container;
-                   this accommodates the avatar, a minimum
-                   two-character username, and the closing X.
-                   90px at 20px/1em */
-                min-width: 4.5em;
-                display: grid;
-                grid-template-columns:
-                    var(--length-search-input-pill-image) minmax(0, 1fr)
-                    var(--length-input-pill-exit);
-                align-content: center;
-                /* Don't inherit large line-height for user pills themselves, either. */
-                line-height: 1.1;
 
-                .pill-image {
+            &:not(.deactivated-pill) {
-                    height: var(--length-search-input-pill-image);
+                background-color: var(--color-background-user-pill);
-                    width: var(--length-search-input-pill-image);
-                }
-
-                .pill-image-border {
-                    border: none;
-                }
-
-                &:not(.deactivated-pill) {
-                    background-color: var(--color-background-user-pill);
-                }
             }
         }
     }
@@ -382,10 +382,13 @@
 
         .search_list_item {
             max-width: 100%;
+            display: flex;
+            gap: 5px;
+            align-items: center;
         }
 
         .search_list_item .pill-container {
-            margin: 1px 5px;
+            margin: 2px 0;
             /* This contains only one pill, which handles its own border */
             border: none;
             cursor: pointer;

web/templates/search_list_item.hbs

@@ -1,10 +1,14 @@
 <div class="search_list_item">
-    <span>{{{ description_html }}}</span>
+    {{#each pills}}
-    {{#if is_people}}
-    {{#each users}}
         <span class="pill-container">
-            {{> input_pill user_pill_context}}
+            {{#if (eq this.type "search_user")}}
+                {{> search_user_pill this}}
+            {{else if (eq this.operator "search")}}
+                {{{this.description_html}}}
+            {{else}}
+                {{> input_pill this}}
+            {{/if}}
         </span>
     {{/each}}
-    {{/if}}
+    {{#if description_html}}<div class="description">{{{description_html}}}</div>{{/if}}
 </div>

web/tests/search.test.cjs

@@ -8,11 +8,16 @@ const $ = require("./lib/zjquery.cjs");
 
 const bootstrap_typeahead = mock_esm("../src/bootstrap_typeahead");
 
+const people = zrequire("people");
 const search = zrequire("search");
 const search_pill = zrequire("search_pill");
 const search_suggestion = zrequire("search_suggestion");
+const {set_realm} = zrequire("state_data");
 const stream_data = zrequire("stream_data");
 
+const realm = {};
+set_realm(realm);
+
 function stub_pills() {
     const $pill_container = $("#searchbox-input-container.pill-container");
     const $pill_input = $.create("pill_input");
@@ -40,7 +45,6 @@ run_test("initialize", ({override, override_rewire, mock_template}) => {
     stub_pills();
 
     mock_template("search_list_item.hbs", true, (data, html) => {
-        assert.equal(typeof data.description_html, "string");
         if (data.is_people) {
             for (const user of data.users) {
                 assert.equal(typeof user.user_pill_context.id, "number");
@@ -117,11 +121,14 @@ run_test("initialize", ({override, override_rewire, mock_template}) => {
             const source = opts.source("ver");
             assert.deepStrictEqual(source, expected_source_value);
 
-            /* Test item_html */
+            /* Test highlighter */
-            let expected_value = `<div class="search_list_item">\n    <span>Search for ver</span>\n</div>\n`;
+            let description_html = "Search for ver";
+            let expected_value = `<div class="search_list_item">\n        <span class="pill-container">\n                ${description_html}\n        </span>\n    \n</div>\n`;
             assert.equal(opts.item_html(source[0]), expected_value);
 
-            expected_value = `<div class="search_list_item">\n    <span>Stream Verona</span>\n</div>\n`;
+            const search_string = "channel: Verona";
+            description_html = "Stream Verona";
+            expected_value = `<div class="search_list_item">\n        <span class="pill-container">\n                <div class='pill ' tabindex=0>\n    <span class="pill-label">\n        <span class="pill-value">\n            ${search_string}\n        </span></span>\n    <div class="exit">\n        <a role="button" class="zulip-icon zulip-icon-close pill-close-button"></a>\n    </div>\n</div>\n                    </span>\n    <div class="description">${description_html}</div>\n</div>\n`;
             assert.equal(opts.item_html(source[1]), expected_value);
 
             /* Test sorter */
@@ -205,17 +212,24 @@ run_test("initialize", ({override, override_rewire, mock_template}) => {
             const source = opts.source("zo");
             assert.deepStrictEqual(source, expected_source_value);
 
-            /* Test item_html */
+            /* Test highlighter */
-            let expected_value = `<div class="search_list_item">\n    <span>Search for zo</span>\n</div>\n`;
+            const description_html = "Search for zo";
+            let expected_value = `<div class="search_list_item">\n        <span class="pill-container">\n                ${description_html}\n        </span>\n    \n</div>\n`;
             assert.equal(opts.item_html(source[0]), expected_value);
 
-            expected_value = `<div class="search_list_item">\n    <span>sent by</span>\n        <span class="pill-container">\n            <div class='pill ' tabindex=0>\n    <img class="pill-image" src="https://secure.gravatar.com/avatar/0f030c97ab51312c7bbffd3966198ced?d&#x3D;identicon&amp;version&#x3D;1" />\n    <div class="pill-image-border"></div>\n    <span class="pill-label">\n        <span class="pill-value">\n            Zoe\n        </span></span>\n    <div class="exit">\n        <a role="button" class="zulip-icon zulip-icon-close pill-close-button"></a>\n    </div>\n</div>\n        </span>\n</div>\n`;
+            people.add_active_user({
+                email: "user7@zulipdev.com",
+                user_id: 3,
+                full_name: "Zoe",
+            });
+            override(realm, "realm_enable_guest_user_indicator", true);
+            expected_value = `<div class="search_list_item">\n        <span class="pill-container">\n                <div class="user-pill-container pill" tabindex=0>\n    <span class="pill-label">sender:\n    </span>\n        <div class="pill" data-user-id="3">\n            <img class="pill-image" src="/avatar/3" />\n            <div class="pill-image-border"></div>\n            <span class="pill-label">\n                <span class="pill-value">Zoe</span></span>\n            <div class="exit">\n                <a role="button" class="zulip-icon zulip-icon-close pill-close-button"></a>\n            </div>\n        </div>\n</div>\n        </span>\n    \n</div>\n`;
             assert.equal(opts.item_html(source[1]), expected_value);
 
-            expected_value = `<div class="search_list_item">\n    <span>direct messages with</span>\n        <span class="pill-container">\n            <div class='pill ' tabindex=0>\n    <img class="pill-image" src="https://secure.gravatar.com/avatar/0f030c97ab51312c7bbffd3966198ced?d&#x3D;identicon&amp;version&#x3D;1" />\n    <div class="pill-image-border"></div>\n    <span class="pill-label">\n        <span class="pill-value">\n            Zoe\n        </span></span>\n    <div class="exit">\n        <a role="button" class="zulip-icon zulip-icon-close pill-close-button"></a>\n    </div>\n</div>\n        </span>\n</div>\n`;
+            expected_value = `<div class="search_list_item">\n        <span class="pill-container">\n                <div class="user-pill-container pill" tabindex=0>\n    <span class="pill-label">dm:\n    </span>\n        <div class="pill" data-user-id="3">\n            <img class="pill-image" src="/avatar/3" />\n            <div class="pill-image-border"></div>\n            <span class="pill-label">\n                <span class="pill-value">Zoe</span></span>\n            <div class="exit">\n                <a role="button" class="zulip-icon zulip-icon-close pill-close-button"></a>\n            </div>\n        </div>\n</div>\n        </span>\n    \n</div>\n`;
             assert.equal(opts.item_html(source[2]), expected_value);
 
-            expected_value = `<div class="search_list_item">\n    <span>group direct messages including</span>\n        <span class="pill-container">\n            <div class='pill ' tabindex=0>\n    <img class="pill-image" src="https://secure.gravatar.com/avatar/0f030c97ab51312c7bbffd3966198ced?d&#x3D;identicon&amp;version&#x3D;1" />\n    <div class="pill-image-border"></div>\n    <span class="pill-label">\n        <span class="pill-value">\n            Zoe\n        </span></span>\n    <div class="exit">\n        <a role="button" class="zulip-icon zulip-icon-close pill-close-button"></a>\n    </div>\n</div>\n        </span>\n</div>\n`;
+            expected_value = `<div class="search_list_item">\n        <span class="pill-container">\n                <div class="user-pill-container pill" tabindex=0>\n    <span class="pill-label">dm-including:\n    </span>\n        <div class="pill" data-user-id="3">\n            <img class="pill-image" src="/avatar/3" />\n            <div class="pill-image-border"></div>\n            <span class="pill-label">\n                <span class="pill-value">Zoe</span></span>\n            <div class="exit">\n                <a role="button" class="zulip-icon zulip-icon-close pill-close-button"></a>\n            </div>\n        </div>\n</div>\n        </span>\n    \n</div>\n`;
             assert.equal(opts.item_html(source[3]), expected_value);
 
             /* Test sorter */