stream_settings: Restructure stream creation panel. #29433

sujalshah-bit · 2024-03-23T22:21:02Z

User group section

panel 1	panel 2

Channel creation section

panel 1	panel 2

Advance configuration:

img	img

Notes

The issue description doesn't include a mention of a "Configure Settings" button. While implementing these changes, I realized that users might need to revisit the configuration settings to modify certain details, such as the stream name. Therefore, I added a button for navigating back to the configuration settings when needed.
Also the color of the button should be different I guess please provide feedback on both of this.

Fixes part of #29403.

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.

timabbott · 2024-04-12T17:19:25Z

Is this ready for review?

sujalshah-bit · 2024-04-13T01:26:20Z

@timabbott as i have mentioned above last part that is Change how adding users works this is part is still remaining for that part i still not don't know how to implement this

timabbott · 2024-04-23T18:19:11Z

Let's just move the "Change how adding users work" part to a separate issue; this already has enough going on as it is. @sujalshah-bit can you do a careful pass to make sure you've got the right strings? "Advance configuration" in your screenshot looks like a typo, "Advanced" is the word you want.

sujalshah-bit · 2024-04-23T19:33:40Z

I will make the update soon.

timabbott · 2024-05-09T22:29:15Z

@sahil839 can you review this one?

sahil839

Posted a comment.

A note on commit message, the first commit should have "Fixes part of .." to make sure we do not close the issue if we merge a part of this PR initially. The commit should have "Fixes .." only in the commit which completes the said issue.

web/templates/stream_settings/stream_creation_form.hbs

web/templates/stream_settings/stream_types.hbs

web/templates/stream_settings/stream_creation_form.hbs

web/src/settings_config.ts

web/templates/stream_settings/stream_creation_form.hbs

web/src/stream_create.js

web/src/user_group_create.js

sahil839 · 2024-05-10T06:44:07Z

Also, the current changes also results in the "Advanced configuration" expandable view in the stream edit form as same template is used for both the places. The issue does not mention anything about changing it. @alya do we want that change in stream edit UI as well?

alya · 2024-05-14T23:43:21Z

The section label should be "Advanced configurations", as noted in the issue.

alya · 2024-05-14T23:45:41Z

Please be sure not to introduce a regression by using "stream" instead of "channel".

alya · 2024-05-14T23:47:38Z

Currently, I can click "Continue to add subscribers" without filling out required info on the first page (channel name). We should make sure to catch that before the user gets to the next page.

Similarly, it shouldn't be possible to continue to the next page if you use a channel name that's already taken, or there's some other error.

alya · 2024-05-14T23:55:23Z

The issue description doesn't include a mention of a "Configure Settings" button. While implementing these changes, I realized that users might need to revisit the configuration settings to modify certain details, such as the stream name. Therefore, I added a button for navigating back to the configuration settings when needed.

Good idea! We might want to make it more clear that this is a "back" button, but the current state on this PR is probably OK for now.

alya · 2024-05-14T23:56:13Z

Also, the current changes also results in the "Advanced configuration" expandable view in the stream edit form as same template is used for both the places. The issue does not mention anything about changing it. @alya do we want that change in stream edit UI as well?

Good question! I wasn't thinking about that, but it feels reasonable to me.

alya · 2024-05-14T23:57:33Z

Clicking anywhere on the "Advanced configuration" section name should expand/collapse it, with an appropriate pointer cursor.

alya · 2024-05-14T23:59:04Z

I'm seeing the triangle not rotating down correctly when the section is expanded in the "create channel" UI:

alya · 2024-05-15T00:01:54Z

That's all I've got for now. I didn't read @sahil839 's feedback carefully, so there may be some duplication.

sujalshah-bit · 2024-05-17T03:37:13Z

Currently, I can click "Continue to add subscribers" without filling out required info on the first page (channel name). We should make sure to catch that before the user gets to the next page.
Similarly, it shouldn't be possible to continue to the next page if you use a channel name that's already taken, or there's some other error.

I am not able to reproduce that bug. In my dev env it is working as expected

I'm seeing the triangle not rotating down correctly when the section is expanded in the "create channel" UI:

