diff --git a/web/src/compose.js b/web/src/compose.js index 9b4d5d3b20..486557ff61 100644 --- a/web/src/compose.js +++ b/web/src/compose.js @@ -9,6 +9,7 @@ import * as channel from "./channel"; import * as compose_actions from "./compose_actions"; import * as compose_banner from "./compose_banner"; import {get_recipient_label} from "./compose_closed_ui"; +import * as compose_recipient from "./compose_recipient"; import * as compose_state from "./compose_state"; import * as compose_ui from "./compose_ui"; import * as compose_validate from "./compose_validate"; @@ -766,11 +767,11 @@ export function initialize() { }); $("#compose-textarea").on("focus", () => { - compose_actions.update_placeholder_text(); + compose_recipient.update_placeholder_text(); }); $("#stream_message_recipient_topic").on("focus", () => { - compose_actions.update_placeholder_text(); + compose_recipient.update_placeholder_text(); }); $("body").on("click", ".formatting_button", (e) => { diff --git a/web/src/compose_actions.js b/web/src/compose_actions.js index 97b1424de1..e8a5c83501 100644 --- a/web/src/compose_actions.js +++ b/web/src/compose_actions.js @@ -48,69 +48,12 @@ function hide_box() { $("#compose_controls").show(); } -function get_focus_area(msg_type, opts) { - // Set focus to "Topic" when narrowed to a stream+topic and "New topic" button clicked. - if (msg_type === "stream" && opts.stream && !opts.topic) { - return "#stream_message_recipient_topic"; - } else if ( - (msg_type === "stream" && opts.stream) || - (msg_type === "private" && opts.private_message_recipient) - ) { - if (opts.trigger === "new topic button") { - return "#stream_message_recipient_topic"; - } - return "#compose-textarea"; - } - - if (msg_type === "stream") { - return "#compose_select_recipient_widget"; - } - return "#private_message_recipient"; -} - -// Export for testing -export const _get_focus_area = get_focus_area; - -export function set_focus(msg_type, opts) { - if (window.getSelection().toString() === "" || opts.trigger !== "message click") { - const focus_area = get_focus_area(msg_type, opts); - const $elt = $(focus_area); - $elt.trigger("focus").trigger("select"); - } -} - -export function show_compose_box(msg_type, opts) { - if (msg_type === "stream") { - $("#compose-direct-recipient").hide(); - $("#stream_message_recipient_topic").show(); - $("#stream_toggle").addClass("active"); - $("#private_message_toggle").removeClass("active"); - $("#compose-recipient").removeClass("compose-recipient-direct-selected"); - } else { - $("#compose-direct-recipient").show(); - $("#stream_message_recipient_topic").hide(); - $("#stream_toggle").removeClass("active"); - $("#private_message_toggle").addClass("active"); - $("#compose-recipient").addClass("compose-recipient-direct-selected"); - // TODO: When "Direct message" is selected, we show "DM" on the dropdown - // button. It would be nice if the dropdown supported a way to attach - // the "DM" button display string so we wouldn't have to manually change - // it here. - const direct_message_label = $t({defaultMessage: "DM"}); - $("#compose_select_recipient_name").html( - ` ${direct_message_label}`, - ); - } - compose_banner.clear_errors(); - compose_banner.clear_warnings(); +function show_compose_box(msg_type, opts) { + compose_recipient.update_compose_for_message_type(msg_type, opts); $("#compose").css({visibility: "visible"}); // When changing this, edit the 42px in _maybe_autoscroll $(".new_message_textarea").css("min-height", "3em"); - - if (opts.trigger === "toggle recipient type") { - update_placeholder_text(); - } - set_focus(msg_type, opts); + compose_ui.set_focus(msg_type, opts); } export function clear_textarea() { @@ -164,7 +107,7 @@ export function complete_starting_tasks(msg_type, opts) { compose_fade.start_compose(msg_type); stream_bar.decorate(opts.stream, $("#stream_message_recipient_topic .message_header_stream")); $(document).trigger(new $.Event("compose_started.zulip", opts)); - update_placeholder_text(); + compose_recipient.update_placeholder_text(); compose_recipient.update_narrow_to_recipient_visibility(); } @@ -220,22 +163,6 @@ function same_recipient_as_before(msg_type, opts) { ); } -export function update_placeholder_text() { - // Change compose placeholder text only if compose box is open. - if (!$("#compose-textarea").is(":visible")) { - return; - } - - const opts = { - message_type: compose_state.get_message_type(), - stream: compose_state.stream_name(), - topic: compose_state.topic(), - private_message_recipient: compose_pm_pill.get_emails(), - }; - - $("#compose-textarea").attr("placeholder", compose_ui.compute_placeholder_text(opts)); -} - export function start(msg_type, opts) { if (page_params.is_spectator) { spectators.login_to_access(); diff --git a/web/src/compose_pm_pill.js b/web/src/compose_pm_pill.js index dea9a5e881..fdd5f8db73 100644 --- a/web/src/compose_pm_pill.js +++ b/web/src/compose_pm_pill.js @@ -1,6 +1,6 @@ import $ from "jquery"; -import * as compose_actions from "./compose_actions"; +import * as compose_recipient from "./compose_recipient"; import * as input_pill from "./input_pill"; import * as people from "./people"; import * as user_pill from "./user_pill"; @@ -29,11 +29,11 @@ export function initialize() { widget = initialize_pill(); widget.onPillCreate(() => { - compose_actions.update_placeholder_text(); + compose_recipient.update_placeholder_text(); }); widget.onPillRemove(() => { - compose_actions.update_placeholder_text(); + compose_recipient.update_placeholder_text(); }); } diff --git a/web/src/compose_recipient.js b/web/src/compose_recipient.js index c98168c79f..1d2be36038 100644 --- a/web/src/compose_recipient.js +++ b/web/src/compose_recipient.js @@ -3,11 +3,11 @@ import $ from "jquery"; import _ from "lodash"; -// todo: fix circular import here -import * as compose_actions from "./compose_actions"; import * as compose_banner from "./compose_banner"; import * as compose_fade from "./compose_fade"; +import * as compose_pm_pill from "./compose_pm_pill"; import * as compose_state from "./compose_state"; +import * as compose_ui from "./compose_ui"; import * as compose_validate from "./compose_validate"; import {DropdownListWidget} from "./dropdown_list_widget"; import {$t} from "./i18n"; @@ -134,12 +134,39 @@ function switch_message_type(message_type) { const opts = { message_type, - trigger: "toggle recipient type", stream: compose_state.stream_name(), topic: compose_state.topic(), private_message_recipient: compose_state.private_message_recipient(), }; - compose_actions.show_compose_box(message_type, opts); + update_compose_for_message_type(message_type, opts); + update_placeholder_text(); + compose_ui.set_focus(message_type, opts); +} + +export function update_compose_for_message_type(message_type) { + if (message_type === "stream") { + $("#compose-direct-recipient").hide(); + $("#stream_message_recipient_topic").show(); + $("#stream_toggle").addClass("active"); + $("#private_message_toggle").removeClass("active"); + $("#compose-recipient").removeClass("compose-recipient-direct-selected"); + } else { + $("#compose-direct-recipient").show(); + $("#stream_message_recipient_topic").hide(); + $("#stream_toggle").removeClass("active"); + $("#private_message_toggle").addClass("active"); + $("#compose-recipient").addClass("compose-recipient-direct-selected"); + // TODO: When "Direct message" is selected, we show "DM" on the dropdown + // button. It would be nice if the dropdown supported a way to attach + // the "DM" button display string so we wouldn't have to manually change + // it here. + const direct_message_label = $t({defaultMessage: "DM"}); + $("#compose_select_recipient_name").html( + ` ${direct_message_label}`, + ); + } + compose_banner.clear_errors(); + compose_banner.clear_warnings(); } export function on_compose_select_recipient_update(new_value) { @@ -246,3 +273,21 @@ export function initialize() { compose_state.set_recipient_edited_manually(true); }); } + +export function update_placeholder_text() { + // Change compose placeholder text only if compose box is open. + if (!$("#compose-textarea").is(":visible")) { + return; + } + + const opts = { + message_type: compose_state.get_message_type(), + stream: compose_state.stream_name(), + topic: compose_state.topic(), + // TODO: to remove a circular import, PM recipient needs + // to be calculated in compose_state instead of compose_pm_pill. + private_message_recipient: compose_pm_pill.get_emails(), + }; + + $("#compose-textarea").attr("placeholder", compose_ui.compute_placeholder_text(opts)); +} diff --git a/web/src/compose_ui.js b/web/src/compose_ui.js index 44cc98b203..cbfac1738f 100644 --- a/web/src/compose_ui.js +++ b/web/src/compose_ui.js @@ -33,6 +33,37 @@ export function autosize_textarea($textarea) { } } +function get_focus_area(msg_type, opts) { + // Set focus to "Topic" when narrowed to a stream+topic and "New topic" button clicked. + if (msg_type === "stream" && opts.stream && !opts.topic) { + return "#stream_message_recipient_topic"; + } else if ( + (msg_type === "stream" && opts.stream) || + (msg_type === "private" && opts.private_message_recipient) + ) { + if (opts.trigger === "new topic button") { + return "#stream_message_recipient_topic"; + } + return "#compose-textarea"; + } + + if (msg_type === "stream") { + return "#compose_select_recipient_widget"; + } + return "#private_message_recipient"; +} + +// Export for testing +export const _get_focus_area = get_focus_area; + +export function set_focus(msg_type, opts) { + if (window.getSelection().toString() === "" || opts.trigger !== "message click") { + const focus_area = get_focus_area(msg_type, opts); + const $elt = $(focus_area); + $elt.trigger("focus").trigger("select"); + } +} + export function smart_insert_inline($textarea, syntax) { function is_space(c) { return c === " " || c === "\t" || c === "\n"; diff --git a/web/src/server_events_dispatch.js b/web/src/server_events_dispatch.js index 89a4943216..c7ecc9d0d6 100644 --- a/web/src/server_events_dispatch.js +++ b/web/src/server_events_dispatch.js @@ -8,7 +8,6 @@ import * as blueslip from "./blueslip"; import * as bot_data from "./bot_data"; import {buddy_list} from "./buddy_list"; import * as compose from "./compose"; -import * as compose_actions from "./compose_actions"; import * as compose_fade from "./compose_fade"; import * as compose_pm_pill from "./compose_pm_pill"; import * as compose_recipient from "./compose_recipient"; @@ -827,7 +826,7 @@ export function dispatch_normal_event(event) { // Update the status text in compose box placeholder when opened to self. if (compose_pm_pill.get_user_ids().includes(event.user_id)) { - compose_actions.update_placeholder_text(); + compose_recipient.update_placeholder_text(); } } diff --git a/web/tests/compose.test.js b/web/tests/compose.test.js index b7760c291d..4e2fce9af7 100644 --- a/web/tests/compose.test.js +++ b/web/tests/compose.test.js @@ -591,7 +591,7 @@ test_ui("trigger_submit_compose_form", ({override, override_rewire}) => { assert.ok(compose_finish_checked); }); -test_ui("on_events", ({override}) => { +test_ui("on_events", ({override, override_rewire}) => { mock_stream_header_colorblock(); initialize_handlers({override}); @@ -752,7 +752,7 @@ test_ui("on_events", ({override}) => { stopPropagation: noop, }; - override(compose_actions, "update_placeholder_text", noop); + override_rewire(compose_recipient, "update_placeholder_text", noop); handler(event); diff --git a/web/tests/compose_actions.test.js b/web/tests/compose_actions.test.js index e40a3b34fa..2e7619a830 100644 --- a/web/tests/compose_actions.test.js +++ b/web/tests/compose_actions.test.js @@ -31,6 +31,7 @@ const compose_pm_pill = mock_esm("../src/compose_pm_pill"); const compose_ui = mock_esm("../src/compose_ui", { autosize_textarea: noop, is_full_size: () => false, + set_focus: noop, }); const hash_util = mock_esm("../src/hash_util"); const narrow_state = mock_esm("../src/narrow_state", { @@ -80,7 +81,6 @@ compose_recipient.compose_recipient_widget = { const start = compose_actions.start; const cancel = compose_actions.cancel; -const get_focus_area = compose_actions._get_focus_area; const respond_to_message = compose_actions.respond_to_message; const reply_with_mention = compose_actions.reply_with_mention; const quote_and_reply = compose_actions.quote_and_reply; @@ -126,7 +126,6 @@ test("start", ({override, override_rewire}) => { override_private_message_recipient({override}); override_rewire(compose_actions, "autosize_message_content", () => {}); override_rewire(compose_actions, "expand_compose_box", () => {}); - override_rewire(compose_actions, "set_focus", () => {}); override_rewire(compose_actions, "complete_starting_tasks", () => {}); override_rewire(compose_actions, "blur_compose_inputs", () => {}); override_rewire(compose_actions, "clear_textarea", () => {}); @@ -251,7 +250,6 @@ test("start", ({override, override_rewire}) => { test("respond_to_message", ({override, override_rewire}) => { mock_banners(); - override_rewire(compose_actions, "set_focus", () => {}); override_rewire(compose_actions, "complete_starting_tasks", () => {}); override_rewire(compose_actions, "clear_textarea", () => {}); override_rewire(compose_recipient, "on_compose_select_recipient_update", noop); @@ -296,7 +294,6 @@ test("reply_with_mention", ({override, override_rewire}) => { mock_banners(); mock_stream_header_colorblock(); compose_state.set_message_type("stream"); - override_rewire(compose_actions, "set_focus", () => {}); override_rewire(compose_actions, "complete_starting_tasks", () => {}); override_rewire(compose_actions, "clear_textarea", () => {}); override_private_message_recipient({override}); @@ -351,7 +348,6 @@ test("quote_and_reply", ({disallow, override, override_rewire}) => { }; people.add_active_user(steve); - override_rewire(compose_actions, "set_focus", () => {}); override_rewire(compose_actions, "complete_starting_tasks", () => {}); override_rewire(compose_actions, "clear_textarea", () => {}); override_private_message_recipient({override}); @@ -436,23 +432,6 @@ test("quote_and_reply", ({disallow, override, override_rewire}) => { assert.ok(replaced); }); -test("get_focus_area", () => { - assert.equal(get_focus_area("private", {}), "#private_message_recipient"); - assert.equal( - get_focus_area("private", { - private_message_recipient: "bob@example.com", - }), - "#compose-textarea", - ); - assert.equal(get_focus_area("stream", {}), "#compose_select_recipient_widget"); - assert.equal(get_focus_area("stream", {stream: "fun"}), "#stream_message_recipient_topic"); - assert.equal(get_focus_area("stream", {stream: "fun", topic: "more"}), "#compose-textarea"); - assert.equal( - get_focus_area("stream", {stream: "fun", topic: "more", trigger: "new topic button"}), - "#stream_message_recipient_topic", - ); -}); - test("focus_in_empty_compose", () => { document.activeElement = {id: "compose-textarea"}; compose_state.set_message_type("stream"); diff --git a/web/tests/compose_pm_pill.test.js b/web/tests/compose_pm_pill.test.js index f649b115a9..41cc5b7572 100644 --- a/web/tests/compose_pm_pill.test.js +++ b/web/tests/compose_pm_pill.test.js @@ -6,7 +6,7 @@ const {mock_esm, zrequire} = require("./lib/namespace"); const {run_test} = require("./lib/test"); const $ = require("./lib/zjquery"); -const compose_actions = mock_esm("../src/compose_actions"); +const compose_recipient = mock_esm("../src/compose_recipient"); const input_pill = mock_esm("../src/input_pill"); const people = zrequire("people"); @@ -17,7 +17,7 @@ let pills = { }; run_test("pills", ({override}) => { - override(compose_actions, "update_placeholder_text", () => {}); + override(compose_recipient, "update_placeholder_text", () => {}); const othello = { user_id: 1, @@ -145,7 +145,7 @@ run_test("pills", ({override}) => { // We stub the return value of input_pill.create(), manually add widget functions to it. pills.onPillCreate = (callback) => { // Exercise our callback for line coverage. It is - // just compose_actions.update_placeholder_text(), + // just compose_recipient.update_placeholder_text(), // which we override. callback(); }; diff --git a/web/tests/compose_ui.test.js b/web/tests/compose_ui.test.js index 2992e77302..28a4062d30 100644 --- a/web/tests/compose_ui.test.js +++ b/web/tests/compose_ui.test.js @@ -749,3 +749,21 @@ run_test("right-to-left", () => { assert.equal($textarea.hasClass("rtl"), false); }); + +const get_focus_area = compose_ui._get_focus_area; +run_test("get_focus_area", () => { + assert.equal(get_focus_area("private", {}), "#private_message_recipient"); + assert.equal( + get_focus_area("private", { + private_message_recipient: "bob@example.com", + }), + "#compose-textarea", + ); + assert.equal(get_focus_area("stream", {}), "#compose_select_recipient_widget"); + assert.equal(get_focus_area("stream", {stream: "fun"}), "#stream_message_recipient_topic"); + assert.equal(get_focus_area("stream", {stream: "fun", topic: "more"}), "#compose-textarea"); + assert.equal( + get_focus_area("stream", {stream: "fun", topic: "more", trigger: "new topic button"}), + "#stream_message_recipient_topic", + ); +}); diff --git a/web/tests/compose_validate.test.js b/web/tests/compose_validate.test.js index c67b9604f4..7aeff4da77 100644 --- a/web/tests/compose_validate.test.js +++ b/web/tests/compose_validate.test.js @@ -12,7 +12,6 @@ const {page_params} = require("./lib/zpage_params"); const channel = mock_esm("../src/channel"); -const compose_actions = zrequire("compose_actions"); const compose_banner = zrequire("compose_banner"); const compose_pm_pill = zrequire("compose_pm_pill"); const compose_state = zrequire("compose_state"); @@ -148,7 +147,7 @@ test_ui("validate_stream_message_address_info", ({mock_template}) => { }); test_ui("validate", ({mock_template}) => { - compose_actions.update_placeholder_text = () => {}; + compose_recipient.update_placeholder_text = () => {}; compose_recipient.on_compose_select_recipient_update = () => {}; function initialize_pm_pill() {