compose: Prompt topic typeahead after selecting stream.

Lets the user get a consecutive topic typeahead after selecting a
particular stream via the typeahead menu. Previously, they had to
manually enter ">" after stream typeahead selection.

Previously, whenever ">" was encountered, it would result in returning
"topic_jump," which caused the topic list not to get triggered since
the return value of "topic_jump" meant that the topic was already
selected.

Changes are also made to the regexes to accurately trigger
the stream or topic typeaheads based on the presence of
">" in the current token.

This effectively returned an empty array from the get_candidates
function in lookup() in the bootstrap_typeahead.ts file. The
current implementation is changed to return the sliced
token instead which is then processed inside get_candidates
to trigger topic or stream typeahead based on the state
of the current token.

Added the function definition for the hideAfterSelect() function in
the TypeAhead constructor inside composebox_typeahead.ts. It has a
default implementation of returning true, which closes the typeahead
after a selection is made. It was changed so that it didn't close the
typeahead when the stream is being completed.
Updated tests and added one to test this new behavior.

Fixes: #32184.
This commit is contained in:
Apoorva Pendse 2024-11-18 19:44:54 +05:30
parent a8abcf5210
commit 0965d4e893
3 changed files with 57 additions and 29 deletions

View File

@ -282,6 +282,9 @@ export class Typeahead<ItemType extends string | object> {
} }
this.$menu = $(MENU_HTML).appendTo(this.$container); this.$menu = $(MENU_HTML).appendTo(this.$container);
this.$header = $(HEADER_ELEMENT_HTML).appendTo(this.$container); this.$header = $(HEADER_ELEMENT_HTML).appendTo(this.$container);
// in case of composebox_typeahead, source will run the get candidates method
// which will fetch the relevant candidates based on the value of completing
// which is set by checking what the given token startsWith
this.source = options.source; this.source = options.source;
this.dropup = options.dropup ?? false; this.dropup = options.dropup ?? false;
this.automated = options.automated ?? (() => false); this.automated = options.automated ?? (() => false);

View File

