From adddb3d54f76914b3f120f7724f30d23b7ccf067 Mon Sep 17 00:00:00 2001 From: N-Shar-ma Date: Tue, 23 Jan 2024 00:33:18 +0530 Subject: [PATCH] notifications: Collapse blockquotes and "user said" paragraphs. Since notifications have limited space for the contents of a message, a quote from a previous message, or elsewhere, can take up most of the notification, leaving little room for the actual message, and reducing the usefulness of the notification. To fix this, we collapse blockquotes and "user said" paragraphs to make space for the actual message. --- web/src/hash_parser.ts | 12 ++++++ web/src/message_notifications.js | 1 + web/src/ui_util.ts | 55 +++++++++++++++++++++++++ web/tests/hash_util.test.js | 36 +++++++++++++++++ web/tests/notifications.test.js | 1 + web/tests/ui_util.test.js | 69 ++++++++++++++++++++++++++++++++ 6 files changed, 174 insertions(+) create mode 100644 web/tests/ui_util.test.js diff --git a/web/src/hash_parser.ts b/web/src/hash_parser.ts index a286fe75d8..85228484a4 100644 --- a/web/src/hash_parser.ts +++ b/web/src/hash_parser.ts @@ -34,6 +34,18 @@ export function get_current_hash_section(): string { return get_hash_section(window.location.hash); } +export function is_same_server_message_link(url: string): boolean { + // A same server message link always has category `narrow`, + // section `stream` or `dm`, and ends with `/near/`, + // where is a sequence of digits. + return ( + get_hash_category(url) === "narrow" && + (get_hash_section(url) === "stream" || get_hash_section(url) === "dm") && + get_nth_hash_section(url, -2) === "near" && + /^\d+$/.test(get_nth_hash_section(url, -1)) + ); +} + export function is_overlay_hash(hash: string): boolean { // Hash changes within this list are overlays and should not unnarrow (etc.) const overlay_list = [ diff --git a/web/src/message_notifications.js b/web/src/message_notifications.js index 545e26f9ea..9fd6b3b516 100644 --- a/web/src/message_notifications.js +++ b/web/src/message_notifications.js @@ -20,6 +20,7 @@ function get_notification_content(message) { const $content = $("
").html(message.content); ui_util.replace_emoji_with_text($content); ui_util.change_katex_to_raw_latex($content); + ui_util.potentially_collapse_quotes($content); spoilers.hide_spoilers_in_notification($content); if ( diff --git a/web/src/ui_util.ts b/web/src/ui_util.ts index c1f3f2428e..839d420a4c 100644 --- a/web/src/ui_util.ts +++ b/web/src/ui_util.ts @@ -1,6 +1,7 @@ import $ from "jquery"; import * as blueslip from "./blueslip"; +import * as hash_parser from "./hash_parser"; import * as keydown_util from "./keydown_util"; // Add functions to this that have no non-trivial @@ -44,6 +45,60 @@ export function change_katex_to_raw_latex($element: JQuery): void { }); } +export function is_user_said_paragraph($element: JQuery): boolean { + // Irrespective of language, the user said paragraph has these exact elements: + // 1. A user mention + // 2. A same server message link ("said") + // 3. A colon (:) + const $user_mention = $element.find(".user-mention"); + if ($user_mention.length !== 1) { + return false; + } + const $message_link = $element.find("a[href]").filter((_index, element) => { + const href = $(element).attr("href")!; + return href ? hash_parser.is_same_server_message_link(href) : false; + }); + if ($message_link.length !== 1) { + return false; + } + const remaining_text = $element + .text() + .replace($user_mention.text(), "") + .replace($message_link.text(), ""); + return remaining_text.trim() === ":"; +} + +export function get_collapsible_status_array($elements: JQuery): boolean[] { + return [...$elements].map( + (element) => $(element).is("blockquote") || is_user_said_paragraph($(element)), + ); +} + +export function potentially_collapse_quotes($element: JQuery): boolean { + const $children = $element.children(); + const collapsible_status = get_collapsible_status_array($children); + + if (collapsible_status.every(Boolean) || collapsible_status.every((x) => !x)) { + // If every element is collapsible or none of them is collapsible, + // we don't collapse any element. + return false; + } + + for (const [index, element] of [...$children].entries()) { + if (collapsible_status[index]) { + if (index > 0 && collapsible_status[index - 1]) { + // If the previous element was also collapsible, remove its text + // to have a single collapsed block instead of multiple in a row. + $(element).text(""); + } else { + // Else, collapse this element. + $(element).text("[…]"); + } + } + } + return true; +} + export function blur_active_element(): void { // this blurs anything that may perhaps be actively focused on. if (document.activeElement instanceof HTMLElement) { diff --git a/web/tests/hash_util.test.js b/web/tests/hash_util.test.js index 1a04e4f7e3..4e657ec7b7 100644 --- a/web/tests/hash_util.test.js +++ b/web/tests/hash_util.test.js @@ -87,6 +87,42 @@ run_test("get_current_nth_hash_section", () => { assert.equal(hash_parser.get_current_nth_hash_section(3), ""); }); +run_test("test_is_same_server_message_link", () => { + const dm_message_link = "#narrow/dm/9,15-dm/near/43"; + assert.equal(hash_parser.is_same_server_message_link(dm_message_link), true); + + const group_message_link = "#narrow/dm/9,16,15-group/near/68"; + assert.equal(hash_parser.is_same_server_message_link(group_message_link), true); + + const stream_message_link = "#narrow/stream/8-design/topic/desktop/near/82"; + assert.equal(hash_parser.is_same_server_message_link(stream_message_link), true); + + const stream_link = "#narrow/stream/8-design"; + assert.equal(hash_parser.is_same_server_message_link(stream_link), false); + + const topic_link = "#narrow/stream/8-design/topic/desktop"; + assert.equal(hash_parser.is_same_server_message_link(topic_link), false); + + const dm_link = "#narrow/dm/15-John"; + assert.equal(hash_parser.is_same_server_message_link(dm_link), false); + + const search_link = "#narrow/search/database"; + assert.equal(hash_parser.is_same_server_message_link(search_link), false); + + const different_server_message_link = + "https://fakechat.zulip.org/#narrow/dm/8,1848,2369-group/near/1717378"; + assert.equal(hash_parser.is_same_server_message_link(different_server_message_link), false); + + const drafts_link = "#drafts"; + assert.equal(hash_parser.is_same_server_message_link(drafts_link), false); + + const empty_link = "#"; + assert.equal(hash_parser.is_same_server_message_link(empty_link), false); + + const non_zulip_link = "https://www.google.com"; + assert.equal(hash_parser.is_same_server_message_link(non_zulip_link), false); +}); + run_test("build_reload_url", () => { window.location.hash = "#settings/profile"; assert.equal(hash_util.build_reload_url(), "+oldhash=settings%2Fprofile"); diff --git a/web/tests/notifications.test.js b/web/tests/notifications.test.js index 15bc318463..d93ade929f 100644 --- a/web/tests/notifications.test.js +++ b/web/tests/notifications.test.js @@ -341,6 +341,7 @@ test("message_is_notifiable", () => { test("basic_notifications", () => { $("
").set_find_results(".emoji", {replaceWith() {}}); $("
").set_find_results("span.katex", {each() {}}); + $("
").children = () => []; let n; // Object for storing all notification data for assertions. let last_closed_message_id = null; diff --git a/web/tests/ui_util.test.js b/web/tests/ui_util.test.js new file mode 100644 index 0000000000..162298ac1c --- /dev/null +++ b/web/tests/ui_util.test.js @@ -0,0 +1,69 @@ +"use strict"; + +const {strict: assert} = require("assert"); + +const {zrequire} = require("./lib/namespace"); +const {run_test} = require("./lib/test"); +const $ = require("./lib/zjquery"); + +const ui_util = zrequire("ui_util"); + +run_test("potentially_collapse_quotes", ({override_rewire}) => { + const $element = $.create("message-content"); + let children = []; + $element.children = () => children; + + children = [ + $.create("normal paragraph 1"), + $.create("blockquote"), + $.create("normal paragraph 2"), + $.create("user said paragraph"), + $.create("message quote"), + $.create("normal paragraph 3"), + ]; + override_rewire(ui_util, "get_collapsible_status_array", () => [ + false, + true, + false, + true, + true, + false, + ]); + // When there are both collapsible and non-collapsible elements, for + // multiple collapsible elements in a row, only the first element + // should be collapsed, and the rest's text should be removed. Non- + // collapsible elements should not be touched. + let collapsed = ui_util.potentially_collapse_quotes($element); + assert.equal(collapsed, true); + let expected_texts = ["never-been-set", "[…]", "never-been-set", "[…]", "", "never-been-set"]; + assert.deepEqual( + $element.children().map(($el) => $el.text()), + expected_texts, + ); + + children = [ + $.create("normal paragraph 4"), + $.create("normal paragraph 5"), + $.create("normal paragraph 6"), + ]; + override_rewire(ui_util, "get_collapsible_status_array", () => [false, false, false]); + // For all non-collapsible elements, none should be collapsed. + collapsed = ui_util.potentially_collapse_quotes($element); + assert.equal(collapsed, false); + expected_texts = ["never-been-set", "never-been-set", "never-been-set"]; + assert.deepEqual( + $element.children().map(($el) => $el.text()), + expected_texts, + ); + + children = [$.create("blockquote 1"), $.create("blockquote 2"), $.create("blockquote 3")]; + override_rewire(ui_util, "get_collapsible_status_array", () => [true, true, true]); + // For all collapsible elements, none should be collapsed. + collapsed = ui_util.potentially_collapse_quotes($element); + assert.equal(collapsed, false); + expected_texts = ["never-been-set", "never-been-set", "never-been-set"]; + assert.deepEqual( + $element.children().map(($el) => $el.text()), + expected_texts, + ); +});