diff --git a/tools/test-js-with-node b/tools/test-js-with-node index efbad6aae2..7febdb6368 100755 --- a/tools/test-js-with-node +++ b/tools/test-js-with-node @@ -198,6 +198,7 @@ EXEMPT_FILES = make_set( "web/src/scroll_bar.ts", "web/src/scroll_util.ts", "web/src/search.ts", + "web/src/search_pill.ts", "web/src/sent_messages.ts", "web/src/sentry.ts", "web/src/server_events.js", diff --git a/web/e2e-tests/drafts.test.ts b/web/e2e-tests/drafts.test.ts index b6825d3224..ab7a6439d6 100644 --- a/web/e2e-tests/drafts.test.ts +++ b/web/e2e-tests/drafts.test.ts @@ -47,6 +47,7 @@ async function test_restore_stream_message_draft_by_opening_compose_box(page: Pa await page.click(".search_icon"); await page.waitForSelector("#search_query", {visible: true}); await common.clear_and_type(page, "#search_query", "stream:Denmark topic:tests"); + await page.keyboard.press("Enter"); // Wait for narrow to complete. const wait_for_change = true; await common.get_current_msg_list_id(page, wait_for_change); diff --git a/web/e2e-tests/message-basics.test.ts b/web/e2e-tests/message-basics.test.ts index 74cc388d81..55a94cce81 100644 --- a/web/e2e-tests/message-basics.test.ts +++ b/web/e2e-tests/message-basics.test.ts @@ -179,7 +179,11 @@ async function search_and_check( ): Promise { await page.click(".search_icon"); await page.waitForSelector(".navbar-search.expanded", {visible: true}); + // Close the "in: home" pill + await page.click(".navbar-search .pill-close-button"); await common.select_item_via_typeahead(page, "#search_query", search_str, item_to_select); + // Enter to trigger search + await page.keyboard.press("Enter"); await check(page); assert.strictEqual(await page.title(), expected_narrow_title); await un_narrow(page); @@ -189,7 +193,11 @@ async function search_and_check( async function search_silent_user(page: Page, str: string, item: string): Promise { await page.click(".search_icon"); await page.waitForSelector(".navbar-search.expanded", {visible: true}); + // Close the "in: home" pill + await page.click(".navbar-search .pill-close-button"); await common.select_item_via_typeahead(page, "#search_query", str, item); + // Enter to trigger search + await page.keyboard.press("Enter"); await page.waitForSelector(".empty_feed_notice", {visible: true}); const expect_message = "You haven't received any messages sent by Email Gateway yet."; assert.strictEqual( @@ -224,7 +232,11 @@ async function expect_non_existing_users(page: Page): Promise { async function search_non_existing_user(page: Page, str: string, item: string): Promise { await page.click(".search_icon"); await page.waitForSelector(".navbar-search.expanded", {visible: true}); + // Close the "in: home" pill + await page.click(".navbar-search .pill-close-button"); await common.select_item_via_typeahead(page, "#search_query", str, item); + // Enter to trigger search + await page.keyboard.press("Enter"); await expect_non_existing_user(page); await un_narrow(page); await expect_home(page); diff --git a/web/src/message_view_header.ts b/web/src/message_view_header.ts index 5dc3fcac03..b2fc9fe056 100644 --- a/web/src/message_view_header.ts +++ b/web/src/message_view_header.ts @@ -157,7 +157,6 @@ function build_message_view_header(filter: Filter | undefined): void { // message_view_header on a template where it's never used if (filter && !filter.is_common_narrow()) { search.open_search_bar_and_close_narrow_description(); - search.set_search_bar_text(narrow_state.search_string()); } else { const context = get_message_view_header_context(filter); append_and_display_title_area(context); diff --git a/web/src/search.ts b/web/src/search.ts index f67ca0cf1e..11a4e1d564 100644 --- a/web/src/search.ts +++ b/web/src/search.ts @@ -9,18 +9,18 @@ import {Filter} from "./filter"; import * as keydown_util from "./keydown_util"; import * as narrow_state from "./narrow_state"; import * as popovers from "./popovers"; +import * as search_pill from "./search_pill"; +import type {SearchPillWidget} from "./search_pill"; import * as search_suggestion from "./search_suggestion"; import type {NarrowTerm} from "./state_data"; // Exported for unit testing export let is_using_input_method = false; +export let search_pill_widget: SearchPillWidget | null = null; +let search_input_has_changed = false; let search_typeahead: Typeahead; -export function set_search_bar_text(text: string): void { - $("#search_query").text(text); -} - function get_search_bar_text(): string { return $("#search_query").text(); } @@ -32,15 +32,7 @@ type NarrowSearchOptions = { type OnNarrowSearch = (terms: NarrowTerm[], options: NarrowSearchOptions) => void; -function narrow_or_search_for_term( - search_string: string, - {on_narrow_search}: {on_narrow_search: OnNarrowSearch}, -): string { - if (search_string === "") { - exit_search({keep_search_narrow_open: true}); - return ""; - } - const $search_query_box = $("#search_query"); +function narrow_or_search_for_term({on_narrow_search}: {on_narrow_search: OnNarrowSearch}): string { if (is_using_input_method) { // Neither narrow nor search when using input tools as // `updater` is also triggered when 'enter' is triggered @@ -48,7 +40,13 @@ function narrow_or_search_for_term( return get_search_bar_text(); } - const terms = Filter.parse(search_string); + assert(search_pill_widget !== null); + const search_query = search_pill.get_current_search_string_for_widget(search_pill_widget); + if (search_query === "") { + exit_search({keep_search_narrow_open: true}); + return ""; + } + const terms = Filter.parse(search_query); on_narrow_search(terms, {trigger: "search"}); // It's sort of annoying that this is not in a position to @@ -57,13 +55,19 @@ function narrow_or_search_for_term( // Narrowing will have already put some terms in the search box, // so leave the current text in. - $search_query_box.trigger("blur"); + $("#search_query").trigger("blur"); return get_search_bar_text(); } export function initialize({on_narrow_search}: {on_narrow_search: OnNarrowSearch}): void { const $search_query_box = $("#search_query"); const $searchbox_form = $("#searchbox_form"); + const $pill_container = $("#searchbox-input-container.pill-container"); + + search_pill_widget = search_pill.create_pills($pill_container); + search_pill_widget.onPillRemove(() => { + search_input_has_changed = true; + }); // Data storage for the typeahead. // This maps a search string to an object with a "description_html" field. @@ -78,7 +82,13 @@ export function initialize({on_narrow_search}: {on_narrow_search: OnNarrowSearch }; search_typeahead = new Typeahead(bootstrap_typeahead_input, { source(query: string): string[] { - const suggestions = search_suggestion.get_suggestions(query); + if (query !== "") { + search_input_has_changed = true; + } + assert(search_pill_widget !== null); + const query_from_pills = + search_pill.get_current_search_string_for_widget(search_pill_widget); + const suggestions = search_suggestion.get_suggestions(`${query_from_pills} ${query}`); // Update our global search_map hash search_map = suggestions.lookup_table; return suggestions.strings; @@ -86,21 +96,44 @@ export function initialize({on_narrow_search}: {on_narrow_search: OnNarrowSearch non_tippy_parent_element: "#searchbox_form", items: search_suggestion.max_num_of_search_results, helpOnEmptyStrings: true, - naturalSearch: true, + stopAdvance: true, + requireHighlight: false, highlighter_html(item: string): string { const obj = search_map.get(item); return render_search_list_item(obj); }, + // When the user starts typing new search operands, + // we want to highlight the first typeahead row by default + // so that pressing Enter creates the default pill. + // But when user isn't in the middle of typing a new pill, + // pressing Enter should let them search for what's currently + // in the search bar, so we remove the highlight (so that + // Enter won't have anything to select). + shouldHighlightFirstResult(): boolean { + return get_search_bar_text() !== ""; + }, matcher(): boolean { return true; }, updater(search_string: string): string { - return narrow_or_search_for_term(search_string, {on_narrow_search}); + if (search_string) { + search_input_has_changed = true; + // Reset the search box and add the pills based on the selected + // search suggestion. + assert(search_pill_widget !== null); + const search_terms = Filter.parse(search_string); + search_pill.set_search_bar_contents(search_terms, search_pill_widget); + $search_query_box.trigger("focus"); + } + return get_search_bar_text(); }, sorter(items: string[]): string[] { return items; }, - advanceKeyCodes: [8], + // Turns off `stopPropagation` in the typeahead code for + // backspace, arrow left, arrow right, so that we can + // manage those events for input pills. + advanceKeyCodes: [8, 37, 39], // Use our custom typeahead `on_escape` hook to exit // the search bar as soon as the user hits Esc. @@ -113,16 +146,15 @@ export function initialize({on_narrow_search}: {on_narrow_search: OnNarrowSearch open_search_bar_and_close_narrow_description(); } }, + // This is here so that we can close the search bar + // when a user opens it and immediately changes their + // mind and clicks away. closeInputFieldOnHide(): void { - // Don't close the search bar if the user has changed - // the text from the default, they might accidentally - // click away and not want to lose it. - if (get_initial_search_string() !== get_search_bar_text()) { - return; - } - const filter = narrow_state.filter(); - if (!filter || filter.is_common_narrow()) { - close_search_bar_and_open_narrow_description(); + if (!search_input_has_changed) { + const filter = narrow_state.filter(); + if (!filter || filter.is_common_narrow()) { + close_search_bar_and_open_narrow_description(); + } } }, }); @@ -151,17 +183,10 @@ export function initialize({on_narrow_search}: {on_narrow_search: OnNarrowSearch return; } - if (keydown_util.is_enter_event(e) && $search_query_box.is(":focus")) { - // We just pressed Enter and the box had focus, which - // means we didn't use the typeahead at all. In that - // case, we should act as though we're searching by - // terms. (The reason the other actions don't call - // this codepath is that they first all blur the box to - // indicate that they've done what they need to do) - - // Pill is already added during keydown event of input pills. - narrow_or_search_for_term(get_search_bar_text(), {on_narrow_search}); - $search_query_box.trigger("blur"); + if (e.key === "Escape" && $search_query_box.is(":focus")) { + exit_search({keep_search_narrow_open: false}); + } else if (keydown_util.is_enter_event(e) && $search_query_box.is(":focus")) { + narrow_or_search_for_term({on_narrow_search}); } }); @@ -214,30 +239,21 @@ export function initialize({on_narrow_search}: {on_narrow_search: OnNarrowSearch export function initiate_search(): void { open_search_bar_and_close_narrow_description(); + $("#search_query").trigger("focus"); // Open the typeahead after opening the search bar, so that we don't // get a weird visual jump where the typeahead results are narrow // before the search bar expands and then wider it expands. search_typeahead.lookup(false); - $("#search_query").trigger("select"); -} - -// This is what the default searchbox text would be for this narrow, -// NOT what might be currently displayed there. We can use this both -// to set the initial text and to see if the user has changed it. -function get_initial_search_string(): string { - let search_string = narrow_state.search_string(); - if (search_string !== "" && !narrow_state.filter()?.is_keyword_search()) { - // saves the user a keystroke for quick searches - search_string = search_string + " "; - } - return search_string; } // we rely entirely on this function to ensure -// the searchbar has the right text. -function reset_searchbox_text(): void { - set_search_bar_text(get_initial_search_string()); +// the searchbar has the right text/pills. +function reset_searchbox(): void { + assert(search_pill_widget !== null); + search_pill_widget.clear(); + search_input_has_changed = false; + search_pill.set_search_bar_contents(narrow_state.search_terms(), search_pill_widget); } function exit_search(opts: {keep_search_narrow_open: boolean}): void { @@ -261,7 +277,7 @@ export function open_search_bar_and_close_narrow_description(): void { // otherwise fill the input field with the text terms for // the current narrow. if (get_search_bar_text() === "") { - reset_searchbox_text(); + reset_searchbox(); } $(".navbar-search").addClass("expanded"); $("#message_view_header").addClass("hidden"); @@ -274,7 +290,15 @@ export function close_search_bar_and_open_narrow_description(): void { // in width as the search bar closes, which doesn't look great. $("#searchbox_form .dropdown-menu").hide(); - set_search_bar_text(""); + if (search_pill_widget !== null) { + search_pill_widget.clear(); + } + $(".navbar-search").removeClass("expanded"); $("#message_view_header").removeClass("hidden"); + + if ($("#search_query").is(":focus")) { + $("#search_query").trigger("blur"); + $(".app").trigger("focus"); + } } diff --git a/web/src/search_pill.ts b/web/src/search_pill.ts new file mode 100644 index 0000000000..592db163bb --- /dev/null +++ b/web/src/search_pill.ts @@ -0,0 +1,53 @@ +import {Filter} from "./filter"; +import * as input_pill from "./input_pill"; +import type {InputPillContainer} from "./input_pill"; +import type {NarrowTerm} from "./state_data"; + +type SearchPill = { + display_value: string; + type: string; + description_html: string; +}; +export type SearchPillWidget = InputPillContainer; + +export function create_item_from_search_string(search_string: string): SearchPill { + const search_terms = Filter.parse(search_string); + const description_html = Filter.search_description_as_html(search_terms); + return { + display_value: search_string, + type: "search", + description_html, + }; +} + +export function get_search_string_from_item(item: SearchPill): string { + return item.display_value; +} + +export function create_pills($pill_container: JQuery): SearchPillWidget { + const pills = input_pill.create({ + $container: $pill_container, + create_item_from_text: create_item_from_search_string, + get_text_from_item: get_search_string_from_item, + split_text_on_comma: false, + }); + return pills; +} + +export function set_search_bar_contents( + search_terms: NarrowTerm[], + pill_widget: SearchPillWidget, +): void { + pill_widget.clear(); + for (const term of search_terms) { + const input = Filter.unparse([term]); + pill_widget.appendValue(input); + } + pill_widget.clear_text(); +} + +export function get_current_search_string_for_widget(pill_widget: SearchPillWidget): string { + const items = pill_widget.items(); + const search_strings = items.map((item) => item.display_value); + return search_strings.join(" "); +} diff --git a/web/styles/search.css b/web/styles/search.css index ba2b9e7af4..1a07d2cc03 100644 --- a/web/styles/search.css +++ b/web/styles/search.css @@ -58,7 +58,7 @@ } .zulip-icon-close { - font-size: 13px; + font-size: 10px; } .navbar-search:not(.expanded) { @@ -159,6 +159,13 @@ .search-input { /* Avoid massive inheritance chain on font-size. */ font-size: var(--base-font-size-px); + /* override height-input-pill from default input pills to account for pill border */ + line-height: calc( + var(--height-input-pill) + var(--vertical-spacing-input-pill) + ); + min-height: calc( + var(--height-input-pill) + var(--vertical-spacing-input-pill) + ); /* override bootstrap padding for input[type="text"] */ padding: 0; border: none; @@ -189,9 +196,36 @@ #searchbox-input-container { display: flex; align-items: center; - /* override .input-append style from app_components.js */ + /* The next two styles override .input-append style from app_components.js */ letter-spacing: normal; + font-size: 100%; height: var(--search-box-height); + /* Override styles for .pill-container that aren't relevant for search. */ + flex-wrap: nowrap; + padding: 0; + border: none; + + .user-pill-container { + padding: 2px; + height: 22px; + min-width: fit-content; + + > .pill-label { + min-width: fit-content; + white-space: nowrap; + width: fit-content; + } + + .pill { + height: 22px; + margin: 2px; + } + + .pill-image { + width: 22px; + height: 22px; + } + } } @media (width >= $md_min) { diff --git a/web/templates/navbar.hbs b/web/templates/navbar.hbs index c92b46942c..d5ab77b678 100644 --- a/web/templates/navbar.hbs +++ b/web/templates/navbar.hbs @@ -22,7 +22,7 @@