@ -418,7 +418,8 @@ export function tokenize_compose_str(s: string): string {
while (i > min_i) { while (i > min_i) {
i -= 1; i -= 1;
switch (s[i]) { const current_char = s[i];
switch (current_char) {
case "`": case "`":
case "~": case "~":
// Code block must start on a new line // Code block must start on a new line
@ -449,21 +450,7 @@ export function tokenize_compose_str(s: string): string {
} }
break; break;
case ">": case ">":
// topic_jump // No need to set to topic_jump, since we are triggering it after setting the stream, we return '#stream_name>' text instead
//
// If you hit `>` immediately after completing the typeahead for mentioning a stream,
// this will reposition the user from. If | is the cursor, implements:
//
// `#**stream name** >|` => `#**stream name>|`.
if (
s.slice(Math.max(0, i - 2), i) === "**" ||
s.slice(Math.max(0, i - 3), i) === "** "
) {
// return any string as long as its not ''.
return ">topic_jump";
}
// maybe topic_list; let's let the stream_topic_regex decide later.
return ">topic_list";
} }
} }
@ -900,13 +887,21 @@ export function get_candidates(
return typeahead_helper.sort_slash_commands(matches_list, token); return typeahead_helper.sort_slash_commands(matches_list, token);
} }
if (ALLOWED_MARKDOWN_FEATURES.stream && current_token.startsWith("#")) { // Not a stream if it ends with '> some text'
if (
ALLOWED_MARKDOWN_FEATURES.stream &&
current_token.startsWith("#") &&
!/>[a-zA-Z0-9\s]*$/.test(current_token)
) {
if (current_token.length === 1) { if (current_token.length === 1) {
// only "#" is entered
return []; return [];
} }
// get whatever is present after '#'
current_token = current_token.slice(1); current_token = current_token.slice(1);
if (current_token.startsWith("**")) { if (current_token.startsWith("**")) {
// ignore first three characters, in case of something like "#**core" as the query
current_token = current_token.slice(2); current_token = current_token.slice(2);
} }
@ -916,7 +911,8 @@ export function get_candidates(
} }
completing = "stream"; completing = "stream";
token = current_token; token = current_token; // contains the stream name to search for
// Example if #**core is the query entered, token="core"
const candidate_list: StreamPillData[] = stream_data.get_unsorted_subs().map((sub) => ({ const candidate_list: StreamPillData[] = stream_data.get_unsorted_subs().map((sub) => ({
...sub, ...sub,
type: "stream", type: "stream",
@ -930,7 +926,7 @@ export function get_candidates(
// Stream regex modified from marked.js // Stream regex modified from marked.js
// Matches '#**stream name** >' at the end of a split. // Matches '#**stream name** >' at the end of a split.
const stream_regex = /#\*\*([^*>]+)\*\*\s?>$/; const stream_regex = /#\*\*([^*>]+)\*\*\s?>$/;
const should_jump_inside_typeahead = stream_regex.test(split[0]); const should_jump_inside_typeahead = stream_regex.test(current_token);
if (should_jump_inside_typeahead) { if (should_jump_inside_typeahead) {
completing = "topic_jump"; completing = "topic_jump";
token = ">"; token = ">";
@ -960,7 +956,10 @@ export function get_candidates(
} }
const stream_id = stream_data.get_stream_id(stream_name); const stream_id = stream_data.get_stream_id(stream_name);
const topic_list = topics_seen_for(stream_id); let topic_list: string[] = [];
topic_list.push(stream_name); // option to only select stream
// topic list will also contain the stream name at the top in case user only wants to select current stream.
topic_list = [...topic_list, ...topics_seen_for(stream_id)];
if (should_show_custom_query(token, topic_list)) { if (should_show_custom_query(token, topic_list)) {
topic_list.push(token); topic_list.push(token);
} }
@ -1129,7 +1128,7 @@ export function content_typeahead_selected(
// use markdown link syntax // use markdown link syntax
beginning += topic_link_util.get_fallback_markdown_link(item.name); beginning += topic_link_util.get_fallback_markdown_link(item.name);
} else { } else {
beginning += "#**" + item.name + "** "; beginning += "#**" + item.name + ">";
} }
} }
compose_validate.warn_if_private_stream_is_linked(item, $textbox); compose_validate.warn_if_private_stream_is_linked(item, $textbox);
@ -1173,6 +1172,15 @@ export function content_typeahead_selected(
// "beginning" contains all the text before the cursor, so we use lastIndexOf to // "beginning" contains all the text before the cursor, so we use lastIndexOf to
// avoid any other stream+topic mentions in the message. // avoid any other stream+topic mentions in the message.
const syntax_start_index = beginning.lastIndexOf("#**"); const syntax_start_index = beginning.lastIndexOf("#**");
const topic_start_index = beginning.lastIndexOf(">");
const topic_name = item.topic;
// eliminate #** while deducing stream name
const stream_name = beginning.slice(syntax_start_index + 3, topic_start_index);
if (topic_name === stream_name) {
// means the user opted to select only the stream and not the associated topics.
beginning = beginning.slice(0, syntax_start_index) + "#**" + stream_name + "** ";
break;
}
beginning = beginning =
beginning.slice(0, syntax_start_index) + beginning.slice(0, syntax_start_index) +
topic_link_util.get_stream_topic_link_syntax( topic_link_util.get_stream_topic_link_syntax(
@ -1275,9 +1283,6 @@ export function initialize_topic_edit_typeahead(
function get_header_html(): string | false { function get_header_html(): string | false {
let tip_text = ""; let tip_text = "";
switch (completing) { switch (completing) {
case "stream":
tip_text = $t({defaultMessage: "Press > for list of topics"});
break;
case "silent_mention": case "silent_mention":
tip_text = $t({defaultMessage: "Silent mentions do not trigger notifications."}); tip_text = $t({defaultMessage: "Silent mentions do not trigger notifications."});
break; break;
@ -1310,7 +1315,7 @@ export function initialize_compose_typeahead($element: JQuery<HTMLTextAreaElemen
// matching and sorting inside the `source` field to avoid // matching and sorting inside the `source` field to avoid
// O(n) behavior in the number of users in the organization // O(n) behavior in the number of users in the organization
// inside the typeahead library. // inside the typeahead library.
source: get_candidates, source: get_candidates, // responsible for mutating the state of completing
highlighter_html: content_highlighter_html, highlighter_html: content_highlighter_html,
matcher() { matcher() {
return true; return true;
@ -1323,6 +1328,13 @@ export function initialize_compose_typeahead($element: JQuery<HTMLTextAreaElemen
automated: compose_automated_selection, automated: compose_automated_selection,
trigger_selection: compose_trigger_selection, trigger_selection: compose_trigger_selection,
header_html: get_header_html, header_html: get_header_html,
hideAfterSelect() {
// Don't remove the typeahead if we are completing the stream or topic
if (completing === "stream") {
return false;
}
return true;
},
}), }),
); );
} }

View File

@ -772,19 +772,19 @@ test("content_typeahead_selected", ({override}) => {
query = "#swed"; query = "#swed";
ct.get_or_set_token_for_testing("swed"); ct.get_or_set_token_for_testing("swed");
actual_value = ct.content_typeahead_selected(sweden_stream, query, input_element); actual_value = ct.content_typeahead_selected(sweden_stream, query, input_element);
expected_value = "#**Sweden** "; expected_value = "#**Sweden>";
assert.equal(actual_value, expected_value); assert.equal(actual_value, expected_value);
query = "Hello #swed"; query = "Hello #swed";
ct.get_or_set_token_for_testing("swed"); ct.get_or_set_token_for_testing("swed");
actual_value = ct.content_typeahead_selected(sweden_stream, query, input_element); actual_value = ct.content_typeahead_selected(sweden_stream, query, input_element);
expected_value = "Hello #**Sweden** "; expected_value = "Hello #**Sweden>";
assert.equal(actual_value, expected_value); assert.equal(actual_value, expected_value);
query = "#**swed"; query = "#**swed";
ct.get_or_set_token_for_testing("swed"); ct.get_or_set_token_for_testing("swed");
actual_value = ct.content_typeahead_selected(sweden_stream, query, input_element); actual_value = ct.content_typeahead_selected(sweden_stream, query, input_element);
expected_value = "#**Sweden** "; expected_value = "#**Sweden>";
assert.equal(actual_value, expected_value); assert.equal(actual_value, expected_value);
// topic_list // topic_list
@ -816,6 +816,18 @@ test("content_typeahead_selected", ({override}) => {
expected_value = "Hello #**Sweden>testing** "; expected_value = "Hello #**Sweden>testing** ";
assert.equal(actual_value, expected_value); assert.equal(actual_value, expected_value);
query = "Hello #**Sweden>";
ct.get_or_set_token_for_testing("");
actual_value = ct.content_typeahead_selected(
{
topic: "Sweden",
type: "topic_list",
},
query,
input_element,
);
expected_value = "Hello #**Sweden** ";
assert.equal(actual_value, expected_value);
// syntax // syntax
ct.get_or_set_completing_for_tests("syntax"); ct.get_or_set_completing_for_tests("syntax");
@ -1901,7 +1913,8 @@ test("tokenizing", () => {
assert.equal(ct.tokenize_compose_str("foo ~~~why = why_not\n~~~"), "~~~"); assert.equal(ct.tokenize_compose_str("foo ~~~why = why_not\n~~~"), "~~~");
// The following cases are kinda judgment calls... // The following cases are kinda judgment calls...
assert.equal(ct.tokenize_compose_str("foo @toomanycharactersisridiculoustocomplete"), ""); // max scanning limit is 40 characters until chars like @, # , / are found
assert.equal(ct.tokenize_compose_str("foo @toomanycharactersistooridiculoustocomplete"), "");
assert.equal(ct.tokenize_compose_str("foo #bar@foo"), "#bar@foo"); assert.equal(ct.tokenize_compose_str("foo #bar@foo"), "#bar@foo");
}); });