Add `Not subscribe` tab in channels settings. #30021

sujalshah-bit · 2024-05-08T20:47:24Z

PR summary:

Add a Not Subscribed tab to the channels settings that will contain all the channels the user is not subscribed to.
Display the text No Channels to show. View all channels. when no channels is available to show.
Changes the redirect links for Browse channel and Browse 1 more channels to the Not subscribed tab.
Hide the Not subscribed tab if the user is a guest user.
Add a tooltip for for the disabled tabs for guests. ( Originally posted by @alya in stream_settings: Add not-subscribed tab to stream settings. #26049 (comment) )

Continuing the works of: #26049

not-subscribed.mp4

tooltip

Empty 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.

zulipbot · 2024-05-08T20:47:31Z

Hello @zulip/server-streams members, this pull request was labeled with the "area: stream settings" label, so you may want to check it out!

alya · 2024-05-08T22:47:11Z

Thanks! The tooltip should refer to "channels", not "streams" now.

sujalshah-bit · 2024-05-09T14:22:08Z

@alya made the changes :)

timabbott · 2024-05-10T19:01:25Z

@sahil839 can you review this one?

sahil839

Posted few comments.

Also, the "Fixes .." line in commit message should be in the last commit which completes all the points mentioned in the issue. In the previous commit, you can write "Fixes part of ..".

sahil839 · 2024-05-13T07:11:27Z

web/src/stream_edit.js

