From 5d1de4c037fabda6622353b422af6b145d3d40de Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Thu, 14 Nov 2024 14:53:22 +0530 Subject: [PATCH] stream-settings: Use new pills UI for can_remove_subscribers_group. --- web/src/settings_components.ts | 82 +++++++++++++++++-- web/src/settings_org.ts | 12 +-- web/src/stream_create.ts | 25 ++++-- web/src/stream_data.ts | 10 +-- web/src/stream_edit.ts | 39 ++------- web/src/stream_events.js | 2 +- web/src/stream_settings_components.ts | 31 ------- web/src/stream_settings_ui.js | 5 +- web/src/stream_types.ts | 4 +- web/src/stream_ui_updates.ts | 19 +++++ web/styles/subscriptions.css | 10 ++- .../group_setting_value_pill_input.hbs | 2 +- .../stream_settings/stream_creation_form.hbs | 1 - .../stream_settings/stream_types.hbs | 6 +- web/tests/stream_data.test.cjs | 2 +- web/tests/stream_events.test.cjs | 2 +- 16 files changed, 150 insertions(+), 102 deletions(-) diff --git a/web/src/settings_components.ts b/web/src/settings_components.ts index 2f2a02adc4..b76070a0b4 100644 --- a/web/src/settings_components.ts +++ b/web/src/settings_components.ts @@ -29,6 +29,7 @@ import * as settings_data from "./settings_data.ts"; import type {CustomProfileField, GroupSettingValue} from "./state_data.ts"; import {current_user, realm, realm_schema} from "./state_data.ts"; import * as stream_data from "./stream_data.ts"; +import * as stream_settings_containers from "./stream_settings_containers.ts"; import type {StreamSubscription} from "./sub_store.ts"; import {stream_subscription_schema} from "./sub_store.ts"; import type {GroupSettingPillContainer} from "./typeahead_helper.ts"; @@ -477,7 +478,6 @@ const dropdown_widget_map = new Map([ ["realm_signup_announcements_stream_id", null], ["realm_zulip_update_announcements_stream_id", null], ["realm_default_code_block_language", null], - ["can_remove_subscribers_group", null], ["realm_can_access_all_users_group", null], ["realm_can_create_web_public_channel_group", null], ]); @@ -856,9 +856,12 @@ export function check_stream_settings_property_changed( const current_val = get_stream_settings_property_value(property_name, sub); let proposed_val; switch (property_name) { - case "can_remove_subscribers_group": - proposed_val = get_dropdown_list_widget_setting_value($elem); + case "can_remove_subscribers_group": { + const pill_widget = get_group_setting_widget(property_name); + assert(pill_widget !== null); + proposed_val = get_group_setting_widget_value(pill_widget); break; + } case "message_retention_days": assert(elem instanceof HTMLSelectElement); proposed_val = get_message_retention_setting_value($(elem), false); @@ -873,7 +876,7 @@ export function check_stream_settings_property_changed( blueslip.error("Element refers to unknown property", {property_name}); } } - return current_val !== proposed_val; + return !_.isEqual(current_val, proposed_val); } export function get_group_setting_widget_value( @@ -1099,6 +1102,19 @@ export function populate_data_for_stream_settings_request( continue; } + const stream_group_settings = new Set(["can_remove_subscribers_group"]); + if (stream_group_settings.has(property_name)) { + const old_value = get_stream_settings_property_value( + stream_settings_property_schema.parse(property_name), + sub, + ); + data[property_name] = JSON.stringify({ + new: input_value, + old: old_value, + }); + continue; + } + assert(typeof input_value !== "object"); data[property_name] = input_value; } @@ -1368,9 +1384,12 @@ function should_disable_save_button_for_group_settings(settings: string[]): bool setting_name_without_prefix, "realm", ); + } else if (setting_name === "can_remove_subscribers_group") { + group_setting_config = group_permission_settings.get_group_permission_setting_config( + setting_name, + "stream", + ); } else { - // We do not have any stream settings using the new UI currently, - // so we know that this block will be called for group setting only. group_setting_config = group_permission_settings.get_group_permission_setting_config( setting_name, "group", @@ -1476,6 +1495,7 @@ export const group_setting_widget_map = new Map { + save_discard_stream_settings_widget_status_handler($subsection, sub); + }); + pill_widget.onPillRemove(() => { + save_discard_stream_settings_widget_status_handler($subsection, sub); + }); + } else { + const default_group_name = group_permission_settings.get_group_permission_setting_config( + setting_name, + "stream", + )!.default_group_name; + const default_group_id = user_groups.get_user_group_from_name(default_group_name)!.id; + set_group_setting_widget_value(pill_widget, default_group_id); + } + + return pill_widget; +} + export function set_time_input_formatted_text( $time_select_elem: JQuery, formatted_text: string, diff --git a/web/src/settings_org.ts b/web/src/settings_org.ts index 157e65a9b8..517b414453 100644 --- a/web/src/settings_org.ts +++ b/web/src/settings_org.ts @@ -608,13 +608,15 @@ export function discard_stream_property_element_changes( sub, ); switch (property_name) { - case "can_remove_subscribers_group": - assert(typeof property_value === "number"); - settings_components.set_dropdown_list_widget_setting_value( - property_name, - property_value, + case "can_remove_subscribers_group": { + const pill_widget = settings_components.get_group_setting_widget(property_name); + assert(pill_widget !== null); + settings_components.set_group_setting_widget_value( + pill_widget, + group_setting_value_schema.parse(property_value), ); break; + } case "stream_privacy": { assert(typeof property_value === "string"); $elem.find(`input[value='${CSS.escape(property_value)}']`).prop("checked", true); diff --git a/web/src/stream_create.ts b/web/src/stream_create.ts index e951014e88..afaf61df0d 100644 --- a/web/src/stream_create.ts +++ b/web/src/stream_create.ts @@ -13,6 +13,7 @@ import * as keydown_util from "./keydown_util.ts"; import * as loading from "./loading.ts"; import * as onboarding_steps from "./onboarding_steps.ts"; import * as people from "./people.ts"; +import * as settings_components from "./settings_components.ts"; import * as settings_data from "./settings_data.ts"; import {current_user, realm} from "./state_data.ts"; import * as stream_create_subscribers from "./stream_create_subscribers.ts"; @@ -20,6 +21,7 @@ import * as stream_data from "./stream_data.ts"; import * as stream_settings_components from "./stream_settings_components.ts"; import * as stream_settings_data from "./stream_settings_data.ts"; import * as stream_ui_updates from "./stream_ui_updates.ts"; +import type {GroupSettingPillContainer} from "./typeahead_helper.ts"; import type {HTMLSelectOneElement} from "./types.ts"; import * as ui_report from "./ui_report.ts"; import * as util from "./util.ts"; @@ -363,11 +365,10 @@ function create_stream(): void { const principals = JSON.stringify(user_ids); set_current_user_subscribed_to_created_stream(user_ids.includes(current_user.user_id)); - assert(stream_settings_components.new_stream_can_remove_subscribers_group_widget !== null); - const widget_value = - stream_settings_components.new_stream_can_remove_subscribers_group_widget.value(); - assert(typeof widget_value === "number"); - const can_remove_subscribers_group_id = widget_value; + assert(can_remove_subscribers_group_widget !== undefined); + const can_remove_subscribers_group_value = settings_components.get_group_setting_widget_value( + can_remove_subscribers_group_widget, + ); loading.make_indicator($("#stream_creating_indicator"), { text: $t({defaultMessage: "Creating channel..."}), @@ -383,7 +384,7 @@ function create_stream(): void { message_retention_days: JSON.stringify(message_retention_selection), announce: JSON.stringify(announce), principals, - can_remove_subscribers_group: can_remove_subscribers_group_id, + can_remove_subscribers_group: JSON.stringify(can_remove_subscribers_group_value), }; // Subscribe yourself and possible other people to a new stream. @@ -506,6 +507,15 @@ export function show_new_stream_modal(): void { clear_error_display(); } +let can_remove_subscribers_group_widget: GroupSettingPillContainer | undefined; + +function set_up_group_setting_widgets(): void { + can_remove_subscribers_group_widget = settings_components.create_stream_group_setting_widget({ + $pill_container: $("#id_new_can_remove_subscribers_group"), + setting_name: "can_remove_subscribers_group", + }); +} + export function set_up_handlers(): void { stream_announce_previous_value = settings_data.user_can_create_public_streams() || @@ -580,8 +590,7 @@ export function set_up_handlers(): void { } }); - assert(stream_settings_components.new_stream_can_remove_subscribers_group_widget !== null); - stream_settings_components.new_stream_can_remove_subscribers_group_widget.setup(); + set_up_group_setting_widgets(); } export function initialize(): void { diff --git a/web/src/stream_data.ts b/web/src/stream_data.ts index 4112177a59..0f1f002645 100644 --- a/web/src/stream_data.ts +++ b/web/src/stream_data.ts @@ -9,7 +9,7 @@ import type {User} from "./people.ts"; import * as people from "./people.ts"; import * as settings_config from "./settings_config.ts"; import * as settings_data from "./settings_data.ts"; -import type {StateData} from "./state_data.ts"; +import type {GroupSettingValue, StateData} from "./state_data.ts"; import {current_user, realm} from "./state_data.ts"; import type {StreamPostPolicy} from "./stream_types.ts"; import * as sub_store from "./sub_store.ts"; @@ -424,11 +424,11 @@ export function update_message_retention_setting( sub.message_retention_days = message_retention_days; } -export function update_can_remove_subscribers_group_id( +export function update_can_remove_subscribers_group( sub: StreamSubscription, - can_remove_subscribers_group_id: number, + can_remove_subscribers_group: GroupSettingValue, ): void { - sub.can_remove_subscribers_group = can_remove_subscribers_group_id; + sub.can_remove_subscribers_group = can_remove_subscribers_group; } export function receives_notifications( @@ -569,7 +569,7 @@ export function can_unsubscribe_others(sub: StreamSubscription): boolean { return true; } - return user_groups.is_user_in_group( + return user_groups.is_user_in_setting_group( sub.can_remove_subscribers_group, people.my_current_user_id(), ); diff --git a/web/src/stream_edit.ts b/web/src/stream_edit.ts index 2244c9cb74..3ceafdf555 100644 --- a/web/src/stream_edit.ts +++ b/web/src/stream_edit.ts @@ -17,7 +17,6 @@ import * as channel from "./channel.ts"; import * as confirm_dialog from "./confirm_dialog.ts"; import {show_copied_confirmation} from "./copied_tooltip.ts"; import * as dialog_widget from "./dialog_widget.ts"; -import * as dropdown_widget from "./dropdown_widget.ts"; import {$t, $t_html} from "./i18n.ts"; import * as keydown_util from "./keydown_util.ts"; import * as narrow_state from "./narrow_state.ts"; @@ -46,7 +45,6 @@ import * as stream_ui_updates from "./stream_ui_updates.ts"; import * as sub_store from "./sub_store.ts"; import type {StreamSubscription} from "./sub_store.ts"; import * as ui_report from "./ui_report.ts"; -import * as user_groups from "./user_groups.ts"; import {user_settings} from "./user_settings.ts"; import * as util from "./util.ts"; @@ -234,37 +232,12 @@ export function stream_settings(sub: StreamSubscription): StreamSetting[] { }); } -function setup_dropdown(sub: StreamSubscription, slim_sub: StreamSubscription): void { - const can_remove_subscribers_group_widget = new dropdown_widget.DropdownWidget({ - widget_name: "can_remove_subscribers_group", - get_options: () => - user_groups.get_realm_user_groups_for_dropdown_list_widget( - "can_remove_subscribers_group", - "stream", - ), - item_click_callback(event, dropdown) { - dropdown.hide(); - event.preventDefault(); - event.stopPropagation(); - can_remove_subscribers_group_widget.render(); - settings_components.save_discard_stream_settings_widget_status_handler( - $(".advanced-configurations-container"), - slim_sub, - ); - }, - $events_container: $("#subscription_overlay .subscription_settings"), - default_id: sub.can_remove_subscribers_group, - unique_id_type: dropdown_widget.DataTypes.NUMBER, - on_mount_callback(dropdown) { - $(dropdown.popper).css("min-width", "300px"); - $(dropdown.popper).find(".simplebar-content").css("width", "max-content"); - }, +function setup_group_setting_widgets(sub: StreamSubscription): void { + settings_components.create_stream_group_setting_widget({ + $pill_container: $("#id_can_remove_subscribers_group"), + setting_name: "can_remove_subscribers_group", + sub, }); - settings_components.set_dropdown_setting_widget( - "can_remove_subscribers_group", - can_remove_subscribers_group_widget, - ); - can_remove_subscribers_group_widget.setup(); } export function show_settings_for(node: HTMLElement): void { @@ -318,7 +291,7 @@ export function show_settings_for(node: HTMLElement): void { show_subscription_settings(sub); settings_org.set_message_retention_setting_dropdown(sub); stream_ui_updates.enable_or_disable_permission_settings_in_edit_panel(sub); - setup_dropdown(sub, slim_sub); + setup_group_setting_widgets(slim_sub); $("#channels_overlay_container").on( "click", diff --git a/web/src/stream_events.js b/web/src/stream_events.js index cff1be0a36..865fc64327 100644 --- a/web/src/stream_events.js +++ b/web/src/stream_events.js @@ -107,7 +107,7 @@ export function update_property(stream_id, property, value, other_values) { stream_settings_ui.update_message_retention_setting(sub, value); break; case "can_remove_subscribers_group": - stream_settings_ui.update_can_remove_subscribers_group_id(sub, value); + stream_settings_ui.update_can_remove_subscribers_group(sub, value); break; default: blueslip.warn("Unexpected subscription property type", { diff --git a/web/src/stream_settings_components.ts b/web/src/stream_settings_components.ts index 2e1c57b0e3..0ef4335c1b 100644 --- a/web/src/stream_settings_components.ts +++ b/web/src/stream_settings_components.ts @@ -1,5 +1,4 @@ import $ from "jquery"; -import assert from "minimalistic-assert"; import {z} from "zod"; import render_unsubscribe_private_stream_modal from "../templates/confirm_dialog/confirm_unsubscribe_private_stream.hbs"; @@ -8,8 +7,6 @@ import render_selected_stream_title from "../templates/stream_settings/selected_ import * as channel from "./channel.ts"; import * as confirm_dialog from "./confirm_dialog.ts"; -import * as dropdown_widget from "./dropdown_widget.ts"; -import type {DropdownWidget} from "./dropdown_widget.ts"; import * as hash_util from "./hash_util.ts"; import {$t, $t_html} from "./i18n.ts"; import * as loading from "./loading.ts"; @@ -21,7 +18,6 @@ import {current_user} from "./state_data.ts"; import * as stream_ui_updates from "./stream_ui_updates.ts"; import type {StreamSubscription} from "./sub_store.ts"; import * as ui_report from "./ui_report.ts"; -import * as user_groups from "./user_groups.ts"; export function set_right_panel_title(sub: StreamSubscription): void { let title_icon_color = "#333333"; @@ -108,33 +104,6 @@ export function get_active_data(): { }; } -export let new_stream_can_remove_subscribers_group_widget: DropdownWidget | null = null; - -export function dropdown_setup(): void { - new_stream_can_remove_subscribers_group_widget = new dropdown_widget.DropdownWidget({ - widget_name: "new_stream_can_remove_subscribers_group", - get_options: () => - user_groups.get_realm_user_groups_for_dropdown_list_widget( - "can_remove_subscribers_group", - "stream", - ), - item_click_callback(event, dropdown) { - dropdown.hide(); - event.preventDefault(); - event.stopPropagation(); - assert(new_stream_can_remove_subscribers_group_widget !== null); - new_stream_can_remove_subscribers_group_widget.render(); - }, - $events_container: $("#subscription_overlay"), - on_mount_callback(dropdown) { - $(dropdown.popper).css("min-width", "300px"); - $(dropdown.popper).find(".simplebar-content").css("width", "max-content"); - }, - default_id: user_groups.get_user_group_from_name("role:administrators")!.id, - unique_id_type: dropdown_widget.DataTypes.NUMBER, - }); -} - /* For the given stream_row, remove the tick and replace by a spinner. */ function display_subscribe_toggle_spinner($stream_row: JQuery): void { /* Prevent sending multiple requests by removing the button class. */ diff --git a/web/src/stream_settings_ui.js b/web/src/stream_settings_ui.js index 20a1803b6b..68e6df62d5 100644 --- a/web/src/stream_settings_ui.js +++ b/web/src/stream_settings_ui.js @@ -184,8 +184,8 @@ export function update_message_retention_setting(sub, new_value) { stream_ui_updates.update_setting_element(sub, "message_retention_days"); } -export function update_can_remove_subscribers_group_id(sub, new_value) { - stream_data.update_can_remove_subscribers_group_id(sub, new_value); +export function update_can_remove_subscribers_group(sub, new_value) { + stream_data.update_can_remove_subscribers_group(sub, new_value); stream_ui_updates.update_setting_element(sub, "can_remove_subscribers_group"); stream_edit_subscribers.rerender_subscribers_list(sub); } @@ -696,7 +696,6 @@ export function setup_page(callback) { render_left_panel_superset(); initialize_components(); - stream_settings_components.dropdown_setup(); redraw_left_panel(); stream_create.set_up_handlers(); diff --git a/web/src/stream_types.ts b/web/src/stream_types.ts index 8340a5b3cb..f66f321439 100644 --- a/web/src/stream_types.ts +++ b/web/src/stream_types.ts @@ -1,5 +1,7 @@ import {z} from "zod"; +import {group_setting_value_schema} from "./types.ts"; + export const enum StreamPostPolicy { EVERYONE = 1, ADMINS = 2, @@ -28,7 +30,7 @@ export const stream_schema = z.object({ RESTRICT_NEW_MEMBERS: StreamPostPolicy.RESTRICT_NEW_MEMBERS, MODERATORS: StreamPostPolicy.MODERATORS, }), - can_remove_subscribers_group: z.number(), + can_remove_subscribers_group: group_setting_value_schema, }); export const stream_specific_notification_settings_schema = z.object({ diff --git a/web/src/stream_ui_updates.ts b/web/src/stream_ui_updates.ts index b9caa410e2..fd426ac077 100644 --- a/web/src/stream_ui_updates.ts +++ b/web/src/stream_ui_updates.ts @@ -278,11 +278,30 @@ export function enable_or_disable_permission_settings_in_edit_panel( .find("input, select, button") .prop("disabled", !sub.can_change_stream_permissions); + const $permission_pill_container_elements = + $advanced_configurations_container.find(".pill-container"); + $permission_pill_container_elements + .find(".input") + .prop("contenteditable", sub.can_change_stream_permissions); + if (!sub.can_change_stream_permissions) { $general_settings_container.find(".default-stream").addClass("control-label-disabled"); + $permission_pill_container_elements + .closest(".input-group") + .addClass("group_setting_disabled"); + settings_components.disable_opening_typeahead_on_clicking_label( + $advanced_configurations_container, + ); return; } + $permission_pill_container_elements + .closest(".input-group") + .removeClass("group_setting_disabled"); + settings_components.enable_opening_typeahead_on_clicking_label( + $advanced_configurations_container, + ); + update_default_stream_and_stream_privacy_state($stream_settings); const disable_message_retention_setting = diff --git a/web/styles/subscriptions.css b/web/styles/subscriptions.css index c026774c84..dd437e3ba2 100644 --- a/web/styles/subscriptions.css +++ b/web/styles/subscriptions.css @@ -718,7 +718,8 @@ h4.user_group_setting_subsection_title { } .org-permissions-form, -.group-permissions { +.group-permissions, +.stream-permissions { .group_setting_disabled { cursor: not-allowed; /* This ensures that we do not see the not allowed cursor in the @@ -1143,7 +1144,8 @@ div.settings-radio-input-parent { } } -.group-permissions .pill-container { +.group-permissions .pill-container, +.stream-permissions .pill-container { /* 319px + 2 * (2px padding) + 2 * (1px border) = 325px, which is the total width of dropdown widget buttons */ min-width: 319px; @@ -1159,6 +1161,10 @@ div.settings-radio-input-parent { } } +.stream-permissions .pill-container { + margin-bottom: 10px; +} + .group-permissions .dropdown_widget_with_label_wrapper { display: inline-block; height: 30px; diff --git a/web/templates/settings/group_setting_value_pill_input.hbs b/web/templates/settings/group_setting_value_pill_input.hbs index b4b8371f89..d3eea14a1c 100644 --- a/web/templates/settings/group_setting_value_pill_input.hbs +++ b/web/templates/settings/group_setting_value_pill_input.hbs @@ -1,6 +1,6 @@
-
+
{{~! Squash whitespace so that placeholder is displayed when empty. ~}}
diff --git a/web/templates/stream_settings/stream_creation_form.hbs b/web/templates/stream_settings/stream_creation_form.hbs index 69b94838eb..eacc6dd2d1 100644 --- a/web/templates/stream_settings/stream_creation_form.hbs +++ b/web/templates/stream_settings/stream_creation_form.hbs @@ -28,7 +28,6 @@ {{> stream_types . stream_post_policy=stream_post_policy_values.everyone.code is_stream_edit=false - can_remove_subscribers_setting_widget_name="new_stream_can_remove_subscribers_group" prefix="id_new_" }}
diff --git a/web/templates/stream_settings/stream_types.hbs b/web/templates/stream_settings/stream_types.hbs index 222c95e79e..43459de37d 100644 --- a/web/templates/stream_settings/stream_types.hbs +++ b/web/templates/stream_settings/stream_types.hbs @@ -66,10 +66,10 @@
- {{> ../dropdown_widget_with_label - widget_name=can_remove_subscribers_setting_widget_name + {{> ../settings/group_setting_value_pill_input + setting_name="can_remove_subscribers_group" label=(t 'Who can unsubscribe others from this channel') - value_type="number"}} + prefix=prefix }} {{#if (or is_owner is_stream_edit)}}
diff --git a/web/tests/stream_data.test.cjs b/web/tests/stream_data.test.cjs index c528ead2d9..8bf5da7109 100644 --- a/web/tests/stream_data.test.cjs +++ b/web/tests/stream_data.test.cjs @@ -520,7 +520,7 @@ test("stream_settings", ({override}) => { }); stream_data.update_stream_post_policy(sub, 1); stream_data.update_message_retention_setting(sub, -1); - stream_data.update_can_remove_subscribers_group_id(sub, moderators_group.id); + stream_data.update_can_remove_subscribers_group(sub, moderators_group.id); assert.equal(sub.invite_only, false); assert.equal(sub.history_public_to_subscribers, false); assert.equal(sub.stream_post_policy, settings_config.stream_post_policy_values.everyone.code); diff --git a/web/tests/stream_events.test.cjs b/web/tests/stream_events.test.cjs index c6cb11062b..6992ee6aae 100644 --- a/web/tests/stream_events.test.cjs +++ b/web/tests/stream_events.test.cjs @@ -267,7 +267,7 @@ test("update_property", ({override}) => { // Test stream can_remove_subscribers_group change event { const stub = make_stub(); - override(stream_settings_ui, "update_can_remove_subscribers_group_id", stub.f); + override(stream_settings_ui, "update_can_remove_subscribers_group", stub.f); stream_events.update_property(stream_id, "can_remove_subscribers_group", 3); assert.equal(stub.num_calls, 1); const args = stub.get_args("sub", "val");