paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-04 22:13:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

compose: Pass a container to which the banner should be applied.

Browse Source 
This fixes banners related to message editing incorrectly appearing
next to the compose box.

Fixes: #25230.
This commit is contained in:
Daniil Fadeev
2023-05-02 16:05:37 +07:00
committed by Tim Abbott
parent c70910b5dc
commit 6dc10f8696
10 changed files with 80 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
web/src/compose.js
View File

@@ -249,6 +249,7 @@ export function send_message(request = create_message_object()) {
compose_banner.show_error_message( compose_banner.show_error_message(
response, response,
compose_banner.CLASSNAMES.generic_compose_error, compose_banner.CLASSNAMES.generic_compose_error,
$("#compose_banners"),
$("#compose-textarea"), $("#compose-textarea"),
); );
// For messages that were not locally echoed, we're // For messages that were not locally echoed, we're
@@ -541,7 +542,9 @@ export function initialize() {
)} .compose_banner_action_button`, )} .compose_banner_action_button`,
(event) => { (event) => {
event.preventDefault(); event.preventDefault();
const $edit_form = $(event.target)
.closest(".message_edit_form")
.find(".edit_form_banners");
const $invite_row = $(event.target).parents(".compose_banner"); const $invite_row = $(event.target).parents(".compose_banner");
const user_id = Number.parseInt($invite_row.data("user-id"), 10); const user_id = Number.parseInt($invite_row.data("user-id"), 10);
@@ -556,6 +559,7 @@ export function initialize() {
compose_banner.show_error_message( compose_banner.show_error_message(
error_msg, error_msg,
compose_banner.CLASSNAMES.generic_compose_error, compose_banner.CLASSNAMES.generic_compose_error,
$edit_form.length ? $edit_form : $("#compose_banners"),
$("#compose-textarea"), $("#compose-textarea"),
); );
$(event.target).prop("disabled", true); $(event.target).prop("disabled", true);

11
web/src/compose_banner.ts
View File

@@ -100,7 +100,12 @@ export function clear_all(): void {
$(`#compose_banners`).empty(); $(`#compose_banners`).empty();
} }
export function show_error_message(message: string, classname: string, $bad_input?: JQuery): void { export function show_error_message(
message: string,
classname: string,
$container: JQuery,
$bad_input?: JQuery,
): void {
$(`#compose_banners .${CSS.escape(classname)}`).remove(); $(`#compose_banners .${CSS.escape(classname)}`).remove();
const new_row = render_compose_banner({ const new_row = render_compose_banner({
@@ -111,7 +116,7 @@ export function show_error_message(message: string, classname: string, $bad_inpu
button_text: null, button_text: null,
classname, classname,
}); });
append_compose_banner_to_banner_list(new_row); append_compose_banner_to_banner_list(new_row, $container);
hide_compose_spinner(); hide_compose_spinner();
@@ -129,7 +134,7 @@ export function show_stream_does_not_exist_error(stream_name: string): void {
stream_name, stream_name,
classname: CLASSNAMES.stream_does_not_exist, classname: CLASSNAMES.stream_does_not_exist,
}); });
append_compose_banner_to_banner_list(new_row); append_compose_banner_to_banner_list(new_row, $("#compose_banners"));
hide_compose_spinner(); hide_compose_spinner();
// A copy of `compose_recipient.open_compose_stream_dropup()` that // A copy of `compose_recipient.open_compose_stream_dropup()` that

1
web/src/compose_recipient.js
View File

@@ -121,6 +121,7 @@ export function check_stream_posting_policy_for_compose_box(stream_name) {
defaultMessage: "You do not have permission to post in this stream.", defaultMessage: "You do not have permission to post in this stream.",
}), }),
compose_banner.CLASSNAMES.no_post_permissions, compose_banner.CLASSNAMES.no_post_permissions,
$("#compose_banners"),
); );
} else { } else {
$(".compose_right_float_container").removeClass("disabled-compose-send-button-container"); $(".compose_right_float_container").removeClass("disabled-compose-send-button-container");

42
web/src/compose_validate.js
View File

@@ -59,7 +59,7 @@ export function needs_subscribe_warning(user_id, stream_id) {
return true; return true;
} }
export function warn_if_private_stream_is_linked(linked_stream) { export function warn_if_private_stream_is_linked(linked_stream, $textarea) {
// For PMs, we currently don't warn about links to private // For PMs, we currently don't warn about links to private
// streams, since you are specifically sharing the existence of // streams, since you are specifically sharing the existence of
// the private stream with someone. One could imagine changing // the private stream with someone. One could imagine changing
@@ -104,11 +104,11 @@ export function warn_if_private_stream_is_linked(linked_stream) {
stream_name: linked_stream.name, stream_name: linked_stream.name,
classname: compose_banner.CLASSNAMES.private_stream_warning, classname: compose_banner.CLASSNAMES.private_stream_warning,
}); });
const $container = compose_banner.get_compose_banner_container($textarea);
compose_banner.append_compose_banner_to_banner_list(new_row); compose_banner.append_compose_banner_to_banner_list(new_row, $container);
} }
export function warn_if_mentioning_unsubscribed_user(mentioned) { export function warn_if_mentioning_unsubscribed_user(mentioned, $textarea) {
if (compose_state.get_message_type() !== "stream") { if (compose_state.get_message_type() !== "stream") {
return; return;
} }
@@ -137,8 +137,9 @@ export function warn_if_mentioning_unsubscribed_user(mentioned) {
} }
if (needs_subscribe_warning(user_id, sub.stream_id)) { if (needs_subscribe_warning(user_id, sub.stream_id)) {
const $existing_invites_area = $( const $banner_container = compose_banner.get_compose_banner_container($textarea);
`#compose_banners .${CSS.escape(compose_banner.CLASSNAMES.recipient_not_subscribed)}`, const $existing_invites_area = $banner_container.find(
`.${CSS.escape(compose_banner.CLASSNAMES.recipient_not_subscribed)}`,
); );
const existing_invites = [...$existing_invites_area].map((user_row) => const existing_invites = [...$existing_invites_area].map((user_row) =>
@@ -161,7 +162,8 @@ export function warn_if_mentioning_unsubscribed_user(mentioned) {
}; };
const new_row = render_not_subscribed_warning(context); const new_row = render_not_subscribed_warning(context);
compose_banner.append_compose_banner_to_banner_list(new_row); const $container = compose_banner.get_compose_banner_container($textarea);
compose_banner.append_compose_banner_to_banner_list(new_row, $container);
} }
} }
} }
@@ -221,7 +223,7 @@ export function warn_if_topic_resolved(topic_changed) {
}; };
const new_row = render_compose_banner(context); const new_row = render_compose_banner(context);
compose_banner.append_compose_banner_to_banner_list(new_row); compose_banner.append_compose_banner_to_banner_list(new_row, $("#compose_banners"));
compose_state.set_recipient_viewed_topic_resolved_banner(true); compose_state.set_recipient_viewed_topic_resolved_banner(true);
} else { } else {
clear_topic_resolved_warning(); clear_topic_resolved_warning();
@@ -244,7 +246,10 @@ function show_wildcard_warnings(stream_id) {
// only show one error for any number of @all or @everyone mentions // only show one error for any number of @all or @everyone mentions
if ($(`#compose_banners .${CSS.escape(classname)}`).length === 0) { if ($(`#compose_banners .${CSS.escape(classname)}`).length === 0) {
compose_banner.append_compose_banner_to_banner_list(wildcard_template); compose_banner.append_compose_banner_to_banner_list(
wildcard_template,
$("#compose_banners"),
);
} }
user_acknowledged_wildcard = false; user_acknowledged_wildcard = false;
@@ -363,6 +368,9 @@ function validate_stream_message_mentions(stream_id) {
"You do not have permission to use wildcard mentions in this stream.", "You do not have permission to use wildcard mentions in this stream.",
}), }),
compose_banner.CLASSNAMES.wildcards_not_allowed, compose_banner.CLASSNAMES.wildcards_not_allowed,
// Since we don't trigger validation in edit mode, the only place where
// wildcard warnings will appear is the compose banners container.
$("#compose_banners"),
); );
return false; return false;
} }
@@ -386,6 +394,7 @@ function validate_stream_message_mentions(stream_id) {
} }
export function validation_error(error_type, stream_name) { export function validation_error(error_type, stream_name) {
const $banner_container = $("#compose_banners");
switch (error_type) { switch (error_type) {
case "does-not-exist": case "does-not-exist":
compose_banner.show_stream_does_not_exist_error(stream_name); compose_banner.show_stream_does_not_exist_error(stream_name);
@@ -394,6 +403,7 @@ export function validation_error(error_type, stream_name) {
compose_banner.show_error_message( compose_banner.show_error_message(
$t({defaultMessage: "Error checking subscription."}), $t({defaultMessage: "Error checking subscription."}),
compose_banner.CLASSNAMES.subscription_error, compose_banner.CLASSNAMES.subscription_error,
$banner_container,
$("#compose_select_recipient_widget"), $("#compose_select_recipient_widget"),
); );
return false; return false;
@@ -419,7 +429,7 @@ export function validation_error(error_type, stream_name) {
// closing the banner would be more confusing than helpful. // closing the banner would be more confusing than helpful.
hide_close_button: true, hide_close_button: true,
}); });
compose_banner.append_compose_banner_to_banner_list(new_row); compose_banner.append_compose_banner_to_banner_list(new_row, $banner_container);
return false; return false;
} }
} }
@@ -437,10 +447,12 @@ export function validate_stream_message_address_info(stream_name) {
function validate_stream_message() { function validate_stream_message() {
const stream_name = compose_state.stream_name(); const stream_name = compose_state.stream_name();
const $banner_container = $("#compose_banners");
if (stream_name === "") { if (stream_name === "") {
compose_banner.show_error_message( compose_banner.show_error_message(
$t({defaultMessage: "Please specify a stream."}), $t({defaultMessage: "Please specify a stream."}),
compose_banner.CLASSNAMES.missing_stream, compose_banner.CLASSNAMES.missing_stream,
$banner_container,
$("#compose_select_recipient_widget"), $("#compose_select_recipient_widget"),
); );
return false; return false;
@@ -454,6 +466,7 @@ function validate_stream_message() {
compose_banner.show_error_message( compose_banner.show_error_message(
$t({defaultMessage: "Topics are required in this organization."}), $t({defaultMessage: "Topics are required in this organization."}),
compose_banner.CLASSNAMES.topic_missing, compose_banner.CLASSNAMES.topic_missing,
$banner_container,
$("#stream_message_recipient_topic"), $("#stream_message_recipient_topic"),
); );
return false; return false;
@@ -471,6 +484,7 @@ function validate_stream_message() {
defaultMessage: "You do not have permission to post in this stream.", defaultMessage: "You do not have permission to post in this stream.",
}), }),
compose_banner.CLASSNAMES.no_post_permissions, compose_banner.CLASSNAMES.no_post_permissions,
$banner_container,
); );
return false; return false;
} }
@@ -494,6 +508,7 @@ function validate_stream_message() {
// for now) // for now)
function validate_private_message() { function validate_private_message() {
const user_ids = compose_pm_pill.get_user_ids(); const user_ids = compose_pm_pill.get_user_ids();
const $banner_container = $("#compose_banners");
if ( if (
page_params.realm_private_message_policy === page_params.realm_private_message_policy ===
@@ -504,6 +519,7 @@ function validate_private_message() {
compose_banner.show_error_message( compose_banner.show_error_message(
$t({defaultMessage: "Direct messages are disabled in this organization."}), $t({defaultMessage: "Direct messages are disabled in this organization."}),
compose_banner.CLASSNAMES.private_messages_disabled, compose_banner.CLASSNAMES.private_messages_disabled,
$banner_container,
$("#private_message_recipient"), $("#private_message_recipient"),
); );
return false; return false;
@@ -513,6 +529,7 @@ function validate_private_message() {
compose_banner.show_error_message( compose_banner.show_error_message(
$t({defaultMessage: "Please specify at least one valid recipient."}), $t({defaultMessage: "Please specify at least one valid recipient."}),
compose_banner.CLASSNAMES.missing_private_message_recipient, compose_banner.CLASSNAMES.missing_private_message_recipient,
$banner_container,
$("#private_message_recipient"), $("#private_message_recipient"),
); );
return false; return false;
@@ -529,6 +546,7 @@ function validate_private_message() {
compose_banner.show_error_message( compose_banner.show_error_message(
$t({defaultMessage: "The recipient {recipient} is not valid."}, context), $t({defaultMessage: "The recipient {recipient} is not valid."}, context),
compose_banner.CLASSNAMES.invalid_recipient, compose_banner.CLASSNAMES.invalid_recipient,
$banner_container,
$("#private_message_recipient"), $("#private_message_recipient"),
); );
return false; return false;
@@ -537,6 +555,7 @@ function validate_private_message() {
compose_banner.show_error_message( compose_banner.show_error_message(
$t({defaultMessage: "The recipients {recipients} are not valid."}, context), $t({defaultMessage: "The recipients {recipients} are not valid."}, context),
compose_banner.CLASSNAMES.invalid_recipients, compose_banner.CLASSNAMES.invalid_recipients,
$banner_container,
$("#private_message_recipient"), $("#private_message_recipient"),
); );
return false; return false;
@@ -548,6 +567,7 @@ function validate_private_message() {
compose_banner.show_error_message( compose_banner.show_error_message(
$t({defaultMessage: "You cannot send messages to deactivated users."}, context), $t({defaultMessage: "You cannot send messages to deactivated users."}, context),
compose_banner.CLASSNAMES.deactivated_user, compose_banner.CLASSNAMES.deactivated_user,
$banner_container,
$("#private_message_recipient"), $("#private_message_recipient"),
); );
@@ -579,6 +599,7 @@ export function check_overflow_text() {
{max_length}, {max_length},
), ),
compose_banner.CLASSNAMES.message_too_long, compose_banner.CLASSNAMES.message_too_long,
$("#compose_banners"),
); );
$("#compose-send-button").prop("disabled", true); $("#compose-send-button").prop("disabled", true);
} else if (text.length > 0.9 * max_length) { } else if (text.length > 0.9 * max_length) {
@@ -622,6 +643,7 @@ export function validate() {
"You need to be running Zephyr mirroring in order to send messages!", "You need to be running Zephyr mirroring in order to send messages!",
}), }),
compose_banner.CLASSNAMES.zephyr_not_running, compose_banner.CLASSNAMES.zephyr_not_running,
$("#compose_banners"),
); );
return false; return false;
} }

