diff --git a/web/e2e-tests/message-basics.test.ts b/web/e2e-tests/message-basics.test.ts index aba918dbe4..2a46397b64 100644 --- a/web/e2e-tests/message-basics.test.ts +++ b/web/e2e-tests/message-basics.test.ts @@ -329,6 +329,9 @@ async function test_stream_search_filters_stream_list(page: Page): Promise // Enter the search box and test highlighted suggestion await page.click(".left-sidebar-search-input"); + // Selection is not highlighted until user wants to move the cursor. + await page.waitForSelector(".top_left_inbox.top_left_row.highlighted_row", {hidden: true}); + await arrow(page, "Down"); await page.waitForSelector(".top_left_inbox.top_left_row.highlighted_row", {visible: true}); await page.waitForSelector((await get_stream_li(page, "Verona")) + " .highlighted_row", { @@ -412,6 +415,9 @@ async function test_users_search(page: Page): Promise { // Enter the search box and test selected suggestion navigation await page.click(".user-list-filter"); + // Selection is not highlighted until user wants to move the cursor. + await page.waitForSelector("#buddy-list-other-users .highlighted_user", {hidden: true}); + await arrow(page, "Down"); await page.waitForSelector("#buddy-list-other-users .highlighted_user", {visible: true}); await assert_selected(page, "Desdemona"); await assert_not_selected(page, "Cordelia, Lear's daughter"); diff --git a/web/src/activity_ui.ts b/web/src/activity_ui.ts index cfcddbddcc..9d87aaf1ac 100644 --- a/web/src/activity_ui.ts +++ b/web/src/activity_ui.ts @@ -300,6 +300,9 @@ export function set_cursor_and_filter(): void { on_focus() { user_cursor!.reset(); }, + set_is_highlight_visible(value: boolean) { + user_cursor!.set_is_highlight_visible(value); + }, }); const $input = user_filter.input_field(); diff --git a/web/src/list_cursor.ts b/web/src/list_cursor.ts index 04acb6d150..27d56d0f14 100644 --- a/web/src/list_cursor.ts +++ b/web/src/list_cursor.ts @@ -15,10 +15,19 @@ export class ListCursor { highlight_class: string; list: List; curr_key?: Key | undefined; + // We only the highlight around the current key if: + // 1. The query is not empty. + // 2. User has to navigate up / down the list. + is_highlight_visible: boolean; constructor({highlight_class, list}: {highlight_class: string; list: List}) { this.highlight_class = highlight_class; this.list = list; + this.is_highlight_visible = false; + } + + set_is_highlight_visible(value: boolean): void { + this.is_highlight_visible = value; } clear(): void { @@ -59,7 +68,9 @@ export class ListCursor { return { highlight: () => { - $li.addClass(this.highlight_class); + if (this.is_highlight_visible) { + $li.addClass(this.highlight_class); + } this.adjust_scroll($li); }, clear: () => { @@ -123,6 +134,18 @@ export class ListCursor { if (this.curr_key === undefined) { return; } + + if (!this.is_highlight_visible) { + // Highlight the current key but + // don't move the current selection. + this.is_highlight_visible = true; + const current_row = this.get_row(this.curr_key); + if (current_row) { + current_row.highlight(); + return; + } + } + const key = this.list.prev_key(this.curr_key); if (key === undefined) { // leave the current key @@ -138,6 +161,18 @@ export class ListCursor { this.reset(); return; } + + if (!this.is_highlight_visible) { + // Highlight the current key but + // don't move the current selection. + this.is_highlight_visible = true; + const current_row = this.get_row(this.curr_key); + if (current_row) { + current_row.highlight(); + return; + } + } + const key = this.list.next_key(this.curr_key); if (key === undefined) { // leave the current key diff --git a/web/src/sidebar_ui.ts b/web/src/sidebar_ui.ts index 5d97d7bf5b..b4a27dae0c 100644 --- a/web/src/sidebar_ui.ts +++ b/web/src/sidebar_ui.ts @@ -576,6 +576,7 @@ export function initialize_left_sidebar_cursor(): void { function actually_update_left_sidebar_for_search(): void { const search_value = ui_util.get_left_sidebar_search_term(); const is_left_sidebar_search_active = search_value !== ""; + left_sidebar_cursor.set_is_highlight_visible(is_left_sidebar_search_active); // Update left sidebar navigation area. update_expanded_views_for_search(search_value); diff --git a/web/src/user_search.ts b/web/src/user_search.ts index a3287cec8f..8c9678f61b 100644 --- a/web/src/user_search.ts +++ b/web/src/user_search.ts @@ -15,11 +15,18 @@ export class UserSearch { _reset_items: () => void; _update_list: () => void; _on_focus: () => void; + _set_is_highlight_visible: (value: boolean) => void; - constructor(opts: {reset_items: () => void; update_list: () => void; on_focus: () => void}) { + constructor(opts: { + reset_items: () => void; + update_list: () => void; + on_focus: () => void; + set_is_highlight_visible: (value: boolean) => void; + }) { this._reset_items = opts.reset_items; this._update_list = opts.update_list; this._on_focus = opts.on_focus; + this._set_is_highlight_visible = opts.set_is_highlight_visible; $("#userlist-header-search .input-close-filter-button").on("click", () => { this.clear_search(); @@ -28,6 +35,7 @@ export class UserSearch { this.$input.on("input", () => { const input_is_empty = this.$input.val() === ""; buddy_data.set_is_searching_users(!input_is_empty); + this._set_is_highlight_visible(!input_is_empty); opts.update_list(); }); this.$input.on("focus", (e) => { @@ -54,6 +62,7 @@ export class UserSearch { clear_search(): void { buddy_data.set_is_searching_users(false); + this._set_is_highlight_visible(false); this.$input.val(""); this.$input.trigger("blur"); this._reset_items(); diff --git a/web/tests/list_cursor.test.cjs b/web/tests/list_cursor.test.cjs index 7abb20dd3e..61881f99ef 100644 --- a/web/tests/list_cursor.test.cjs +++ b/web/tests/list_cursor.test.cjs @@ -73,6 +73,7 @@ run_test("single item list", ({override}) => { override(conf.list, "find_li", () => $li_stub); override(cursor, "adjust_scroll", noop); + cursor.set_is_highlight_visible(true); cursor.go_to(valid_key); // Test prev/next, which should just silently do nothing. @@ -108,6 +109,24 @@ run_test("multiple item list", ({override}) => { cursor.go_to(2); assert.equal(cursor.get_key(), 2); assert.ok(!list_items[1].hasClass("highlight")); + // Selection is not highlighted until we want it to. + assert.ok(!list_items[2].hasClass("highlight")); + assert.ok(!list_items[3].hasClass("highlight")); + + // Current selection is not highlighted until user wants + // to move the cursor. + // Also, cursor doesn't move if the selection was not + // previously highlighted. + cursor.next(); + assert.equal(cursor.get_key(), 2); + assert.ok(!list_items[1].hasClass("highlight")); + assert.ok(list_items[2].hasClass("highlight")); + assert.ok(!list_items[3].hasClass("highlight")); + // Reset state + cursor.set_is_highlight_visible(false); + cursor.prev(); + assert.equal(cursor.get_key(), 2); + assert.ok(!list_items[1].hasClass("highlight")); assert.ok(list_items[2].hasClass("highlight")); assert.ok(!list_items[3].hasClass("highlight"));