From 39d123e355514d8b46cedababa2a393286a82807 Mon Sep 17 00:00:00 2001 From: Aman Agrawal Date: Sun, 20 Sep 2020 13:44:24 +0530 Subject: [PATCH] recent_topics: Make it the default view. Go to Recent Topics on "#", no hash and "#recent_topics". Go to Recent Topics as the last destination for escape key. Map `a` key to All messages and change its hash to `#all_messages`. --- docs/subsystems/pointer.md | 2 +- docs/testing/manual-testing.md | 4 ++-- frontend_tests/node_tests/hashchange.js | 6 +++++- frontend_tests/node_tests/hotkey.js | 6 +++++- frontend_tests/puppeteer_lib/common.ts | 2 +- frontend_tests/puppeteer_tests/02-message-basics.ts | 4 +++- frontend_tests/puppeteer_tests/03-compose.ts | 4 +++- frontend_tests/puppeteer_tests/05-stars.ts | 4 +++- frontend_tests/puppeteer_tests/06-edit.ts | 2 ++ frontend_tests/puppeteer_tests/07-navigation.ts | 4 ++-- frontend_tests/puppeteer_tests/09-mention.ts | 2 ++ frontend_tests/puppeteer_tests/12-drafts.ts | 2 ++ frontend_tests/puppeteer_tests/13-delete-message.ts | 2 ++ frontend_tests/puppeteer_tests/14-copy-and-paste.ts | 2 ++ static/js/hashchange.js | 8 +++++--- static/js/hotkey.js | 6 +++++- static/js/narrow.js | 2 +- static/js/recent_topics.js | 7 ------- templates/zerver/app/index.html | 2 +- templates/zerver/app/keyboard_shortcuts.html | 8 ++------ templates/zerver/app/left_sidebar.html | 6 +++--- templates/zerver/help/keyboard-shortcuts.md | 5 ++--- templates/zerver/help/reading-strategies.md | 4 ++-- 23 files changed, 56 insertions(+), 38 deletions(-) diff --git a/docs/subsystems/pointer.md b/docs/subsystems/pointer.md index e775d36655..f608c2284a 100644 --- a/docs/subsystems/pointer.md +++ b/docs/subsystems/pointer.md @@ -53,7 +53,7 @@ streams.) ### Unnarrow: previous sequence -When you unnarrow using e.g. the escape key, you will automatically be +When you unnarrow using e.g. the `a` key, you will automatically be taken to the same message that was selected in the All messages view before you narrowed, unless in the narrow you read new messages, in which case you will be jumped forward to the first unread and non-muted diff --git a/docs/testing/manual-testing.md b/docs/testing/manual-testing.md index 9df91a5874..8ecf9739fb 100644 --- a/docs/testing/manual-testing.md +++ b/docs/testing/manual-testing.md @@ -51,7 +51,7 @@ Try using all the navigation hotkeys: Try narrowing from the message view: - Hotkeys - - use Esc to go to All messages + - use a to go to All messages - use s to narrow to a stream (select message first and verify in sidebar) - use S to narrow to the topic (and verify in sidebar) @@ -63,7 +63,7 @@ Try narrowing from the message view: - narrow to a group PM - Click on the Zulip logo - narrow to a topic - - click on the Zulip logo (and verify you're in the All messages view) + - click on the Zulip logo (and verify you're in the Recent topics view) ### Messagebox ### diff --git a/frontend_tests/node_tests/hashchange.js b/frontend_tests/node_tests/hashchange.js index 1d7441aafa..ee62a4ff23 100644 --- a/frontend_tests/node_tests/hashchange.js +++ b/frontend_tests/node_tests/hashchange.js @@ -19,6 +19,7 @@ const hashchange = zrequire("hashchange"); const stream_data = zrequire("stream_data"); zrequire("navigate"); zrequire("recent_topics"); +recent_topics.show = () => {}; set_global("search", { update_button_visibility: () => {}, @@ -38,6 +39,9 @@ const overlays = set_global("overlays", {}); const settings = set_global("settings", {}); const subs = set_global("subs", {}); const ui_util = set_global("ui_util", {}); +set_global("top_left_corner", { + handle_narrow_deactivated: () => {}, +}); run_test("operators_round_trip", () => { let operators; @@ -161,7 +165,7 @@ function test_helper() { run_test("hash_interactions", () => { const helper = test_helper(); - window.location.hash = "#"; + window.location.hash = "#all_messages"; helper.clear_events(); hashchange.initialize(); diff --git a/frontend_tests/node_tests/hotkey.js b/frontend_tests/node_tests/hotkey.js index fcc7c6b591..5f1c3bd731 100644 --- a/frontend_tests/node_tests/hotkey.js +++ b/frontend_tests/node_tests/hotkey.js @@ -19,6 +19,10 @@ const {run_test} = require("../zjsunit/test"); // it calls any external module other than `ui.foo`, it'll crash. // Future work includes making sure it actually does call `ui.foo()`. +// Since all the tests here are based on narrow starting with all_messages. +// We set our default narrow to all messages here. +window.location.hash = "#all_messages"; + const emoji_codes = zrequire("emoji_codes", "generated/emoji/emoji_codes.json"); set_global("navigator", { @@ -206,7 +210,7 @@ run_test("basic_chars", () => { // Unmapped keys should immediately return false, without // calling any functions outside of hotkey.js. - assert_unmapped("abfmoyz"); + assert_unmapped("bfmoyz"); assert_unmapped("BEFHILNOQTUWXYZ"); // We have to skip some checks due to the way the code is diff --git a/frontend_tests/puppeteer_lib/common.ts b/frontend_tests/puppeteer_lib/common.ts index 7e066a4fa1..b1246180f1 100644 --- a/frontend_tests/puppeteer_lib/common.ts +++ b/frontend_tests/puppeteer_lib/common.ts @@ -227,7 +227,7 @@ class CommonUtils { await this.fill_form(page, "#login_form", params); await page.$eval("#login_form", (form) => (form as HTMLFormElement).submit()); - await page.waitForSelector("#zhome .message_row", {visible: true}); + await page.waitForSelector("#recent_topics_filter_buttons", {visible: true}); } async log_out(page: Page): Promise { diff --git a/frontend_tests/puppeteer_tests/02-message-basics.ts b/frontend_tests/puppeteer_tests/02-message-basics.ts index 9f3fd56ebf..b5f909f0ef 100644 --- a/frontend_tests/puppeteer_tests/02-message-basics.ts +++ b/frontend_tests/puppeteer_tests/02-message-basics.ts @@ -76,7 +76,7 @@ async function un_narrow(page: Page): Promise { if (await page.evaluate(() => $(".message_comp").is(":visible"))) { await page.keyboard.press("Escape"); } - await page.keyboard.press("Escape"); + await page.click(".top_left_all_messages"); await page.waitForSelector("#zhome .message_row", {visible: true}); assert.strictEqual(await page.title(), "home - Zulip Dev - Zulip"); } @@ -414,6 +414,8 @@ async function test_users_search(page: Page): Promise { async function message_basic_tests(page: Page): Promise { await common.log_in(page); + await page.click(".top_left_all_messages"); + await page.waitForSelector("#zhome .message_row", {visible: true}); console.log("Sending messages"); await common.send_multiple_messages(page, [ diff --git a/frontend_tests/puppeteer_tests/03-compose.ts b/frontend_tests/puppeteer_tests/03-compose.ts index e2bb417d55..9f623edfc6 100644 --- a/frontend_tests/puppeteer_tests/03-compose.ts +++ b/frontend_tests/puppeteer_tests/03-compose.ts @@ -142,7 +142,7 @@ async function test_send_multirecipient_pm_from_cordelia_pm_narrow(page: Page): }); // Go back to all messages view and make sure all messages are loaded. - await page.keyboard.press("Escape"); + await page.click(".top_left_all_messages"); await page.waitForSelector("#zhome .message_row", {visible: true}); await page.waitForFunction((selector: string) => $(selector).length !== 0, {}, pm_selector); @@ -216,6 +216,8 @@ async function test_markdown_preview(page: Page): Promise { async function compose_tests(page: Page): Promise { await common.log_in(page); + await page.click(".top_left_all_messages"); + await page.waitForSelector("#zhome .message_row", {visible: true}); await test_send_messages(page); await test_keyboard_shortcuts(page); await test_reply_by_click_prepopulates_stream_topic_names(page); diff --git a/frontend_tests/puppeteer_tests/05-stars.ts b/frontend_tests/puppeteer_tests/05-stars.ts index 8a53e730e9..360079f13f 100644 --- a/frontend_tests/puppeteer_tests/05-stars.ts +++ b/frontend_tests/puppeteer_tests/05-stars.ts @@ -31,12 +31,14 @@ async function test_narrow_to_starred_messages(page: Page): Promise { await common.check_messages_sent(page, "zfilt", [["Verona > stars", [message]]]); // Go back to all messages narrow. - await page.keyboard.press("Escape"); + await page.click(".top_left_all_messages"); await page.waitForSelector("#zhome .message_row", {visible: true}); } async function stars_test(page: Page): Promise { await common.log_in(page); + await page.click(".top_left_all_messages"); + await page.waitForSelector("#zhome .message_row", {visible: true}); await common.send_message(page, "stream", { stream: "Verona", topic: "stars", diff --git a/frontend_tests/puppeteer_tests/06-edit.ts b/frontend_tests/puppeteer_tests/06-edit.ts index 78b057f35e..6f9fb15a92 100644 --- a/frontend_tests/puppeteer_tests/06-edit.ts +++ b/frontend_tests/puppeteer_tests/06-edit.ts @@ -78,6 +78,8 @@ async function test_edit_private_message(page: Page): Promise { async function edit_tests(page: Page): Promise { await common.log_in(page); + await page.click(".top_left_all_messages"); + await page.waitForSelector("#zhome .message_row", {visible: true}); await test_stream_message_edit(page); await test_edit_message_with_slash_me(page); diff --git a/frontend_tests/puppeteer_tests/07-navigation.ts b/frontend_tests/puppeteer_tests/07-navigation.ts index d056ffb953..20c5f66b0e 100644 --- a/frontend_tests/puppeteer_tests/07-navigation.ts +++ b/frontend_tests/puppeteer_tests/07-navigation.ts @@ -5,7 +5,7 @@ import type {Page} from "puppeteer"; import common from "../puppeteer_lib/common"; async function wait_for_tab(page: Page, tab: string): Promise { - const tab_slector = `#${CSS.escape(tab)}.tab-pane.active`; + const tab_slector = `#${CSS.escape(tab)}.tab-pane`; await page.waitForSelector(tab_slector, {visible: true}); } @@ -89,7 +89,7 @@ async function navigation_tests(page: Page): Promise { await wait_for_tab(page, "message_feed_container"); await navigate_to_subscriptions(page); - await navigate_to(page, "", "message_feed_container"); + await navigate_to(page, "all_messages", "message_feed_container"); await navigate_to_settings(page); await navigate_to(page, "narrow/is/private", "message_feed_container"); await navigate_to_subscriptions(page); diff --git a/frontend_tests/puppeteer_tests/09-mention.ts b/frontend_tests/puppeteer_tests/09-mention.ts index 8ccb43c0e2..dd990ef451 100644 --- a/frontend_tests/puppeteer_tests/09-mention.ts +++ b/frontend_tests/puppeteer_tests/09-mention.ts @@ -6,6 +6,8 @@ import common from "../puppeteer_lib/common"; async function test_mention(page: Page): Promise { await common.log_in(page); + await page.click(".top_left_all_messages"); + await page.waitForSelector("#zhome .message_row", {visible: true}); await page.keyboard.press("KeyC"); await page.waitForSelector("#compose", {visible: true}); diff --git a/frontend_tests/puppeteer_tests/12-drafts.ts b/frontend_tests/puppeteer_tests/12-drafts.ts index 1e151185a4..cba8f5dfda 100644 --- a/frontend_tests/puppeteer_tests/12-drafts.ts +++ b/frontend_tests/puppeteer_tests/12-drafts.ts @@ -247,6 +247,8 @@ async function test_delete_draft_on_sending(page: Page): Promise { async function drafts_test(page: Page): Promise { await common.log_in(page); + await page.click(".top_left_all_messages"); + await page.waitForSelector("#zhome .message_row", {visible: true}); await test_empty_drafts(page); diff --git a/frontend_tests/puppeteer_tests/13-delete-message.ts b/frontend_tests/puppeteer_tests/13-delete-message.ts index 899f8754ea..3529c8affe 100644 --- a/frontend_tests/puppeteer_tests/13-delete-message.ts +++ b/frontend_tests/puppeteer_tests/13-delete-message.ts @@ -13,6 +13,8 @@ async function click_delete_and_return_last_msg_id(page: Page): Promise { await common.log_in(page); + await page.click(".top_left_all_messages"); + await page.waitForSelector("#zhome .message_row", {visible: true}); const messages_quantitiy = await page.evaluate(() => $("#zhome .message_row").length); const last_message_id = await click_delete_and_return_last_msg_id(page); diff --git a/frontend_tests/puppeteer_tests/14-copy-and-paste.ts b/frontend_tests/puppeteer_tests/14-copy-and-paste.ts index 92ceb3597d..8629edb86c 100644 --- a/frontend_tests/puppeteer_tests/14-copy-and-paste.ts +++ b/frontend_tests/puppeteer_tests/14-copy-and-paste.ts @@ -123,6 +123,8 @@ async function test_copying_messages_from_several_topics(page: Page): Promise { await common.log_in(page); + await page.click(".top_left_all_messages"); + await page.waitForSelector("#zhome .message_row", {visible: true}); await common.send_multiple_messages(page, [ {stream: "Verona", topic: "copy-paste-topic #1", content: "copy paste test A"}, diff --git a/static/js/hashchange.js b/static/js/hashchange.js index 20c54c8ba1..11c65af49a 100644 --- a/static/js/hashchange.js +++ b/static/js/hashchange.js @@ -65,6 +65,7 @@ function activate_home_tab() { const coming_from_recent_topics = maybe_hide_recent_topics(); ui_util.change_tab_to("#message_feed_container"); narrow.deactivate(coming_from_recent_topics); + top_left_corner.handle_narrow_deactivated(); floating_recipient_bar.update(); search.update_button_visibility(); // We need to maybe scroll to the selected message @@ -111,7 +112,7 @@ function do_hashchange_normal(from_reload) { if (operators === undefined) { // If the narrow URL didn't parse, clear // window.location.hash and send them to the home tab - set_hash(""); + set_hash("#all_messages"); activate_home_tab(); return false; } @@ -133,11 +134,12 @@ function do_hashchange_normal(from_reload) { } case "": case "#": - activate_home_tab(); - break; case "#recent_topics": recent_topics.show(); break; + case "#all_messages": + activate_home_tab(); + break; case "#keyboard-shortcuts": case "#message-formatting": case "#search-operators": diff --git a/static/js/hotkey.js b/static/js/hotkey.js index 158fbe1e71..fdc452773f 100644 --- a/static/js/hotkey.js +++ b/static/js/hotkey.js @@ -98,6 +98,7 @@ const keypress_mappings = { 82: {name: "respond_to_author", message_view_only: true}, // 'R' 83: {name: "narrow_by_topic", message_view_only: true}, //'S' 86: {name: "view_selected_stream", message_view_only: false}, //'V' + 97: {name: "all_messages", message_view_only: true}, // 'a' 99: {name: "compose", message_view_only: true}, // 'c' 100: {name: "open_drafts", message_view_only: true}, // 'd' 101: {name: "edit_message", message_view_only: true}, // 'e' @@ -477,7 +478,7 @@ exports.process_hotkey = function (e, hotkey) { case "tab": case "shift_tab": if ( - window.location.hash === "#recent_topics" && + ["#recent_topics", "#", ""].includes(window.location.hash) && !popovers.any_active() && !overlays.is_active() ) { @@ -735,6 +736,9 @@ exports.process_hotkey = function (e, hotkey) { case "open_recent_topics": hashchange.go_to_location("#"); return true; + case "all_messages": + hashchange.go_to_location("#all_messages"); + return true; } // We don't want hotkeys below this to work when recent topics is diff --git a/static/js/narrow.js b/static/js/narrow.js index a000b37cbb..3f4dfab4d5 100644 --- a/static/js/narrow.js +++ b/static/js/narrow.js @@ -770,7 +770,7 @@ function handle_post_narrow_deactivate_processes() { exports.deactivate = function (coming_from_recent_topics = false) { // NOTE: Never call this function independently, - // always use hashchange.go_to_location("") to + // always use hashchange.go_to_location("#all_messages") to // activate All message narrow. /* Switches current_msg_list from narrowed_msg_list to diff --git a/static/js/recent_topics.js b/static/js/recent_topics.js index a3f663805e..d12fb1f0b6 100644 --- a/static/js/recent_topics.js +++ b/static/js/recent_topics.js @@ -503,13 +503,6 @@ exports.change_focused_element = function (e, input_key) { // handle the key. const $elem = $(e.target); - if ($("#recent_topics_table").find(":focus").length === 0) { - // This is a failsafe to return focus back to recent topics overlay, - // in case it loses focus due to some unknown reason. - set_default_focus(); - return false; - } - if (e.target.id === "recent_topics_search") { // Since the search box a text area, we want the browser to handle // Left/Right and selection within the widget; but if the user diff --git a/templates/zerver/app/index.html b/templates/zerver/app/index.html index 719c85659f..a9a3fc8549 100644 --- a/templates/zerver/app/index.html +++ b/templates/zerver/app/index.html @@ -98,7 +98,7 @@
-
+
diff --git a/templates/zerver/app/keyboard_shortcuts.html b/templates/zerver/app/keyboard_shortcuts.html index e097909db1..b5fe488432 100644 --- a/templates/zerver/app/keyboard_shortcuts.html +++ b/templates/zerver/app/keyboard_shortcuts.html @@ -177,7 +177,7 @@ {% trans %}Narrow to all unmuted messages{% endtrans %} - Esc or Ctrl + [ + A {% trans %}Narrow to current compose box recipient{% endtrans %} @@ -241,11 +241,7 @@ {% trans %}View recent topics{% endtrans %} - T - - - {% trans %}Hide recent topics{% endtrans %} - Esc + T or Esc or Ctrl + [
diff --git a/templates/zerver/app/left_sidebar.html b/templates/zerver/app/left_sidebar.html index 891696116c..ebf30f089c 100644 --- a/templates/zerver/app/left_sidebar.html +++ b/templates/zerver/app/left_sidebar.html @@ -2,8 +2,8 @@