4
web/src/composebox_typeahead.js
View File

@@ -821,7 +821,7 @@ export function content_typeahead_selected(item, event) {
); );
beginning += mention_text + " "; beginning += mention_text + " ";
if (!is_silent) { if (!is_silent) {
compose_validate.warn_if_mentioning_unsubscribed_user(item); compose_validate.warn_if_mentioning_unsubscribed_user(item, $textbox);
} }
} }
break; break;
@@ -848,7 +848,7 @@ export function content_typeahead_selected(item, event) {
} else { } else {
beginning += "** "; beginning += "** ";
} }
compose_validate.warn_if_private_stream_is_linked(item); compose_validate.warn_if_private_stream_is_linked(item, $textbox);
break; break;
case "syntax": { case "syntax": {
// Isolate the end index of the triple backticks/tildes, including // Isolate the end index of the triple backticks/tildes, including

7
web/src/notifications.js
View File

@@ -178,7 +178,10 @@ function notify_unmute(muted_narrow, stream_id, topic_name) {
}), }),
); );
compose_banner.clear_unmute_topic_notifications(); compose_banner.clear_unmute_topic_notifications();
compose_banner.append_compose_banner_to_banner_list($unmute_notification); compose_banner.append_compose_banner_to_banner_list(
$unmute_notification,
$("#compose_banners"),
);
} }
export function notify_above_composebox( export function notify_above_composebox(
@@ -200,7 +203,7 @@ export function notify_above_composebox(
// We pass in include_unmute_banner as false because we don't want to // We pass in include_unmute_banner as false because we don't want to
// clear any unmute_banner associated with this same message. // clear any unmute_banner associated with this same message.
compose_banner.clear_message_sent_banners(false); compose_banner.clear_message_sent_banners(false);
compose_banner.append_compose_banner_to_banner_list($notification); compose_banner.append_compose_banner_to_banner_list($notification, $("#compose_banners"));
} }
if (window.electron_bridge !== undefined) { if (window.electron_bridge !== undefined) {

4
web/src/scheduled_messages.js
View File

@@ -93,6 +93,7 @@ export function open_scheduled_message_in_compose(scheduled_msg) {
} }
export function send_request_to_schedule_message(scheduled_message_data, deliver_at) { export function send_request_to_schedule_message(scheduled_message_data, deliver_at) {
const $banner_container = $("#compose_banners");
const success = function (data) { const success = function (data) {
drafts.draft_model.deleteDraft($("#compose-textarea").data("draft-id")); drafts.draft_model.deleteDraft($("#compose-textarea").data("draft-id"));
compose.clear_compose_box(); compose.clear_compose_box();
@@ -101,7 +102,7 @@ export function send_request_to_schedule_message(scheduled_message_data, deliver
deliver_at, deliver_at,
}); });
compose_banner.clear_message_sent_banners(); compose_banner.clear_message_sent_banners();
compose_banner.append_compose_banner_to_banner_list(new_row); compose_banner.append_compose_banner_to_banner_list(new_row, $banner_container);
}; };
const error = function (xhr) { const error = function (xhr) {
@@ -110,6 +111,7 @@ export function send_request_to_schedule_message(scheduled_message_data, deliver
compose_banner.show_error_message( compose_banner.show_error_message(
response, response,
compose_banner.CLASSNAMES.generic_compose_error, compose_banner.CLASSNAMES.generic_compose_error,
$banner_container,
$("#compose-textarea"), $("#compose-textarea"),
); );
}; };

11
web/src/upload.js
View File

@@ -123,13 +123,10 @@ function add_upload_banner(config, banner_type, banner_text, file_id) {
banner_text, banner_text,
file_id, file_id,
}); });
// TODO: Extend compose_banner.append_compose_banner_to_banner_list compose_banner.append_compose_banner_to_banner_list(
// to support the message edit container too. new_banner,
if (config.mode === "compose") { get_item("banner_container", config),
compose_banner.append_compose_banner_to_banner_list(new_banner); );
} else {
get_item("banner_container", config).append(new_banner);
}
} }
export function show_error_message( export function show_error_message(

8
web/src/zcommand.js
View File

@@ -1,3 +1,5 @@
import $ from "jquery";
import * as channel from "./channel"; import * as channel from "./channel";
import * as compose_banner from "./compose_banner"; import * as compose_banner from "./compose_banner";
import * as dark_theme from "./dark_theme"; import * as dark_theme from "./dark_theme";
@@ -48,7 +50,11 @@ export function send(opts) {
export function tell_user(msg) { export function tell_user(msg) {
// This is a bit hacky, but we don't have a super easy API now // This is a bit hacky, but we don't have a super easy API now
// for just telling users stuff. // for just telling users stuff.
compose_banner.show_error_message(msg, compose_banner.CLASSNAMES.generic_compose_error); compose_banner.show_error_message(
msg,
compose_banner.CLASSNAMES.generic_compose_error,
$("#compose_banners"),
);
} }
export function switch_to_light_theme() { export function switch_to_light_theme() {

21
web/tests/compose_validate.test.js
View File

@@ -613,6 +613,7 @@ test_ui("needs_subscribe_warning", () => {
}); });
test_ui("warn_if_private_stream_is_linked", ({mock_template}) => { test_ui("warn_if_private_stream_is_linked", ({mock_template}) => {
const $textarea = $("<textarea>").attr("id", "compose-textarea");
const test_sub = { const test_sub = {
name: compose_state.stream_name(), name: compose_state.stream_name(),
stream_id: 99, stream_id: 99,
@@ -641,7 +642,7 @@ test_ui("warn_if_private_stream_is_linked", ({mock_template}) => {
banner_rendered = false; banner_rendered = false;
compose_state.set_message_type("stream"); compose_state.set_message_type("stream");
denmark.invite_only = invite_only; denmark.invite_only = invite_only;
compose_validate.warn_if_private_stream_is_linked(denmark); compose_validate.warn_if_private_stream_is_linked(denmark, $textarea);
assert.ok(!banner_rendered); assert.ok(!banner_rendered);
} }
@@ -660,11 +661,12 @@ test_ui("warn_if_private_stream_is_linked", ({mock_template}) => {
}; };
stream_data.add_sub(denmark); stream_data.add_sub(denmark);
banner_rendered = false; banner_rendered = false;
compose_validate.warn_if_private_stream_is_linked(denmark); compose_validate.warn_if_private_stream_is_linked(denmark, $textarea);
assert.ok(banner_rendered); assert.ok(banner_rendered);
}); });
test_ui("warn_if_mentioning_unsubscribed_user", ({override, mock_template}) => { test_ui("warn_if_mentioning_unsubscribed_user", ({override, mock_template}) => {
const $textarea = $("<textarea>").attr("id", "compose-textarea");
stream_value = ""; stream_value = "";
override(settings_data, "user_can_subscribe_other_users", () => true); override(settings_data, "user_can_subscribe_other_users", () => true);
@@ -687,7 +689,7 @@ test_ui("warn_if_mentioning_unsubscribed_user", ({override, mock_template}) => {
compose_state.set_message_type(msg_type); compose_state.set_message_type(msg_type);
page_params.realm_is_zephyr_mirror_realm = is_zephyr_mirror; page_params.realm_is_zephyr_mirror_realm = is_zephyr_mirror;
mentioned_details.is_broadcast = is_broadcast; mentioned_details.is_broadcast = is_broadcast;
compose_validate.warn_if_mentioning_unsubscribed_user(mentioned_details); compose_validate.warn_if_mentioning_unsubscribed_user(mentioned_details, $textarea);
assert.ok(!new_banner_rendered); assert.ok(!new_banner_rendered);
} }
@@ -702,7 +704,7 @@ test_ui("warn_if_mentioning_unsubscribed_user", ({override, mock_template}) => {
// Test with empty stream name in compose box. It should return noop. // Test with empty stream name in compose box. It should return noop.
new_banner_rendered = false; new_banner_rendered = false;
assert.equal(compose_state.stream_name(), ""); assert.equal(compose_state.stream_name(), "");
compose_validate.warn_if_mentioning_unsubscribed_user(mentioned_details); compose_validate.warn_if_mentioning_unsubscribed_user(mentioned_details, $textarea);
assert.ok(!new_banner_rendered); assert.ok(!new_banner_rendered);
compose_state.set_stream_name("random"); compose_state.set_stream_name("random");
@@ -713,7 +715,7 @@ test_ui("warn_if_mentioning_unsubscribed_user", ({override, mock_template}) => {
// Test with invalid stream in compose box. It should return noop. // Test with invalid stream in compose box. It should return noop.
new_banner_rendered = false; new_banner_rendered = false;
compose_validate.warn_if_mentioning_unsubscribed_user(mentioned_details); compose_validate.warn_if_mentioning_unsubscribed_user(mentioned_details, $textarea);
assert.ok(!new_banner_rendered); assert.ok(!new_banner_rendered);
// Test mentioning a user that should gets a warning. // Test mentioning a user that should gets a warning.
@@ -726,8 +728,10 @@ test_ui("warn_if_mentioning_unsubscribed_user", ({override, mock_template}) => {
stream_data.add_sub(sub); stream_data.add_sub(sub);
new_banner_rendered = false; new_banner_rendered = false;
$("#compose_banners .recipient_not_subscribed").length = 0; const $banner_container = $("#compose_banners");
compose_validate.warn_if_mentioning_unsubscribed_user(mentioned_details); $banner_container.set_find_results(".recipient_not_subscribed", []);
compose_validate.warn_if_mentioning_unsubscribed_user(mentioned_details, $textarea);
assert.ok(new_banner_rendered); assert.ok(new_banner_rendered);
// Simulate that the row was added to the DOM. // Simulate that the row was added to the DOM.
@@ -743,7 +747,8 @@ test_ui("warn_if_mentioning_unsubscribed_user", ({override, mock_template}) => {
// Now try to mention the same person again. The template should // Now try to mention the same person again. The template should
// not render. // not render.
new_banner_rendered = false; new_banner_rendered = false;
compose_validate.warn_if_mentioning_unsubscribed_user(mentioned_details); $banner_container.set_find_results(".recipient_not_subscribed", $warning_row);
compose_validate.warn_if_mentioning_unsubscribed_user(mentioned_details, $textarea);
assert.ok(!new_banner_rendered); assert.ok(!new_banner_rendered);
}); });