From 6e43d34a346a8f855183d3e9655c18b1d1819ddc Mon Sep 17 00:00:00 2001 From: Maneesh Shukla Date: Mon, 28 Jul 2025 16:25:07 +0530 Subject: [PATCH] topic-filter: Replace usage of search_pill. Add topic_filter_pill to use for topic filtering instead of search_pill. Fixes part of #35284. --- tools/lib/capitalization.py | 4 ++ tools/test-js-with-node | 1 + web/src/topic_filter_pill.ts | 68 ++++++++++++++++++++++++++++++ web/src/topic_list.ts | 63 +++++++++++++-------------- web/src/topic_list_data.ts | 4 +- web/templates/input_pill.hbs | 6 ++- web/tests/topic_list_data.test.cjs | 4 +- 7 files changed, 112 insertions(+), 38 deletions(-) create mode 100644 web/src/topic_filter_pill.ts diff --git a/tools/lib/capitalization.py b/tools/lib/capitalization.py index 578734fc67..e32505df5d 100644 --- a/tools/lib/capitalization.py +++ b/tools/lib/capitalization.py @@ -176,6 +176,10 @@ IGNORED_PHRASES = [ r"archived", # Used in pills for deactivated users. r"deactivated", + # Used in pills for resolved topics. + r"resolved", + # Used in pills for unresolved topics. + r"unresolved", # This is a reference to a setting/secret and should be lowercase. r"zulip_org_id", # These are custom time unit options for modal dropdowns diff --git a/tools/test-js-with-node b/tools/test-js-with-node index 98e7065f72..bb73200bcd 100755 --- a/tools/test-js-with-node +++ b/tools/test-js-with-node @@ -271,6 +271,7 @@ EXEMPT_FILES = make_set( "web/src/timerender.ts", "web/src/tippyjs.ts", "web/src/todo_widget.ts", + "web/src/topic_filter_pill.ts", "web/src/topic_list.ts", "web/src/topic_popover.ts", "web/src/typing.ts", diff --git a/web/src/topic_filter_pill.ts b/web/src/topic_filter_pill.ts new file mode 100644 index 0000000000..146b2f70b7 --- /dev/null +++ b/web/src/topic_filter_pill.ts @@ -0,0 +1,68 @@ +import render_input_pill from "../templates/input_pill.hbs"; + +import {$t} from "./i18n.ts"; +import type {InputPillConfig, InputPillContainer} from "./input_pill.ts"; +import * as input_pill from "./input_pill.ts"; + +export type TopicFilterPill = { + type: "topic_filter"; + label: string; + syntax: string; +}; + +export type TopicFilterPillWidget = InputPillContainer; + +export const filter_options: TopicFilterPill[] = [ + { + type: "topic_filter", + label: $t({defaultMessage: "unresolved"}), + syntax: "-is:resolved", + }, + { + type: "topic_filter", + label: $t({defaultMessage: "resolved"}), + syntax: "is:resolved", + }, +]; + +export function create_item_from_syntax( + syntax: string, + current_items: TopicFilterPill[], +): TopicFilterPill | undefined { + const existing_syntaxes = current_items.map((item) => item.syntax); + if (existing_syntaxes.includes(syntax)) { + return undefined; + } + + // Find the matching filter option + const filter_option = filter_options.find((option) => option.syntax === syntax); + if (!filter_option) { + return undefined; + } + return filter_option; +} + +export function get_syntax_from_item(item: TopicFilterPill): string { + return item.syntax; +} + +export function create_pills( + $pill_container: JQuery, + pill_config?: InputPillConfig, +): TopicFilterPillWidget { + const pill_container = input_pill.create({ + $container: $pill_container, + pill_config, + create_item_from_text: create_item_from_syntax, + get_text_from_item: get_syntax_from_item, + get_display_value_from_item: get_syntax_from_item, + generate_pill_html(item: TopicFilterPill, disabled?: boolean) { + return render_input_pill({ + display_value: item.label, + disabled, + }); + }, + }); + pill_container.createPillonPaste(() => false); + return pill_container; +} diff --git a/web/src/topic_list.ts b/web/src/topic_list.ts index 2317cb3af6..2e91907877 100644 --- a/web/src/topic_list.ts +++ b/web/src/topic_list.ts @@ -10,43 +10,26 @@ import {all_messages_data} from "./all_messages_data.ts"; import * as blueslip from "./blueslip.ts"; import {Typeahead} from "./bootstrap_typeahead.ts"; import type {TypeaheadInputElement} from "./bootstrap_typeahead.ts"; -import {$t} from "./i18n.ts"; import * as popover_menus from "./popover_menus.ts"; import * as popovers from "./popovers.ts"; import * as scroll_util from "./scroll_util.ts"; -import type {SearchPillWidget} from "./search_pill.ts"; -import * as search_pill from "./search_pill.ts"; import * as sidebar_ui from "./sidebar_ui.ts"; import * as stream_topic_history from "./stream_topic_history.ts"; import * as stream_topic_history_util from "./stream_topic_history_util.ts"; import type {StreamSubscription} from "./sub_store.ts"; import * as sub_store from "./sub_store.ts"; +import * as topic_filter_pill from "./topic_filter_pill.ts"; +import type {TopicFilterPill, TopicFilterPillWidget} from "./topic_filter_pill.ts"; import * as topic_list_data from "./topic_list_data.ts"; import type {TopicInfo} from "./topic_list_data.ts"; import * as typeahead_helper from "./typeahead_helper.ts"; import * as vdom from "./vdom.ts"; -type TopicFilterPill = { - label: string; - syntax: string; -}; - -const filter_options: TopicFilterPill[] = [ - { - label: $t({defaultMessage: "Unresolved topics"}), - syntax: "-is:resolved", - }, - { - label: $t({defaultMessage: "Resolved topics"}), - syntax: "is:resolved", - }, -]; - /* Track all active widgets with a Map by stream_id. We have at max one for now, but we may eventually allow multiple streams to be expanded. */ const active_widgets = new Map(); -export let search_pill_widget: SearchPillWidget | null = null; +export let topic_filter_pill_widget: TopicFilterPillWidget | null = null; export let topic_state_typeahead: Typeahead | undefined; // We know whether we're zoomed or not. @@ -366,7 +349,7 @@ function filter_topics_left_sidebar(topic_names: string[]): string[] { return topic_list_data.filter_topics_by_search_term( topic_names, search_term, - get_typeahead_search_term(), + get_typeahead_search_pills_syntax(), ); } @@ -386,10 +369,10 @@ export class LeftSidebarTopicListWidget extends TopicListWidget { export function clear_topic_search(e: JQuery.Event): void { e.stopPropagation(); - search_pill_widget?.clear(true); + topic_filter_pill_widget?.clear(true); const $input = $("#topic_filter_query"); - // Since the `clear` function of the search_pill_widget + // Since the `clear` function of the topic_filter_pill_widget // takes care of clearing both the text content and the // pills, we just need to trigger an input event on the // contenteditable element to reset the topic list via @@ -499,10 +482,24 @@ export function get_left_sidebar_topic_search_term(): string { return $("#topic_filter_query").text().trim(); } -export function get_typeahead_search_term(): string { - const $pills = $("#left-sidebar-filter-topic-input .pill"); - const value = $pills.find(".pill-value").text().trim(); - return value; +export function get_typeahead_search_pills_syntax(): string { + const pills = topic_filter_pill_widget?.items() ?? []; + + if (pills.length === 0) { + return ""; + } + + // For now, there is only one pill in the left sidebar topic search input. + // This is because we only allow one topic filter pill at a time. + // If we allow multiple pills in the future, we may need to + // change this logic to return the syntax of all pills. + if (pills.length > 1) { + blueslip.warn("Multiple pills found in left sidebar topic search input."); + } + + // We can remove this assumption once we allow multiple pills and hence update the + // callers of this function to handle multiple pills and implement the search accordingly. + return pills[0]!.syntax; } function set_search_bar_text(text: string): void { @@ -519,7 +516,7 @@ export function setup_topic_search_typeahead(): void { return; } - search_pill_widget = search_pill.create_pills($pill_container); + topic_filter_pill_widget = topic_filter_pill.create_pills($pill_container); const typeahead_input: TypeaheadInputElement = { $element: $input, @@ -538,7 +535,7 @@ export function setup_topic_search_typeahead(): void { if ($pills.length > 0) { return []; } - return [...filter_options]; + return [...topic_filter_pill.filter_options]; }, item_html(item: TopicFilterPill) { return typeahead_helper.render_topic_state(item.label); @@ -559,9 +556,9 @@ export function setup_topic_search_typeahead(): void { return items; }, updater(item: TopicFilterPill) { - assert(search_pill_widget !== null); - search_pill_widget.clear(true); - search_pill_widget.appendValue(item.syntax); + assert(topic_filter_pill_widget !== null); + topic_filter_pill_widget.clear(true); + topic_filter_pill_widget.appendValue(item.syntax); set_search_bar_text(""); $input.trigger("focus"); return get_left_sidebar_topic_search_term(); @@ -585,7 +582,7 @@ export function setup_topic_search_typeahead(): void { } }); - search_pill_widget.onPillRemove(() => { + topic_filter_pill_widget.onPillRemove(() => { const stream_id = active_stream_id(); if (stream_id !== undefined) { const widget = active_widgets.get(stream_id); diff --git a/web/src/topic_list_data.ts b/web/src/topic_list_data.ts index 40dfa7ea3f..493a137be9 100644 --- a/web/src/topic_list_data.ts +++ b/web/src/topic_list_data.ts @@ -174,9 +174,9 @@ export function filter_topics_by_search_term( word_separator_regex, ); - if (topics_state === "is: resolved") { + if (topics_state === "is:resolved") { topic_names = topic_names.filter((name) => resolved_topic.is_resolved(name)); - } else if (topics_state === "-is: resolved") { + } else if (topics_state === "-is:resolved") { topic_names = topic_names.filter((name) => !resolved_topic.is_resolved(name)); } diff --git a/web/templates/input_pill.hbs b/web/templates/input_pill.hbs index 5f3f96d8f6..67205668ef 100644 --- a/web/templates/input_pill.hbs +++ b/web/templates/input_pill.hbs @@ -1,4 +1,8 @@ -
+
{{#if has_image}}
diff --git a/web/tests/topic_list_data.test.cjs b/web/tests/topic_list_data.test.cjs index f807b22df8..db45908077 100644 --- a/web/tests/topic_list_data.test.cjs +++ b/web/tests/topic_list_data.test.cjs @@ -62,7 +62,7 @@ test("filter_topics_by_search_term with resolved topics_state", () => { const search_term = ""; // Filter for resolved topics. - let topics_state = "is: resolved"; + let topics_state = "is:resolved"; let result = topic_list_data.filter_topics_by_search_term( topic_names, @@ -73,7 +73,7 @@ test("filter_topics_by_search_term with resolved topics_state", () => { assert.deepEqual(result, ["✔ resolved topic"]); // Filter for unresolved topics. - topics_state = "-is: resolved"; + topics_state = "-is:resolved"; result = topic_list_data.filter_topics_by_search_term(topic_names, search_term, topics_state); assert.deepEqual(result, ["topic 1", "topic 2"]);