mirror of
https://github.com/zulip/zulip.git
synced 2025-11-11 17:36:27 +00:00
compose: Show topic resolve warning only when compose is nonempty.
The resolve topic warning can feel like clutter in the event that the compose box is empty (which often occurs when the user has no intent to send a message), so we configure the validation logic to only display the notice when the compose box is non-empty. We take some care to minimize work the function is doing, beacuse it is called on every keystroke in the compose box. Fixes: #21155.
This commit is contained in:
@@ -789,25 +789,38 @@ test_ui("test warn_if_topic_resolved", ({override, mock_template}) => {
|
||||
|
||||
// The error message area where it is shown
|
||||
const error_area = $("#compose_resolved_topic");
|
||||
compose_validate.clear_topic_resolved_warning();
|
||||
// Hack to make this empty for zjquery; this is conceptually done
|
||||
// in the previous line.
|
||||
error_area.html("");
|
||||
assert.ok(!error_area.visible());
|
||||
|
||||
compose_state.set_message_type("stream");
|
||||
compose_state.stream_name("Do not exist");
|
||||
compose_state.topic(resolved_topic.resolve_name("hello"));
|
||||
compose_state.message_content("content");
|
||||
|
||||
// Do not show a warning if stream name does not exist
|
||||
compose_validate.warn_if_topic_resolved();
|
||||
compose_validate.warn_if_topic_resolved(true);
|
||||
assert.ok(!error_area.visible());
|
||||
|
||||
compose_state.stream_name("random");
|
||||
|
||||
// Show the warning now as stream also exists
|
||||
compose_validate.warn_if_topic_resolved();
|
||||
compose_validate.warn_if_topic_resolved(true);
|
||||
assert.ok(error_area.visible());
|
||||
|
||||
// Call it again with false; this should be a noop.
|
||||
compose_validate.warn_if_topic_resolved(false);
|
||||
assert.ok(error_area.visible());
|
||||
|
||||
compose_state.topic("hello");
|
||||
|
||||
// The warning will be cleared now
|
||||
compose_validate.warn_if_topic_resolved();
|
||||
compose_validate.warn_if_topic_resolved(true);
|
||||
assert.ok(!error_area.visible());
|
||||
|
||||
// Calling with false won't do anything.
|
||||
compose_validate.warn_if_topic_resolved(false);
|
||||
assert.ok(!error_area.visible());
|
||||
});
|
||||
|
||||
@@ -403,6 +403,7 @@ export function initialize() {
|
||||
});
|
||||
|
||||
$("#compose-textarea").on("input propertychange", () => {
|
||||
compose_validate.warn_if_topic_resolved(false);
|
||||
compose_validate.check_overflow_text();
|
||||
});
|
||||
|
||||
@@ -467,14 +468,14 @@ export function initialize() {
|
||||
|
||||
message_edit.with_first_message_id(stream_id, topic_name, (message_id) => {
|
||||
message_edit.toggle_resolve_topic(message_id, topic_name);
|
||||
compose_validate.clear_topic_resolved_warning();
|
||||
compose_validate.clear_topic_resolved_warning(true);
|
||||
});
|
||||
});
|
||||
|
||||
$("#compose_resolved_topic").on("click", ".compose_resolved_topic_close", (event) => {
|
||||
event.preventDefault();
|
||||
|
||||
compose_validate.clear_topic_resolved_warning();
|
||||
compose_validate.clear_topic_resolved_warning(true);
|
||||
});
|
||||
|
||||
$("#compose_invite_users").on("click", ".compose_invite_link", (event) => {
|
||||
|
||||
@@ -291,7 +291,7 @@ export function start(msg_type, opts) {
|
||||
show_box(msg_type, opts);
|
||||
|
||||
// Show a warning if topic is resolved
|
||||
compose_validate.warn_if_topic_resolved();
|
||||
compose_validate.warn_if_topic_resolved(true);
|
||||
|
||||
// Reset the `max-height` property of `compose-textarea` so that the
|
||||
// compose-box do not cover the last messages of the current stream
|
||||
@@ -464,7 +464,7 @@ export function on_topic_narrow() {
|
||||
// See #3300 for context--a couple users specifically asked for
|
||||
// this convenience.
|
||||
compose_state.topic(narrow_state.topic());
|
||||
compose_validate.warn_if_topic_resolved();
|
||||
compose_validate.warn_if_topic_resolved(true);
|
||||
compose_fade.set_focused_recipient("stream");
|
||||
compose_fade.update_message_list();
|
||||
$("#compose-textarea").trigger("focus").trigger("select");
|
||||
|
||||
@@ -172,17 +172,31 @@ export function clear_topic_resolved_warning() {
|
||||
$("#compose-send-status").hide();
|
||||
}
|
||||
|
||||
export function warn_if_topic_resolved() {
|
||||
const stream_name = compose_state.stream_name();
|
||||
export function warn_if_topic_resolved(topic_changed) {
|
||||
// This function is called with topic_changed=false on every
|
||||
// keypress when typing a message, so it should not do anything
|
||||
// expensive in that case.
|
||||
//
|
||||
// Pass topic_changed=true if this function was called in response
|
||||
// to a topic being edited.
|
||||
const topic_name = compose_state.topic();
|
||||
|
||||
if (!topic_changed && !resolved_topic.is_resolved(topic_name)) {
|
||||
// The resolved topic warning will only ever appear when
|
||||
// composing to a resolve topic, so we return early without
|
||||
// inspecting additional fields in this case.
|
||||
return;
|
||||
}
|
||||
|
||||
const stream_name = compose_state.stream_name();
|
||||
const message_content = compose_state.message_content();
|
||||
const sub = stream_data.get_sub(stream_name);
|
||||
const $resolved_notice_area = $("#compose_resolved_topic");
|
||||
|
||||
if (sub && resolved_topic.is_resolved(topic_name)) {
|
||||
const error_area = $("#compose_resolved_topic");
|
||||
|
||||
if (error_area.html()) {
|
||||
clear_topic_resolved_warning(); // This warning already exists
|
||||
if (sub && message_content !== "" && resolved_topic.is_resolved(topic_name)) {
|
||||
if ($resolved_notice_area.html()) {
|
||||
// Error is already displayed; no action required.
|
||||
return;
|
||||
}
|
||||
|
||||
const context = {
|
||||
@@ -192,12 +206,15 @@ export function warn_if_topic_resolved() {
|
||||
};
|
||||
|
||||
const new_row = render_compose_resolved_topic(context);
|
||||
error_area.append(new_row);
|
||||
$resolved_notice_area.append(new_row);
|
||||
|
||||
error_area.show();
|
||||
$resolved_notice_area.show();
|
||||
} else {
|
||||
// Only clear the notice if already displayed.
|
||||
if ($resolved_notice_area.html()) {
|
||||
clear_topic_resolved_warning();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function show_all_everyone_warnings(stream_id) {
|
||||
@@ -638,6 +655,9 @@ function validate_private_message() {
|
||||
}
|
||||
|
||||
export function check_overflow_text() {
|
||||
// This function is called when typing every character in the
|
||||
// compose box, so it's important that it not doing anything
|
||||
// expensive.
|
||||
const text = compose_state.message_content();
|
||||
const max_length = page_params.max_message_length;
|
||||
const indicator = $("#compose_limit_indicator");
|
||||
|
||||
@@ -253,7 +253,7 @@ export function update_messages(events) {
|
||||
) {
|
||||
changed_compose = true;
|
||||
compose_state.topic(new_topic);
|
||||
compose_validate.warn_if_topic_resolved();
|
||||
compose_validate.warn_if_topic_resolved(true);
|
||||
compose_fade.set_focused_recipient("stream");
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user