compose: Show resolve topic banner only once per narrow.

Previously, when sending a message to a resolved topic, if you disissed
the 'You are sending a message to a resolved topic' banner, it would
reappear as soon as the user enters another character.

Fix this by showing the banner at most once per narrow. It does not
reappear if the user closes the banner and continues typing.  It will
only be shown again if the user closes compose, changes stream/topic,
sends a message or otherwise clears the compose box state.

We also remove the existing check for whether this banner is already
visible; this is essentially a more precise version of the same logic.

Fixes #24245.
This commit is contained in:
Pranav2612000 2023-02-02 10:13:24 +05:30 committed by Tim Abbott
parent 0e55b2aed9
commit c58f38dae3
5 changed files with 39 additions and 2 deletions

View File

@ -497,6 +497,7 @@ test_ui("initialize", ({override}) => {
}); });
test_ui("update_fade", ({override}) => { test_ui("update_fade", ({override}) => {
mock_banners();
initialize_handlers({override}); initialize_handlers({override});
const selector = const selector =

View File

@ -778,11 +778,20 @@ test_ui("test warn_if_topic_resolved", ({override, mock_template}) => {
compose_validate.warn_if_topic_resolved(true); compose_validate.warn_if_topic_resolved(true);
assert.ok(error_shown); assert.ok(error_shown);
// We reset the state to be able to show the banner again
compose_state.set_recipient_viewed_topic_resolved_banner(false);
// Call it again with false; this should do the same thing. // Call it again with false; this should do the same thing.
error_shown = false; error_shown = false;
compose_validate.warn_if_topic_resolved(false); compose_validate.warn_if_topic_resolved(false);
assert.ok(error_shown); assert.ok(error_shown);
// Call the func again. This should not show the error because
// we have already shown the error once for this topic.
error_shown = false;
compose_validate.warn_if_topic_resolved(false);
assert.ok(!error_shown);
compose_state.topic("hello"); compose_state.topic("hello");
// The warning will be cleared now // The warning will be cleared now

View File

@ -96,6 +96,11 @@ function update_fade() {
} }
const msg_type = compose_state.get_message_type(); const msg_type = compose_state.get_message_type();
// It's possible that the new topic is not a resolved topic
// so we clear the older warning.
compose_validate.clear_topic_resolved_warning();
compose_validate.warn_if_topic_resolved(); compose_validate.warn_if_topic_resolved();
compose_fade.set_focused_recipient(msg_type); compose_fade.set_focused_recipient(msg_type);
compose_fade.update_all(); compose_fade.update_all();

View File

@ -5,6 +5,14 @@ import * as compose_pm_pill from "./compose_pm_pill";
let message_type = false; // 'stream', 'private', or false-y let message_type = false; // 'stream', 'private', or false-y
let recipient_edited_manually = false; let recipient_edited_manually = false;
// We use this variable to keep track of whether user has viewed the topic resolved
// banner for the current compose session, for a narrow. This prevents the banner
// from popping up for every keystroke while composing.
// The variable is reset on sending a message, closing the compose box and changing
// the narrow and the user should still be able to see the banner once after
// performing these actions
let recipient_viewed_topic_resolved_banner = false;
export function set_recipient_edited_manually(flag) { export function set_recipient_edited_manually(flag) {
recipient_edited_manually = flag; recipient_edited_manually = flag;
} }
@ -21,6 +29,14 @@ export function get_message_type() {
return message_type; return message_type;
} }
export function set_recipient_viewed_topic_resolved_banner(flag) {
recipient_viewed_topic_resolved_banner = flag;
}
export function has_recipient_viewed_topic_resolved_banner() {
return recipient_viewed_topic_resolved_banner;
}
export function recipient_has_topics() { export function recipient_has_topics() {
return message_type !== "stream"; return message_type !== "stream";
} }

View File

@ -167,7 +167,12 @@ export function warn_if_mentioning_unsubscribed_user(mentioned) {
} }
} }
// Called when clearing the compose box and similar contexts to clear
// the warning for composing to a resolved topic, if present. Also clears
// the state for whether this warning has already been shown in the
// current narrow.
export function clear_topic_resolved_warning() { export function clear_topic_resolved_warning() {
compose_state.set_recipient_viewed_topic_resolved_banner(false);
$(`#compose_banners .${compose_banner.CLASSNAMES.topic_resolved}`).remove(); $(`#compose_banners .${compose_banner.CLASSNAMES.topic_resolved}`).remove();
} }
@ -196,8 +201,8 @@ export function warn_if_topic_resolved(topic_changed) {
const $compose_banner_area = $("#compose_banners"); const $compose_banner_area = $("#compose_banners");
if (sub && message_content !== "" && resolved_topic.is_resolved(topic_name)) { if (sub && message_content !== "" && resolved_topic.is_resolved(topic_name)) {
if ($(`#compose_banners .${compose_banner.CLASSNAMES.topic_resolved}`).length) { if (compose_state.has_recipient_viewed_topic_resolved_banner()) {
// Error is already displayed; no action required. // We display the resolved topic banner at most once per narrow.
return; return;
} }
@ -219,6 +224,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_area.append(new_row); $compose_banner_area.append(new_row);
compose_state.set_recipient_viewed_topic_resolved_banner(true);
} else { } else {
clear_topic_resolved_warning(); clear_topic_resolved_warning();
} }