From 35380b095fdf1817ca3f2cfd46451184d526275c Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Fri, 5 Apr 2024 17:02:42 +0530 Subject: [PATCH] compose: Show banner to explain non interleaved view messages fading. In a non interleaved view when composing a message to another conversation we fade messages which the user is not replying to, to reduce the chance they send a message to a recipient they didn't intend to. Also, it reduces the visual/cognitive processing required to figure out where their message is going to go. But, it's not necessarily clear to users that what the fading means, so this commit adds a one-time compose banner to explain what's going on the first time this comes up. Fixes part of #29076. --- web/src/compose_actions.ts | 1 + web/src/compose_banner.ts | 5 ++++ web/src/compose_notifications.ts | 30 +++++++++++++++++++ web/src/compose_setup.js | 15 ++++++++++ web/tests/compose.test.js | 5 ++++ zerver/lib/onboarding_steps.py | 3 ++ ...55_alter_onboardingstep_onboarding_step.py | 17 +++++++++++ zerver/models/onboarding_steps.py | 2 +- zerver/tests/test_onboarding_steps.py | 3 +- 9 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 zerver/migrations/0555_alter_onboardingstep_onboarding_step.py diff --git a/web/src/compose_actions.ts b/web/src/compose_actions.ts index 27c3e8f7fc..61ab873550 100644 --- a/web/src/compose_actions.ts +++ b/web/src/compose_actions.ts @@ -407,6 +407,7 @@ export function cancel(): void { hide_box(); clear_box(); compose_banner.clear_message_sent_banners(); + compose_banner.clear_non_interleaved_view_messages_fading_banner(); call_hooks(compose_cancel_hooks); compose_state.set_message_type(undefined); compose_pm_pill.clear(); diff --git a/web/src/compose_banner.ts b/web/src/compose_banner.ts index 08a07e69a0..86f908a314 100644 --- a/web/src/compose_banner.ts +++ b/web/src/compose_banner.ts @@ -34,6 +34,7 @@ const MESSAGE_SENT_CLASSNAMES = { export const CLASSNAMES = { ...MESSAGE_SENT_CLASSNAMES, + non_interleaved_view_messages_fading: "non_interleaved_view_messages_fading", // unmute topic notifications are styled like warnings but have distinct behaviour unmute_topic_notification: "unmute_topic_notification warning-style", // warnings @@ -147,6 +148,10 @@ export function clear_search_view_banner(): void { $(`#compose_banners .${CSS.escape(CLASSNAMES.search_view)}`).remove(); } +export function clear_non_interleaved_view_messages_fading_banner(): void { + $(`#compose_banners .${CSS.escape(CLASSNAMES.non_interleaved_view_messages_fading)}`).remove(); +} + export function clear_all(): void { scroll_util.get_content_element($(`#compose_banners`)).empty(); } diff --git a/web/src/compose_notifications.ts b/web/src/compose_notifications.ts index e888295601..dba82e84f6 100644 --- a/web/src/compose_notifications.ts +++ b/web/src/compose_notifications.ts @@ -2,6 +2,7 @@ import $ from "jquery"; import assert from "minimalistic-assert"; import render_automatic_new_visibility_policy_banner from "../templates/compose_banner/automatic_new_visibility_policy_banner.hbs"; +import render_compose_banner from "../templates/compose_banner/compose_banner.hbs"; import render_jump_to_sent_message_conversation_banner from "../templates/compose_banner/jump_to_sent_message_conversation_banner.hbs"; import render_message_sent_banner from "../templates/compose_banner/message_sent_banner.hbs"; import render_unmute_topic_banner from "../templates/compose_banner/unmute_topic_banner.hbs"; @@ -304,6 +305,35 @@ export function notify_messages_outside_current_search(messages: Message[]): voi } } +export function maybe_show_one_time_non_interleaved_view_messages_fading_banner(): void { + // Remove message fading banner if exists. Helps in live-updating banner. + compose_banner.clear_non_interleaved_view_messages_fading_banner(); + + if (!onboarding_steps.ONE_TIME_NOTICES_TO_DISPLAY.has("non_interleaved_view_messages_fading")) { + return; + } + + // Wait to display the banner the first time until there's actually fading. + const faded_messages_exist = $(".focused-message-list .recipient_row").hasClass("message-fade"); + if (!faded_messages_exist) { + return; + } + + const context = { + banner_type: compose_banner.INFO, + classname: compose_banner.CLASSNAMES.non_interleaved_view_messages_fading, + banner_text: $t({ + defaultMessage: + "Messages in your view are faded to remind you that you are viewing a different conversation from the one you are composing to.", + }), + button_text: $t({defaultMessage: "Got it"}), + hide_close_button: true, + }; + const new_row_html = render_compose_banner(context); + + compose_banner.append_compose_banner_to_banner_list($(new_row_html), $("#compose_banners")); +} + export function reify_message_id(opts: {old_id: number; new_id: number}): void { const old_id = opts.old_id; const new_id = opts.new_id; diff --git a/web/src/compose_setup.js b/web/src/compose_setup.js index d6c8db3d7a..f8aba57256 100644 --- a/web/src/compose_setup.js +++ b/web/src/compose_setup.js @@ -8,6 +8,7 @@ import * as compose_actions from "./compose_actions"; import * as compose_banner from "./compose_banner"; import * as compose_call from "./compose_call"; import * as compose_call_ui from "./compose_call_ui"; +import * as compose_notifications from "./compose_notifications"; import * as compose_recipient from "./compose_recipient"; import * as compose_send_menu_popover from "./compose_send_menu_popover"; import * as compose_state from "./compose_state"; @@ -317,6 +318,19 @@ export function initialize() { }, ); + const non_interleaved_view_messages_fading_banner_selector = `.${CSS.escape(compose_banner.CLASSNAMES.non_interleaved_view_messages_fading)}`; + $("body").on( + "click", + `${non_interleaved_view_messages_fading_banner_selector} .main-view-banner-action-button`, + (event) => { + event.preventDefault(); + $(event.target) + .parents(`${non_interleaved_view_messages_fading_banner_selector}`) + .remove(); + onboarding_steps.post_onboarding_step_as_read("non_interleaved_view_messages_fading"); + }, + ); + for (const classname of Object.values(compose_banner.CLASSNAMES)) { const classname_selector = `.${CSS.escape(classname)}`; $("body").on("click", `${classname_selector} .main-view-banner-close-button`, (event) => { @@ -478,6 +492,7 @@ export function initialize() { $("textarea#compose-textarea").on("focus", () => { compose_recipient.update_placeholder_text(); + compose_notifications.maybe_show_one_time_non_interleaved_view_messages_fading_banner(); }); $("#compose_recipient_box").on("click", "#recipient_box_clear_topic_button", () => { diff --git a/web/tests/compose.test.js b/web/tests/compose.test.js index 959ceac611..00fd64ca9a 100644 --- a/web/tests/compose.test.js +++ b/web/tests/compose.test.js @@ -796,6 +796,11 @@ test_ui("on_events", ({override, override_rewire}) => { }; override_rewire(compose_recipient, "update_placeholder_text", noop); + override( + compose_notifications, + "maybe_show_one_time_non_interleaved_view_messages_fading_banner", + noop, + ); handler(event); diff --git a/zerver/lib/onboarding_steps.py b/zerver/lib/onboarding_steps.py index 149ab26dc2..fc2accf929 100644 --- a/zerver/lib/onboarding_steps.py +++ b/zerver/lib/onboarding_steps.py @@ -35,6 +35,9 @@ ONE_TIME_NOTICES: list[OneTimeNotice] = [ OneTimeNotice( name="jump_to_conversation_banner", ), + OneTimeNotice( + name="non_interleaved_view_messages_fading", + ), ] # We may introduce onboarding step of types other than 'one time notice' diff --git a/zerver/migrations/0555_alter_onboardingstep_onboarding_step.py b/zerver/migrations/0555_alter_onboardingstep_onboarding_step.py new file mode 100644 index 0000000000..4bb7bd451b --- /dev/null +++ b/zerver/migrations/0555_alter_onboardingstep_onboarding_step.py @@ -0,0 +1,17 @@ +# Generated by Django 5.0.5 on 2024-04-23 07:16 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("zerver", "0554_imageattachment"), + ] + + operations = [ + migrations.AlterField( + model_name="onboardingstep", + name="onboarding_step", + field=models.CharField(max_length=40), + ), + ] diff --git a/zerver/models/onboarding_steps.py b/zerver/models/onboarding_steps.py index 565fb13572..166ea557e9 100644 --- a/zerver/models/onboarding_steps.py +++ b/zerver/models/onboarding_steps.py @@ -7,7 +7,7 @@ from zerver.models.users import UserProfile class OnboardingStep(models.Model): user = models.ForeignKey(UserProfile, on_delete=CASCADE) - onboarding_step = models.CharField(max_length=30) + onboarding_step = models.CharField(max_length=40) timestamp = models.DateTimeField(default=timezone_now) class Meta: diff --git a/zerver/tests/test_onboarding_steps.py b/zerver/tests/test_onboarding_steps.py index f7bf07213a..01a8222891 100644 --- a/zerver/tests/test_onboarding_steps.py +++ b/zerver/tests/test_onboarding_steps.py @@ -25,10 +25,11 @@ class TestGetNextOnboardingSteps(ZulipTestCase): do_mark_onboarding_step_as_read(self.user, "intro_inbox_view_modal") onboarding_steps = get_next_onboarding_steps(self.user) - self.assert_length(onboarding_steps, 3) + self.assert_length(onboarding_steps, 4) self.assertEqual(onboarding_steps[0]["name"], "intro_recent_view_modal") self.assertEqual(onboarding_steps[1]["name"], "first_stream_created_banner") self.assertEqual(onboarding_steps[2]["name"], "jump_to_conversation_banner") + self.assertEqual(onboarding_steps[3]["name"], "non_interleaved_view_messages_fading") with self.settings(TUTORIAL_ENABLED=False): onboarding_steps = get_next_onboarding_steps(self.user)