This bug only occurs when the user clicks on a stream and then clicks the create stream button. I'm trying to understand what's causing it, but I haven't figured it out much. @sahil839, can you help?

bandicam.2024-05-17.08-59-18-129.mp4

left one comment above for user_group discussion @sahil839 kindly review it.

Other than that I think I have covers all feedbacks kindly review @sahil839 @alya.

sahil839

Posted few comments.

web/src/stream_create.js

web/src/stream_settings_components.js

web/src/user_group_create.js

sahil839 · 2024-05-17T09:54:06Z

I am not able to reproduce that bug. In my dev env it is working as expected

I can reproduce this. I just opened the stream creation form, did nothing and simply clicked on "Continue to add subscribers" button.

This bug only occurs when the user clicks on a stream and then clicks the create stream button. I'm trying to understand what's causing it, but I haven't figured it out much. @sahil839, can you help?

This is because there are two elements with same ID "toggle-advanced-configurations-icon" once the user clicks on a stream - one for stream creation form and one for the stream settings edit form. The correct solution here would be to use a class for that element and then update the JS code to make sure the class is changed for the correct element.

sahil839

Posted some comments.

sahil839 · 2024-05-23T07:12:09Z

web/src/stream_create.js

+    e.stopPropagation();
+
+    const stream_name = $("#create_stream_name").val().trim();
+    const name_ok = stream_name_error.validate_for_submit(stream_name);


A better variable name could be is_stream_name_valid.

sahil839 · 2024-05-23T07:14:52Z

web/src/stream_create.js

+    if (name_ok) {
+        stream_settings_components.show_subs_pane.create_stream("subscribers_container");
+    } else {
+        stream_settings_components.show_subs_pane.create_stream("configure_channel_settings");


Can we just do nothing in case the name is not valid?

sahil839 · 2024-05-23T07:15:49Z

web/src/stream_settings_components.js

    },
 };

+export function update_footer_buttons(container_name = "configure_channel_settings") {


I think there is no need for a default value as the caller will pass container name always.

sahil839 · 2024-05-23T07:17:23Z

web/src/user_group_components.ts

    },
 };
