onboarding: Add a one-time intro tooltip for the "go to conversation" button. #32773

saubhagya-patel · 2024-12-19T12:39:58Z

This PR adds a one-time intro tooltip for the "go to conversation" button. It includes the following changes:

Add a one-time go-to conversation tooltip as an onboarding step.
Implement tests to verify the tooltip.

The tooltip is displayed highlighting the "go to conversation" button without being hovered.
Conditions for showing the tooltip as mentioned in the issue #32166:

The "Go to conversation" button is active.
You just confirmed a topic recipient change to a topic that we believe contains messages and is not the current view.
You have no compose banners showing.
You haven't clicked the button (which is the mechanism that would dismiss it).
You haven't been shown the same tooltip before.

Screenshots and screen captures:

Go to conversation button tooltip.

Tooltip not being shown when banner is displayed.

Tooltip in an interleaved view.

Tooltip when a user confirms a topic recipient change to a topic not in current view.

Tooltip when a user confirms a topic recipient change to an existing topic.

Tooltip when a user confirms a direct message recipient change to a different recipient.

Dismissing the tooltip.

Tooltip not being shown again.

Tooltip in Recent conversations view.

Self-review checklist

Self-reviewed the changes for clarity and maintainability
(variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

Explains differences from previous plans (e.g., issue description).
Highlights technical choices and bugs encountered.
Calls out remaining decisions and concerns.
Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

Each commit is a coherent idea.
Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

Visual appearance of the changes.
Responsiveness and internationalization.
Strings and tooltips.
End-to-end functionality of buttons, interactions and flows.
Corner cases, error conditions, and easily imagined bugs.

saubhagya-patel · 2024-12-19T13:37:11Z

Following the questions mentioned in the issue:

Do we want to show this tooltip twice (once for DMs and once for topics), as specced? Or just once?
- Using this implementation the tooltip will only be shown once similar to the compose banners being shown in interleaved views.
Should this tooltip be shown to existing users?
- Please correct me if I am wrong, I think the tooltip will show once to the existing users as well with the updated set of changes until the Zulip server has disabled the tutorial.

saubhagya-patel · 2024-12-19T13:37:42Z

Tagging @userAdityaa for a review following this comment.

alya · 2024-12-19T17:29:13Z

Playing with it, I like showing it just once (total), since it's the same for DMs and non-DMs.

alya · 2024-12-19T17:29:54Z

We can decide a bit later whether to mark existing users as having already completed this onboarding step.

alya · 2024-12-19T17:30:20Z

@prakhar1144 Are you up for reviewing this one?

alya · 2024-12-19T17:31:08Z

This PR, rather than the issue, is the right place to post this comment:

Following the continuing unfinished work documentation, please note that PR #32773 takes a different approach than the one in this PR. Specifically, this PR adds the tooltip when updating the class of the "go-to conversation" button, whereas PR #32773 introduces the tooltip when the #compose-textarea is focused.

pratik-pc · 2024-12-19T21:47:13Z

web/src/compose_setup.js

+        compose_tooltips.hide_go_to_conversation_button_intro_tooltip();
+        if (
+            onboarding_steps.ONE_TIME_NOTICES_TO_DISPLAY.has(
+                "intro_go_to_conversation_button_tooltip",
+            )
+        ) {


I think it's better to call hide_go_to_conversation_button_intro_tooltip only if ONE_TIME_NOTICES_TO_DISPLAY contains it, instead of calling it on every narrow.

That sounds like a reasonable approach to me and I have fixed it. Thank you

pratik-pc · 2024-12-19T22:02:35Z

web/src/compose_actions.ts

@@ -455,6 +459,7 @@ export let cancel = (): void => {
    compose_state.set_message_type(undefined);
    compose_pm_pill.clear();
    $(document).trigger("compose_canceled.zulip");
+    compose_tooltips.hide_go_to_conversation_button_intro_tooltip();


Here, we are calling it every time as well.

This is now fixed. Thank you.

saubhagya-patel · 2024-12-20T11:07:04Z

Following the review received I have updated this PR. The following changes are made:

As mentioned in this comment, the compose_tooltips.hide_go_to_conversation_button_intro_tooltip() function is called only if onboarding_steps.ONE_TIME_NOTICES_TO_DISPLAY has the go-to conversation button tooltip notice (intro_go_to_conversation_button_tooltip).
No visual changes are made.

This PR is ready for review. Thank you @pratik-pc for the feedback.

pratik-pc · 2024-12-20T22:56:08Z

web/src/compose_tooltips.ts

+    const faded_messages_exist = $(".focused-message-list .recipient_row").hasClass("message-fade");
+    if (!faded_messages_exist) {
+        return false;
+    }


Not quite sure if this approach is correct. This would fail if there were no message_list in the current view, such as when composing from the recent conversation view and changing the recipient.

Hmm, that sounds right, I will fix it in the subsequent push. Thank you.

pratik-pc · 2024-12-20T23:04:55Z

web/src/compose_tooltips.ts

+
+    // This helps in updating tooltip when switching between topics.
+    hide_go_to_conversation_button_intro_tooltip();
+
+    if (
+        !onboarding_steps.ONE_TIME_NOTICES_TO_DISPLAY.has("intro_go_to_conversation_button_tooltip")
+    ) {
+        return false;
+    }


I think we should call hide_go_to_conversation_button_intro_tooltip after the onboarding_step check here as well, unless there's a specific case you are trying to avoid.

I must have missed it, I apologize for the inconvenience caused. It is fixed in the subsequent push.

saubhagya-patel · 2024-12-21T10:06:06Z

Following the review received, I have updated this PR. The following changes are made:

As mentioned in this comment, in the compose_tooltips.can_show_go_to_conversation_button_intro_tooltip() function, the hide_go_to_conversation_button_intro_tooltip() function is now called after checking if onboarding_steps.ONE_TIME_NOTICES_TO_DISPLAY has the go-to conversation button tooltip (intro_go_to_conversation_button_tooltip).
As mentioned in this comment, to check whether a user is composing to the same topic/DM as the current view, I have updated the approach from using faded_messages_exist to confirming whether there is the same topic/DM in the narrow_state and compose box (similar to the function compose_recipient.update_narrow_to_recipient_visibility()).
Visual Changes: Now, the tooltip is also shown in different views. Please check the attached GIF for the Recent conversations view.

GIF of tooltip in Recent conversations view.

@pratik-pc, I have made the required changes. Please let me know if any further adjustments are needed. Thank you.

pratik-pc · 2024-12-23T20:27:14Z

web/src/compose_tooltips.ts

+    if (
+        onboarding_steps.ONE_TIME_NOTICES_TO_DISPLAY.has("intro_go_to_conversation_button_tooltip")
+    ) {
+        hide_go_to_conversation_button_intro_tooltip();
+    }
+
+    if (
+        !onboarding_steps.ONE_TIME_NOTICES_TO_DISPLAY.has("intro_go_to_conversation_button_tooltip")
+    ) {
+        return false;
+    }


This can just simply be

if ( !onboarding_steps.ONE_TIME_NOTICES_TO_DISPLAY.has("intro_go_to_conversation_button_tooltip") ) { return false; } hide_go_to_conversation_button_intro_tooltip();

pratik-pc · 2024-12-23T20:27:22Z

web/src/compose_tooltips.ts

+    const message_type = compose_state.get_message_type();
+    if (message_type === "stream") {
+        const stream_exists = Boolean(compose_state.stream_id());
+
+        if (
+            stream_exists &&
+            compose_recipient.composing_to_current_topic_narrow() &&
+            compose_state.has_full_recipient()
+        ) {
+            return false;
+        }
+    } else if (message_type === "private") {
+        const recipients = compose_state.private_message_recipient();
+        if (
+            recipients &&
+            compose_recipient.composing_to_current_private_message_narrow() &&
+            compose_state.has_full_recipient()
+        ) {
+            return false;
+        }
+    }


It's better to refactor this into its own function from update_narrow_to_recipient_visibility in a separate commit and then use it here and update_narrow_to_recipient_visibility.

Sure, that sounds better, I have implemented this in the following push.

pratik-pc · 2024-12-23T20:27:26Z

web/src/compose_tooltips.ts

+    const $go_to_conversation_button = $(".conversation-arrow");
+    if (!$go_to_conversation_button.hasClass("narrow_to_compose_recipients")) {
+        return false;
+    }
+
+    const is_any_compose_banner_visible = compose_banner.is_any_banner_visible();
+    if (is_any_compose_banner_visible) {
+        return false;
+    }


This can also be

if ( !$(".conversation-arrow").hasClass("narrow_to_compose_recipients") || compose_banner.is_any_banner_visible() ) { return false; }

This commit extracts the logic of updating the "narrow_to_compose_recipients" class applied on the go-to conversation button from the function update_narrow_to_recipient_visibility() into a separate function should_display_narrow_to_recipient(). Fixes part of zulip#32166.

This commit includes the following changes: - Add a one-time go-to conversation tooltip as an onboarding step. - Implement tests to verify the tooltip. Fixes zulip#32166.

saubhagya-patel · 2024-12-24T07:28:48Z

Following the review received, I have updated this PR. The following changes are made:

The function can_show_go_to_conversation_button_intro_tooltip() in the compose_tooltips.ts file is refactored following the comment[1] and comment[2].
Following this comment, a separate commit is added in which the logic of function update_narrow_to_recipient_visibility() is refactored into a separate function named should_display_narrow_to_recipient() in the compose_recipient.ts file.
No visual changes are made.

@pratik-pc, thank you for the ongoing feedback. I hope the updated changes look good to you. Please let me know if any further adjustments are needed. Thank you.

pratik-pc · 2025-01-08T21:44:40Z

You'd probably want to call hide_go_to_conversation_button_intro_tooltip or show_go_to_conversation_button_intro_tooltip when narrow_to_compose_recipients class is disabled and stream dropdown is changed to avoid getting in a bad ui state. Maybe test it out again for any ui issues.

zulipbot added size: XL area: popovers Popovers, including general message actions. priority: high labels Dec 19, 2024

saubhagya-patel marked this pull request as ready for review December 19, 2024 13:16

saubhagya-patel mentioned this pull request Dec 19, 2024

onboarding: Add one-time go-to-conversation tooltip. #32169

Open

12 tasks

alya assigned prakhar1144 Dec 19, 2024

alya added the maintainer review PR is ready for review by Zulip maintainers. label Dec 19, 2024

pratik-pc reviewed Dec 19, 2024

View reviewed changes

saubhagya-patel force-pushed the tooltip-for-go-to-conversation branch from 2906d2a to ef9af1d Compare December 20, 2024 10:38

pratik-pc reviewed Dec 20, 2024

View reviewed changes

saubhagya-patel force-pushed the tooltip-for-go-to-conversation branch from ef9af1d to 59149ad Compare December 21, 2024 09:36

pratik-pc reviewed Dec 23, 2024

View reviewed changes

saubhagya-patel added 2 commits December 24, 2024 12:23

onboarding: Add a one-time go-to conversation tooltip.

792cb48

This commit includes the following changes: - Add a one-time go-to conversation tooltip as an onboarding step. - Implement tests to verify the tooltip. Fixes zulip#32166.

saubhagya-patel force-pushed the tooltip-for-go-to-conversation branch from 59149ad to 792cb48 Compare December 24, 2024 07:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

onboarding: Add a one-time intro tooltip for the "go to conversation" button. #32773

onboarding: Add a one-time intro tooltip for the "go to conversation" button. #32773

saubhagya-patel commented Dec 19, 2024 •

edited

Loading

saubhagya-patel commented Dec 19, 2024

saubhagya-patel commented Dec 19, 2024

alya commented Dec 19, 2024 •

edited

Loading

alya commented Dec 19, 2024

alya commented Dec 19, 2024

alya commented Dec 19, 2024

pratik-pc Dec 19, 2024

saubhagya-patel Dec 20, 2024

pratik-pc Dec 19, 2024

saubhagya-patel Dec 20, 2024

saubhagya-patel commented Dec 20, 2024

pratik-pc Dec 20, 2024

saubhagya-patel Dec 21, 2024

pratik-pc Dec 20, 2024

saubhagya-patel Dec 21, 2024

saubhagya-patel commented Dec 21, 2024

pratik-pc Dec 23, 2024

pratik-pc Dec 23, 2024

saubhagya-patel Dec 24, 2024

pratik-pc Dec 23, 2024

saubhagya-patel commented Dec 24, 2024

pratik-pc commented Jan 8, 2025

onboarding: Add a one-time intro tooltip for the "go to conversation" button. #32773

Are you sure you want to change the base?

onboarding: Add a one-time intro tooltip for the "go to conversation" button. #32773

Conversation

saubhagya-patel commented Dec 19, 2024 • edited Loading

saubhagya-patel commented Dec 19, 2024

saubhagya-patel commented Dec 19, 2024

alya commented Dec 19, 2024 • edited Loading

alya commented Dec 19, 2024

alya commented Dec 19, 2024

alya commented Dec 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saubhagya-patel commented Dec 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saubhagya-patel commented Dec 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saubhagya-patel commented Dec 24, 2024

pratik-pc commented Jan 8, 2025

saubhagya-patel commented Dec 19, 2024 •

edited

Loading

alya commented Dec 19, 2024 •

edited

Loading