mirror of
https://github.com/zulip/zulip.git
synced 2025-11-18 04:43:58 +00:00
compose_validate: Fix buggy separated handling of empty compose box.
When I adjusted 4fbf91c135 to no longer
do a full re-validation on every character typed, that unmasked bugs
in how the compose_setup handler code and compose_validate split
responsibility for validating 0-length messages.
The user-facing result was the send button not enabling properly when
starting to type.
Fix this by moving the 0-length message validation into
compose_validate, where we already handle issues of too-long messages,
sharing the same basic logic for determining when to do a full
validate() call.
A benefit of this reworking is now all of the logic for handling the
`invalid` class is in one place in validate, rather than split with
the compose_setup module.
This commit is contained in:
@@ -77,9 +77,7 @@ export function initialize() {
|
|||||||
}
|
}
|
||||||
compose_validate.warn_if_topic_resolved(false);
|
compose_validate.warn_if_topic_resolved(false);
|
||||||
const compose_text_length = compose_validate.check_overflow_text($("#send_message_form"));
|
const compose_text_length = compose_validate.check_overflow_text($("#send_message_form"));
|
||||||
if (compose_text_length !== 0 && $("textarea#compose-textarea").hasClass("invalid")) {
|
|
||||||
$("textarea#compose-textarea").toggleClass("invalid", false);
|
|
||||||
}
|
|
||||||
// Change compose close button tooltip as per condition.
|
// Change compose close button tooltip as per condition.
|
||||||
// We save compose text in draft only if its length is > 2.
|
// We save compose text in draft only if its length is > 2.
|
||||||
if (compose_text_length > 2) {
|
if (compose_text_length > 2) {
|
||||||
|
|||||||
@@ -893,6 +893,7 @@ export function check_overflow_text($container: JQuery): number {
|
|||||||
const $indicator = $container.find(".message-limit-indicator");
|
const $indicator = $container.find(".message-limit-indicator");
|
||||||
const is_edit_container = $textarea.closest(".message_row").length > 0;
|
const is_edit_container = $textarea.closest(".message_row").length > 0;
|
||||||
|
|
||||||
|
const old_no_message_content = no_message_content;
|
||||||
const old_message_too_long = message_too_long;
|
const old_message_too_long = message_too_long;
|
||||||
|
|
||||||
if (text.length > max_length) {
|
if (text.length > max_length) {
|
||||||
@@ -936,13 +937,21 @@ export function check_overflow_text($container: JQuery): number {
|
|||||||
set_message_too_long_for_compose(false);
|
set_message_too_long_for_compose(false);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (!is_edit_container && message_too_long !== old_message_too_long) {
|
|
||||||
// If this keystroke changed the truth status for whether
|
if (!is_edit_container) {
|
||||||
// we're too long, then we need to refresh the send button
|
// Update the state for whether the message is empty.
|
||||||
// status. This is expensive, but naturally debounced by the
|
set_no_message_content(text.length === 0);
|
||||||
// fact that crossing the MAX_MESSAGE_LENGTH threshold is
|
if (
|
||||||
// rare.
|
message_too_long !== old_message_too_long ||
|
||||||
validate_and_update_send_button_status();
|
old_no_message_content !== no_message_content
|
||||||
|
) {
|
||||||
|
// If this keystroke changed the truth status for whether
|
||||||
|
// the message is empty or too long, then we need to
|
||||||
|
// refresh the send button status from scratch. This is
|
||||||
|
// expensive, but naturally debounced by the fact this
|
||||||
|
// changes rarely.
|
||||||
|
validate_and_update_send_button_status();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
return text.length;
|
return text.length;
|
||||||
}
|
}
|
||||||
@@ -1022,10 +1031,15 @@ export let validate = (scheduling_message: boolean, show_banner = true): boolean
|
|||||||
set_no_message_content(no_message_content);
|
set_no_message_content(no_message_content);
|
||||||
if (no_message_content) {
|
if (no_message_content) {
|
||||||
if (show_banner) {
|
if (show_banner) {
|
||||||
|
// If you tried actually sending a message with empty
|
||||||
|
// compose, flash the textarea as invalid.
|
||||||
$("textarea#compose-textarea").toggleClass("invalid", true);
|
$("textarea#compose-textarea").toggleClass("invalid", true);
|
||||||
$("textarea#compose-textarea").trigger("focus");
|
$("textarea#compose-textarea").trigger("focus");
|
||||||
}
|
}
|
||||||
return false;
|
return false;
|
||||||
|
} else if ($("textarea#compose-textarea").hasClass("invalid")) {
|
||||||
|
// Hide the invalid indicator now that it's non-empty.
|
||||||
|
$("textarea#compose-textarea").toggleClass("invalid", false);
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($("#zephyr-mirror-error").is(":visible")) {
|
if ($("#zephyr-mirror-error").is(":visible")) {
|
||||||
|
|||||||
@@ -314,6 +314,9 @@ test("respond_to_message", ({override, override_rewire, mock_template}) => {
|
|||||||
override_private_message_recipient({override});
|
override_private_message_recipient({override});
|
||||||
mock_template("inline_decorated_stream_name.hbs", false, noop);
|
mock_template("inline_decorated_stream_name.hbs", false, noop);
|
||||||
|
|
||||||
|
override(realm, "realm_direct_message_permission_group", nobody.id);
|
||||||
|
override(realm, "realm_direct_message_initiator_group", everyone.id);
|
||||||
|
|
||||||
// Test direct message
|
// Test direct message
|
||||||
const person = {
|
const person = {
|
||||||
user_id: 22,
|
user_id: 22,
|
||||||
|
|||||||
@@ -279,6 +279,8 @@ test_ui("validate", ({mock_template, override}) => {
|
|||||||
assert.ok(compose_validate.validate());
|
assert.ok(compose_validate.validate());
|
||||||
|
|
||||||
let zephyr_error_rendered = false;
|
let zephyr_error_rendered = false;
|
||||||
|
// For this first block, we should fail due to empty compose.
|
||||||
|
let expected_invalid_state = true;
|
||||||
mock_template("compose_banner/compose_banner.hbs", false, (data) => {
|
mock_template("compose_banner/compose_banner.hbs", false, (data) => {
|
||||||
if (data.classname === compose_banner.CLASSNAMES.zephyr_not_running) {
|
if (data.classname === compose_banner.CLASSNAMES.zephyr_not_running) {
|
||||||
assert.equal(
|
assert.equal(
|
||||||
@@ -296,14 +298,16 @@ test_ui("validate", ({mock_template, override}) => {
|
|||||||
compose_state.private_message_recipient("welcome-bot@example.com");
|
compose_state.private_message_recipient("welcome-bot@example.com");
|
||||||
$("textarea#compose-textarea").toggleClass = (classname, value) => {
|
$("textarea#compose-textarea").toggleClass = (classname, value) => {
|
||||||
assert.equal(classname, "invalid");
|
assert.equal(classname, "invalid");
|
||||||
assert.equal(value, true);
|
assert.equal(value, expected_invalid_state);
|
||||||
};
|
};
|
||||||
assert.ok(!compose_validate.validate());
|
assert.ok(!compose_validate.validate());
|
||||||
assert.ok(!$("#compose-send-button .loader").visible());
|
assert.ok(!$("#compose-send-button .loader").visible());
|
||||||
assert.equal($("#compose-send-button").prop("disabled"), false);
|
assert.equal($("#compose-send-button").prop("disabled"), false);
|
||||||
compose_validate.validate();
|
compose_validate.validate();
|
||||||
|
|
||||||
|
// Now add content to compose, and expect to see the banner.
|
||||||
add_content_to_compose_box();
|
add_content_to_compose_box();
|
||||||
|
expected_invalid_state = false;
|
||||||
let zephyr_checked = false;
|
let zephyr_checked = false;
|
||||||
$("#zephyr-mirror-error").is = (arg) => {
|
$("#zephyr-mirror-error").is = (arg) => {
|
||||||
assert.equal(arg, ":visible");
|
assert.equal(arg, ":visible");
|
||||||
|
|||||||
Reference in New Issue
Block a user