+        }
+        case "not-subscribed": {
+            browser_history.update("#channels/notsubscribed");
+


I think these extra lines before break can be removed.

sahil839 · 2024-05-13T07:16:34Z

web/src/stream_settings_ui.js

@@ -744,6 +757,11 @@ export function change_state(section, left_side_tab, right_side_tab) {
        return;
    }

+    if (section === "notsubscribed") {
+        toggler.goto("not-subscribed");


Why does this not call empty_right_panel?

sahil839 · 2024-05-13T07:18:49Z

web/src/stream_ui_updates.js


 export function is_subscribed_stream_tab_active() {
    // Returns true if "Subscribed" tab in stream settings is open
    // otherwise false.
    return show_subscribed;
 }

+export function is_not_subscribed_stream_tab_active() {
+    // Returns true if "Subscribed" tab in stream settings is open


This comment is incorrect.

sahil839 · 2024-05-13T07:22:20Z

web/src/stream_settings_ui.js

+        case "all-streams": {
+            stream_ui_updates.set_show_subscribed(false);
+            stream_ui_updates.set_show_not_subscribed(false);
+


These empty lines can also be removed.

web/src/hash_util.ts

sahil839 · 2024-05-13T07:30:23Z

web/src/stream_settings_ui.js

@@ -508,6 +513,7 @@ export function get_left_panel_params() {
    const params = {
        input,
        show_subscribed: stream_ui_updates.show_subscribed,
+        show_not_subscribed: stream_ui_updates.show_not_subscribed,


Maybe we can just store the selected name in a single variable instead of using two boolean variables, but I am not sure.

Let us keep it as it is for now and wait for further feedback.

sahil839 · 2024-05-13T07:53:28Z

web/src/stream_ui_updates.js

+            (is_subscribed_stream_tab_active() && sub.subscribed) ||
+            (is_not_subscribed_stream_tab_active() && !sub.subscribed)
+        ) {
+            $row.removeClass("notdisplayed");
        } else if (sub.invite_only || current_user.is_guest) {


This becomes confusing as we show the non-subscribed stream in the tab even when the stream is private, but then the stream is hidden ultimately as the client will receive the "remove" event. So, the behavior is fine but we can probably improve the code to avoid any future bugs.

Not sure what would be the best way to handle this, but just mentioning it here.

Currently we have same behavior if private stream is unsubscribed it is shown in the all streams tab.

I think it is shown for admins only and not non-admins.

Just to be clear, the behavior in the PR is correct, I am just not sure whether we should remove the stream here only or we should wait for "remove" event.

should I ask it on CZO ?

I think we can wait for a review by Tim first.

sujalshah-bit · 2024-05-17T10:53:22Z

@sahil839 made the changes according to your feedbacks kindly review :)

sahil839

Posted one comment. Also, please update the commit message to include "Fixes .." line.

sahil839 · 2024-05-20T03:44:51Z

web/templates/subscribe_to_more_streams.hbs

        <i class="fa fa-plus-circle" aria-hidden="true"></i>
-        {{~t "Browse {can_subscribe_stream_count} more channels" ~}}
+        {{~t "Browse {can_subscribe_stream_count} mores channels" ~}}


I think this is a typo.

sahil839 · 2024-05-20T06:46:36Z

web/src/stream_ui_updates.js

+            (is_subscribed_stream_tab_active() && sub.subscribed) ||
+            (is_not_subscribed_stream_tab_active() && !sub.subscribed)
+        ) {
+            $row.removeClass("notdisplayed");
        } else if (sub.invite_only || current_user.is_guest) {


I think it is shown for admins only and not non-admins.

Just to be clear, the behavior in the PR is correct, I am just not sure whether we should remove the stream here only or we should wait for "remove" event.

This commit changes the variable and function name from "subscribed_only" to "show_subscribed" and "set_subscribed_only" to "set_show_subscribed" so that in the next commit, when the "not subscribed" tab is added, we can use a similar variable named "show_unsubscribed" and function named "set_show_unsubscribed".

This commit adds a new tab to the stream settings overlay called "Not subscribed" which lists all the streams the user is not subscribed to. This tab is disabled if the user is a guest. Introduced a new variable called "show_not_subscribed" to replicate a similar model to how the subscribed tab is working. Currently, there are only two tabs: "subscribed" and "all streams" so we can use an if-else condition. Refactored the 'update_stream_row_in_settings_tab' function inside stream_ui_updates.js to include the case of a 'Not-subscribed' tab, so that the tab is immediately updated in any add/remove subscription event. Fixed and added the node tests for these changes.

In the "not-subscribed" tab, when there are no channels to show, we display a text message saying "No channels to show. View all channels" along with a link that redirects the user to the "All channels" tab (All channels(#channels/all)). Updated the update_empty_left_panel_message function in stream_settings_ui.js to modify and determine which banner to display when no channels are available to show, modified that function to also display the banner of the "not-subscribed" tab using a new classname called 'not_subscribed_streams_tab_empty_text'.

This commit changes the redirect links for "Browse channels" and "Browse 1 more stream" to the Not subscribed(#streams/notsubscribed) tab.

Render tooltips for both the tabs if the user is guest user. Fixes zulip#21869.

sujalshah-bit · 2024-05-23T04:19:04Z

@sahil839 made the changes. kindly review :)

sahil839 · 2024-05-23T06:39:49Z

Looks good to me. @timabbott ready for your review. There are a couple of things - here and here, that I am not sure and need feeback.

zulipbot added size: XL area: stream settings feedback wanted new feature A proposed new feature for the product labels May 8, 2024

sujalshah-bit force-pushed the not-subscribe-tab branch from 41fb128 to d2764b1 Compare May 8, 2024 21:22

sujalshah-bit changed the title ~~Not subscribe tab~~ Add Not subscribe tab in channels settings May 8, 2024

sujalshah-bit changed the title ~~Add Not subscribe tab in channels settings~~ Add Not subscribe tab in channels settings May 8, 2024

sujalshah-bit changed the title ~~Add Not subscribe tab in channels settings~~ Add Not subscribe tab in **channels settings**. May 8, 2024

sujalshah-bit changed the title ~~Add Not subscribe tab in **channels settings**.~~ Add Not subscribe tab in channels settings. May 8, 2024

sujalshah-bit force-pushed the not-subscribe-tab branch from d2764b1 to 50b9f81 Compare May 9, 2024 13:55

sahil839 reviewed May 13, 2024

View reviewed changes

sujalshah-bit force-pushed the not-subscribe-tab branch from 50b9f81 to 04ae6ed Compare May 17, 2024 10:14

sahil839 reviewed May 20, 2024

View reviewed changes

zulipbot added the has conflicts label May 20, 2024

sujalshah-bit added 6 commits May 23, 2024 08:05

stream_settings: Change the if-else with switch statement.

9fe68e9

left_sidebar: Change redirect link for Browse channels.

c673218

This commit changes the redirect links for "Browse channels" and "Browse 1 more stream" to the Not subscribed(#streams/notsubscribed) tab.

tooltip: Render tooltips for Not subscribed and All channels tabs.

1679d67

Render tooltips for both the tabs if the user is guest user. Fixes zulip#21869.

sujalshah-bit force-pushed the not-subscribe-tab branch from 04ae6ed to 1679d67 Compare May 23, 2024 03:18

zulipbot removed the has conflicts label May 23, 2024

sujalshah-bit requested a review from sahil839 May 23, 2024 04:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `Not subscribe` tab in channels settings. #30021

Add `Not subscribe` tab in channels settings. #30021

sujalshah-bit commented May 8, 2024 •

edited

zulipbot commented May 8, 2024

alya commented May 8, 2024

sujalshah-bit commented May 9, 2024

timabbott commented May 10, 2024

sahil839 left a comment

sahil839 May 13, 2024

sahil839 May 13, 2024

sahil839 May 13, 2024

sahil839 May 13, 2024

sahil839 May 13, 2024

sahil839 May 13, 2024

sujalshah-bit May 17, 2024

sahil839 May 20, 2024

sujalshah-bit May 23, 2024

sahil839 May 23, 2024

sujalshah-bit commented May 17, 2024

sahil839 left a comment

sahil839 May 20, 2024

sahil839 May 20, 2024

sujalshah-bit commented May 23, 2024

sahil839 commented May 23, 2024 •

edited

Add Not subscribe tab in channels settings. #30021

Are you sure you want to change the base?

Add Not subscribe tab in channels settings. #30021

Conversation

sujalshah-bit commented May 8, 2024 • edited

PR summary:

zulipbot commented May 8, 2024

alya commented May 8, 2024

sujalshah-bit commented May 9, 2024

timabbott commented May 10, 2024

sahil839 left a comment

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sujalshah-bit commented May 17, 2024

sahil839 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sujalshah-bit commented May 23, 2024

sahil839 commented May 23, 2024 • edited

Add `Not subscribe` tab in channels settings. #30021

Add `Not subscribe` tab in channels settings. #30021

sujalshah-bit commented May 8, 2024 •

edited

sahil839 commented May 23, 2024 •

edited