+
+export function update_footer_buttons(container_name = "configure_user_group_settings"): void {


Here also, we can remove the default value.

sahil839 · 2024-05-23T07:17:46Z

web/src/user_group_create.js

+    e.stopPropagation();
+
+    const group_name = $("#create_user_group_name").val().trim();
+    const name_ok = user_group_name_error.validate_for_submit(group_name);


This variable can be named similar to how we would have it for streams.

sahil839 · 2024-05-23T07:28:33Z

Since we now collapse settings like "Who can post to stream", "Message retention", etc. in the stream edit view as well, do we want to show the "Save changes" button on "Advanced configurations" heading? @alya FYI.

Removed the "Stream permissions" heading to simplify the panel. Moved the "Announce new stream in {stream}" option just above "Default stream for new users". Implement a collapsible "Advanced configurations" section, collapsed by default, to accommodate less commonly changed settings. This allows for future expansion of stream permission configurations. Fixes part of zulip#29403.

Update the descriptions of Web-public, Public, Private, shared history and Private, protected history.

Changed the title of the first panel to "Create channel: configure settings" and title of second panel to "Create channel: add subscribers". Implement logic in stream_create.js to show and hide both the panels. Add two buttons to facilitate switching between the tabs or panels. Added new descriptions to 'stream type descriptions'.

Extract `show_user_group_settings_pane` with related functions and variable.

Break the current user group creation panel into two panel "Create user group: configure settings" and "Create user group: add subscribers".

zulipbot added the size: XL label Mar 23, 2024

sujalshah-bit force-pushed the issue-29403 branch 2 times, most recently from 93995bd to 0df3503 Compare March 25, 2024 10:53

zulipbot added the has conflicts label Apr 24, 2024

sujalshah-bit force-pushed the issue-29403 branch from 0df3503 to efe56e1 Compare April 27, 2024 11:06

sujalshah-bit force-pushed the issue-29403 branch from efe56e1 to b479ae4 Compare May 9, 2024 15:29

sujalshah-bit changed the title ~~[WIP]stream_settings: Restructure stream creation panel.~~ stream_settings: Restructure stream creation panel. May 9, 2024

zulipbot removed the has conflicts label May 9, 2024

sahil839 reviewed May 10, 2024

View reviewed changes

sujalshah-bit force-pushed the issue-29403 branch 4 times, most recently from e1b0c48 to 77a974d Compare May 17, 2024 03:09

sahil839 reviewed May 17, 2024

View reviewed changes

web/src/stream_create.js Outdated Show resolved Hide resolved

web/src/stream_settings_components.js Outdated Show resolved Hide resolved

web/src/user_group_create.js Outdated Show resolved Hide resolved

sujalshah-bit force-pushed the issue-29403 branch 2 times, most recently from 14b8108 to 1f1cd4a Compare May 23, 2024 02:10

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

sahil839 reviewed May 23, 2024

View reviewed changes

sujalshah-bit added 5 commits May 27, 2024 11:36

stream_settings: Update stream type descriptions.

746dd36

Update the descriptions of Web-public, Public, Private, shared history and Private, protected history.

user_group: Extract show_user_group_settings_pane.

7818f91

Extract `show_user_group_settings_pane` with related functions and variable.

user_group: Move "Choose subscribers" into a separate second panel.

cf29396

Break the current user group creation panel into two panel "Create user group: configure settings" and "Create user group: add subscribers".

sujalshah-bit force-pushed the issue-29403 branch from 1f1cd4a to cf29396 Compare May 27, 2024 06:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stream_settings: Restructure stream creation panel. #29433

stream_settings: Restructure stream creation panel. #29433

sujalshah-bit commented Mar 23, 2024 •

edited

timabbott commented Apr 12, 2024

sujalshah-bit commented Apr 13, 2024

timabbott commented Apr 23, 2024

sujalshah-bit commented Apr 23, 2024

timabbott commented May 9, 2024

sahil839 left a comment

sahil839 commented May 10, 2024

alya commented May 14, 2024

alya commented May 14, 2024

alya commented May 14, 2024 •

edited

alya commented May 14, 2024

alya commented May 14, 2024

alya commented May 14, 2024

alya commented May 14, 2024

alya commented May 15, 2024

sujalshah-bit commented May 17, 2024

sahil839 left a comment

sahil839 commented May 17, 2024

sahil839 left a comment

sahil839 May 23, 2024

sahil839 May 23, 2024

sahil839 May 23, 2024

sahil839 May 23, 2024

sahil839 May 23, 2024

sahil839 commented May 23, 2024 •

edited

stream_settings: Restructure stream creation panel. #29433

Are you sure you want to change the base?

stream_settings: Restructure stream creation panel. #29433

Conversation

sujalshah-bit commented Mar 23, 2024 • edited

User group section

Channel creation section

Advance configuration:

Notes

timabbott commented Apr 12, 2024

sujalshah-bit commented Apr 13, 2024

timabbott commented Apr 23, 2024

sujalshah-bit commented Apr 23, 2024

timabbott commented May 9, 2024

sahil839 left a comment

Choose a reason for hiding this comment

sahil839 commented May 10, 2024

alya commented May 14, 2024

alya commented May 14, 2024

alya commented May 14, 2024 • edited

alya commented May 14, 2024

alya commented May 14, 2024

alya commented May 14, 2024

alya commented May 14, 2024

alya commented May 15, 2024

sujalshah-bit commented May 17, 2024

sahil839 left a comment

Choose a reason for hiding this comment

sahil839 commented May 17, 2024

sahil839 left a comment

Choose a reason for hiding this comment

sahil839 May 23, 2024

Choose a reason for hiding this comment

sahil839 May 23, 2024

Choose a reason for hiding this comment

sahil839 May 23, 2024

Choose a reason for hiding this comment

sahil839 May 23, 2024

Choose a reason for hiding this comment

sahil839 May 23, 2024

Choose a reason for hiding this comment

sahil839 commented May 23, 2024 • edited

sujalshah-bit commented Mar 23, 2024 •

edited

alya commented May 14, 2024 •

edited

sahil839 commented May 23, 2024 •

edited