From db09639f6ceadd9e1fbc5711166fade1d40a981e Mon Sep 17 00:00:00 2001
From: Aman Agrawal <amanagr@zulip.com>
Date: Fri, 10 Dec 2021 08:07:42 +0000
Subject: [PATCH] compose: Change UI which toggles `enter_sends` setting.

Use a popover which displays both the options instead of long text.
We only use a small text indicating the current state which user
can click on to trigger the popover.
---
 .../node_tests/composebox_typeahead.js        | 23 ---------
 frontend_tests/puppeteer_lib/common.ts        | 10 +++-
 static/js/click_handlers.js                   |  6 +++
 static/js/composebox_typeahead.js             | 17 -------
 static/js/popover_menus.js                    | 50 ++++++++++++++++++-
 static/js/tippyjs.js                          | 14 ++++++
 static/styles/compose.css                     | 40 ++++++++++++++-
 static/styles/dark_theme.css                  | 19 +++++--
 static/templates/compose.hbs                  | 30 +++++------
 ...compose_select_enter_behaviour_popover.hbs | 36 +++++++++++++
 10 files changed, 180 insertions(+), 65 deletions(-)
 create mode 100644 static/templates/compose_select_enter_behaviour_popover.hbs

diff --git a/frontend_tests/node_tests/composebox_typeahead.js b/frontend_tests/node_tests/composebox_typeahead.js
index ba020fbb72..068a4771b9 100644
--- a/frontend_tests/node_tests/composebox_typeahead.js
+++ b/frontend_tests/node_tests/composebox_typeahead.js
@@ -9,7 +9,6 @@ const {page_params, user_settings} = require("../zjsunit/zpage_params");
 
 const noop = () => {};
 
-const channel = mock_esm("../../static/js/channel");
 const compose = mock_esm("../../static/js/compose", {
     finish: noop,
 });
@@ -1142,32 +1141,11 @@ test("initialize", ({override, mock_template}) => {
     event.key = "a";
     $("form#send_message_form").trigger(event);
 
-    // select_on_focus()
-
-    let channel_patch_called = false;
-    override(channel, "patch", (params) => {
-        assert.equal(params.url, "/json/settings");
-        assert.equal(params.idempotent, true);
-        assert.deepEqual(params.data, {enter_sends: user_settings.enter_sends});
-
-        channel_patch_called = true;
-    });
-    user_settings.enter_sends = false;
-    $(".enter_sends").trigger("click");
-    assert.equal(user_settings.enter_sends, true);
-
-    // Now we re-run both .initialize() and the click handler, this time
-    // with enter_sends: user_settings.enter_sends being true
-    user_settings.enter_sends = true;
-    $(".enter_sends").trigger("click");
-    assert.equal(user_settings.enter_sends, false);
-
     $("#stream_message_recipient_stream").off("focus");
     $("#stream_message_recipient_topic").off("focus");
     $("#private_message_recipient").off("focus");
     $("form#send_message_form").off("keydown");
     $("form#send_message_form").off("keyup");
-    $(".enter_sends").off("click");
     $("#private_message_recipient").off("blur");
     ct.initialize();
 
@@ -1176,7 +1154,6 @@ test("initialize", ({override, mock_template}) => {
     assert.ok(stream_typeahead_called);
     assert.ok(subject_typeahead_called);
     assert.ok(pm_recipient_typeahead_called);
-    assert.ok(channel_patch_called);
     assert.ok(compose_textarea_typeahead_called);
 });
 
diff --git a/frontend_tests/puppeteer_lib/common.ts b/frontend_tests/puppeteer_lib/common.ts
index bd4b22ea77..8f73370a24 100644
--- a/frontend_tests/puppeteer_lib/common.ts
+++ b/frontend_tests/puppeteer_lib/common.ts
@@ -265,12 +265,18 @@ class CommonUtils {
     }
 
     async ensure_enter_does_not_send(page: Page): Promise<void> {
+        let enter_sends = false;
         await page.$eval(".enter_sends_false", (el) => {
             if ((el as HTMLElement).style.display !== "none") {
-                // Click events gets propagated to `.enter_sends` which toggles the value.
-                (el as HTMLElement).click();
+                enter_sends = true;
             }
         });
+
+        if (enter_sends) {
+            const enter_sends_false_selector = ".enter_sends_choice input[value='false']";
+            await page.waitForSelector(enter_sends_false_selector);
+            await page.click(enter_sends_false_selector);
+        }
     }
 
     async assert_compose_box_content(page: Page, expected_value: string): Promise<void> {
diff --git a/static/js/click_handlers.js b/static/js/click_handlers.js
index b4d962eb64..8ea8a97513 100644
--- a/static/js/click_handlers.js
+++ b/static/js/click_handlers.js
@@ -660,6 +660,11 @@ export function initialize() {
             return;
         }
 
+        if ($(".enter_sends").has(e.target).length) {
+            e.preventDefault();
+            return;
+        }
+
         // Don't let clicks in the compose area count as
         // "unfocusing" our compose -- in other words, e.g.
         // clicking "Press Enter to send" should not
@@ -872,6 +877,7 @@ export function initialize() {
                 !$(e.target).closest(".micromodal").length &&
                 !$(e.target).closest("[data-tippy-root]").length &&
                 !$(e.target).closest(".modal-backdrop").length &&
+                !$(e.target).closest(".enter_sends").length &&
                 $(e.target).closest("body").length
             ) {
                 // Unfocus our compose area if we click out of it. Don't let exits out
diff --git a/static/js/composebox_typeahead.js b/static/js/composebox_typeahead.js
index ecbc736589..1ce4250ccb 100644
--- a/static/js/composebox_typeahead.js
+++ b/static/js/composebox_typeahead.js
@@ -7,7 +7,6 @@ import pygments_data from "../generated/pygments_data.json";
 import * as emoji from "../shared/js/emoji";
 import * as typeahead from "../shared/js/typeahead";
 
-import * as channel from "./channel";
 import * as compose from "./compose";
 import * as compose_pm_pill from "./compose_pm_pill";
 import * as compose_state from "./compose_state";
@@ -1177,22 +1176,6 @@ export function initialize() {
     $("form#send_message_form").on("keydown", handle_keydown);
     $("form#send_message_form").on("keyup", handle_keyup);
 
-    $(".enter_sends").on("click", () => {
-        user_settings.enter_sends = !user_settings.enter_sends;
-        $(`.enter_sends_${!user_settings.enter_sends}`).hide();
-        $(`.enter_sends_${user_settings.enter_sends}`).show();
-
-        // Refocus in the content box so you can continue typing or
-        // press Enter to send.
-        $("#compose-textarea").trigger("focus");
-
-        return channel.patch({
-            url: "/json/settings",
-            idempotent: true,
-            data: {enter_sends: user_settings.enter_sends},
-        });
-    });
-
     // limit number of items so the list doesn't fall off the screen
     $("#stream_message_recipient_stream").typeahead({
         source() {
diff --git a/static/js/popover_menus.js b/static/js/popover_menus.js
index 5b27caf22d..f1ba9e4e2d 100644
--- a/static/js/popover_menus.js
+++ b/static/js/popover_menus.js
@@ -6,17 +6,22 @@ import $ from "jquery";
 import {delegate} from "tippy.js";
 
 import render_compose_control_buttons_popover from "../templates/compose_control_buttons_popover.hbs";
+import render_compose_select_enter_behaviour_popover from "../templates/compose_select_enter_behaviour_popover.hbs";
 import render_left_sidebar_stream_setting_popover from "../templates/left_sidebar_stream_setting_popover.hbs";
 import render_mobile_message_buttons_popover_content from "../templates/mobile_message_buttons_popover_content.hbs";
 
+import * as channel from "./channel";
+import * as common from "./common";
 import * as compose_actions from "./compose_actions";
 import * as giphy from "./giphy";
 import * as narrow_state from "./narrow_state";
 import * as popovers from "./popovers";
 import * as settings_data from "./settings_data";
+import {user_settings} from "./user_settings";
 
 let left_sidebar_stream_setting_popover_displayed = false;
 let compose_mobile_button_popover_displayed = false;
+export let compose_enter_sends_popover_displayed = false;
 let compose_control_buttons_popover_instance;
 
 export function get_compose_control_buttons_popover() {
@@ -41,7 +46,8 @@ export function any_active() {
     return (
         left_sidebar_stream_setting_popover_displayed ||
         compose_mobile_button_popover_displayed ||
-        compose_control_buttons_popover_instance
+        compose_control_buttons_popover_instance ||
+        compose_enter_sends_popover_displayed
     );
 }
 
@@ -125,4 +131,46 @@ export function initialize() {
             compose_control_buttons_popover_instance = undefined;
         },
     });
+
+    delegate("body", {
+        ...default_popover_props,
+        target: ".enter_sends",
+        placement: "top",
+        onShow(instance) {
+            on_show_prep(instance);
+            instance.setContent(
+                render_compose_select_enter_behaviour_popover({
+                    enter_sends_true: user_settings.enter_sends,
+                }),
+            );
+            compose_enter_sends_popover_displayed = true;
+        },
+        onMount(instance) {
+            common.adjust_mac_shortcuts(".enter_sends_choices kbd");
+
+            $(instance.popper).one("click", ".enter_sends_choice", (e) => {
+                let selected_behaviour = $(e.currentTarget)
+                    .find("input[type='radio']")
+                    .attr("value");
+                selected_behaviour = selected_behaviour === "true"; // Convert to bool
+                user_settings.enter_sends = selected_behaviour;
+                $(`.enter_sends_${!selected_behaviour}`).hide();
+                $(`.enter_sends_${selected_behaviour}`).show();
+
+                // Refocus in the content box so you can continue typing or
+                // press Enter to send.
+                $("#compose-textarea").trigger("focus");
+
+                return channel.patch({
+                    url: "/json/settings",
+                    idempotent: true,
+                    data: {enter_sends: selected_behaviour},
+                });
+            });
+        },
+        onHidden(instance) {
+            instance.destroy();
+            compose_enter_sends_popover_displayed = false;
+        },
+    });
 }
diff --git a/static/js/tippyjs.js b/static/js/tippyjs.js
index 9a70bf0d5b..284ddde20d 100644
--- a/static/js/tippyjs.js
+++ b/static/js/tippyjs.js
@@ -1,7 +1,9 @@
 import $ from "jquery";
 import tippy, {delegate} from "tippy.js";
 
+import {$t} from "./i18n";
 import * as message_lists from "./message_lists";
+import * as popover_menus from "./popover_menus";
 import * as reactions from "./reactions";
 import * as rows from "./rows";
 import * as timerender from "./timerender";
@@ -185,4 +187,16 @@ export function initialize() {
             instance.destroy();
         },
     });
+
+    delegate("body", {
+        target: [".enter_sends_true", ".enter_sends_false"],
+        content: $t({defaultMessage: "Change send shortcut"}),
+        onShow() {
+            // Don't show tooltip if the popover is displayed.
+            if (popover_menus.compose_enter_sends_popover_displayed) {
+                return false;
+            }
+            return true;
+        },
+    });
 }
diff --git a/static/styles/compose.css b/static/styles/compose.css
index 954a64b687..9c46f524a0 100644
--- a/static/styles/compose.css
+++ b/static/styles/compose.css
@@ -441,12 +441,45 @@ input.recipient_box {
     }
 }
 
+.enter_sends_choices {
+    .enter_sends_choice {
+        display: flex;
+        gap: 8px;
+        padding-top: 4px;
+
+        input[type="radio"] {
+            position: relative;
+            top: 5px;
+        }
+
+        &:first-child {
+            padding: 0 0 4px;
+            border-bottom: 1px solid hsla(0, 0%, 0%, 0.2);
+        }
+    }
+
+    .enter_sends_choice_text {
+        display: flex;
+        flex-direction: column;
+    }
+
+    .enter_sends_minor,
+    .enter_sends_minor kbd {
+        opacity: 0.9;
+        font-size: 11px;
+        color: hsl(0, 0%, 50%);
+    }
+}
+
 .enter_sends {
     font-size: 12px;
     height: 14px;
     padding-left: 4px;
     opacity: 0.7;
     margin-bottom: 5px;
+    position: relative;
+    top: -2px;
+    cursor: pointer;
 
     @media (width < $mm_min) {
         font-size: 11px;
@@ -454,6 +487,7 @@ input.recipient_box {
 
     kbd {
         height: 16px;
+        padding: 0 4px;
     }
 
     &:hover {
@@ -465,8 +499,10 @@ input.recipient_box {
         display: none;
     }
 
-    .fa-exchange {
-        margin: 0 4px;
+    i {
+        padding-left: 3px;
+        font-size: 12px;
+        font-weight: 600;
     }
 }
 
diff --git a/static/styles/dark_theme.css b/static/styles/dark_theme.css
index 91dc811d99..c5b7a7070a 100644
--- a/static/styles/dark_theme.css
+++ b/static/styles/dark_theme.css
@@ -63,12 +63,21 @@ body.dark-theme {
         background-color: hsl(211, 28%, 14%);
     }
 
-    .enter_sends kbd {
-        background-color: hsl(211, 29%, 14%);
-        border-color: hsl(211, 29%, 14%);
-        box-shadow: inset 0 -1px 0 hsl(210, 5%, 34%, 0.2);
-        text-shadow: none;
+    .enter_sends,
+    .enter_sends_choices {
         color: hsl(236, 33%, 90%);
+
+        kbd {
+            background-color: hsl(211, 29%, 14%);
+            border-color: hsl(211, 29%, 14%);
+            box-shadow: inset 0 -1px 0 hsl(210, 5%, 34%, 0.2);
+            text-shadow: none;
+            color: hsl(236, 33%, 90%);
+        }
+
+        .enter_sends_minor {
+            color: hsl(0, 0%, 80%);
+        }
     }
 
     table.table-striped thead.table-sticky-headers th {
diff --git a/static/templates/compose.hbs b/static/templates/compose.hbs
index 810721ef96..e8a61500e3 100644
--- a/static/templates/compose.hbs
+++ b/static/templates/compose.hbs
@@ -105,21 +105,21 @@
                                 <div class="compose_bottom_bottom_container">
                                     <span id="compose_limit_indicator"></span>
                                     <div class="enter_sends">
-                                        <label id="enter-sends-label" class="compose_checkbox_label tippy-zulip-tooltip" data-tippy-content="Click to switch">
-                                            <span class="enter_sends_true">
-                                                <kbd>Enter</kbd> {{t 'to send' }}
-                                            </span>
-                                            <span class="enter_sends_false">
-                                                <kbd>Ctrl</kbd> + <kbd>Enter</kbd> {{t 'to send' }}
-                                            </span>
-                                            <i class="fa fa-exchange" aria-hidden="true"></i>
-                                            <span class="enter_sends_true">
-                                                <kbd>Ctrl</kbd> + <kbd>Enter</kbd> {{t 'to add a new line' }}
-                                            </span>
-                                            <span class="enter_sends_false">
-                                                <kbd>Enter</kbd> {{t 'to add a new line' }}
-                                            </span>
-                                        </label>
+                                        <span class="enter_sends_true">
+                                            <kbd>Enter</kbd>
+                                            {{#tr}}
+                                                <kbd class="hide">Enter</kbd>
+                                                <span>to send</span>
+                                            {{/tr}}
+                                        </span>
+                                        <span class="enter_sends_false">
+                                            <kbd>Ctrl</kbd> + <kbd>Enter</kbd>
+                                            {{#tr}}
+                                                <kbd class="hide">Enter</kbd>
+                                                <span>to send</span>
+                                            {{/tr}}
+                                        </span>
+                                        <i class="fa fa-caret-down" aria-hidden="true"></i>
                                     </div>
                                 </div>
                             </div>
diff --git a/static/templates/compose_select_enter_behaviour_popover.hbs b/static/templates/compose_select_enter_behaviour_popover.hbs
new file mode 100644
index 0000000000..3926b70143
--- /dev/null
+++ b/static/templates/compose_select_enter_behaviour_popover.hbs
@@ -0,0 +1,36 @@
+<div class="enter_sends_choices grey-box">
+    <label class="enter_sends_choice">
+        <input type="radio" class="prop-element" value="true"{{#if enter_sends_true }} checked{{/if}} />
+        <div class="enter_sends_choice_text">
+            <span>
+                {{#tr}}
+                    <z-button><kbd>Enter</kbd></z-button>
+                    <span>to send</span>
+                {{/tr}}
+            </span>
+            <span class="enter_sends_minor">
+                {{#tr}}
+                    <z-button><kbd>Ctrl</kbd> + <kbd>Enter</kbd></z-button>
+                    <span>to add a new line</span>
+                {{/tr}}
+            </span>
+        </div>
+    </label>
+    <label class="enter_sends_choice">
+        <input type="radio" class="prop-element" value="false"{{#unless enter_sends_true }} checked{{/unless}} />
+        <div class="enter_sends_choice_text">
+            <span>
+                {{#tr}}
+                    <z-button><kbd>Ctrl</kbd> + <kbd>Enter</kbd></z-button>
+                    <span>to send</span>
+                {{/tr}}
+            </span>
+            <span class="enter_sends_minor">
+                {{#tr}}
+                    <z-button><kbd>Enter</kbd></z-button>
+                    <span>to add a new line</span>
+                {{/tr}}
+            </span>
+        </div>
+    </label>
+</div>